Re: pg_upgrade and relkind filtering

On Sat, Dec 31, 2011 at 07:41:00PM -0500, Noah Misch wrote:
> On Mon, Dec 05, 2011 at 05:06:37PM -0500, Bruce Momjian wrote:
> > Pg_upgrade has the following check to make sure the cluster is safe for
> > upgrading:
> >
> > res = executeQueryOrDie(conn,
> > "SELECT n.nspname, c.relname, a.attname
> > "
> > "FROM pg_catalog.pg_class c, "
> > " pg_catalog.pg_namespace n, "
> > " pg_catalog.pg_attribute a "
> > "WHERE c.oid = a.attrelid AND "
> > " NOT a.attisdropped AND "
> > " a.atttypid IN ( "
> > " 'pg_catalog.regproc'::pg_catalog.regtype, "
> > " 'pg_catalog.regprocedure'::pg_catalog.regtype, "
> > " 'pg_catalog.regoper'::pg_catalog.regtype, "
> > " 'pg_catalog.regoperator'::pg_catalog.regtype, "
> > /* regclass.oid is preserved, so 'regclass' is OK */
> > /* regtype.oid is preserved, so 'regtype' is OK */
> > " 'pg_catalog.regconfig'::pg_catalog.regtype, "
> > " 'pg_catalog.regdictionary'::pg_catalog.regtype) AND
> > "
> > " c.relnamespace = n.oid AND "
> > " n.nspname != 'pg_catalog' AND "
> > " n.nspname != 'information_schema'");
> >
> > Based on a report from EnterpriseDB, I noticed that we check all
> > pg_class entries, while there are cases where this is unnecessary
> > because there is no data behind the entry, e.g. views. Here are the
> > relkinds supported:
> >
> > #define RELKIND_RELATION 'r' /* ordinary table */
> > #define RELKIND_INDEX 'i' /* secondary index */
> > #define RELKIND_SEQUENCE 'S' /* sequence object */
> > #define RELKIND_TOASTVALUE 't' /* for out-of-line values */
> > #define RELKIND_VIEW 'v' /* view */
> > #define RELKIND_COMPOSITE_TYPE 'c' /* composite type */
> > #define RELKIND_FOREIGN_TABLE 'f' /* foreign table */
> > #define RELKIND_UNCATALOGED 'u' /* not yet cataloged */
> >
> > What types, other than views, can we skip in this query?
>
> RELKIND_UNCATALOGED should never appear on disk, and RELKIND_SEQUENCE and
> RELKIND_TOASTVALUE do not allow adding columns or changing column types. We
> might as well keep validating them. RELKIND_RELATION and RELKIND_INDEX have
> storage, so we must check those.
>
> The remaining three relkinds (RELKIND_VIEW, RELKIND_COMPOSITE_TYPE,
> RELKIND_FOREIGN_TABLE) have no storage, but all are usable as column types in
> other relations that do have storage. You could skip them iff they're unused
> that way, per a check like find_composite_type_dependencies().

Good point. I have applied the attached comment patch to document why
we check all relkinds for regtypes. Thanks.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +