Re: [HACKERS] 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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c new file mode 100644 index 8594d26..891eb9a *** a/contrib/pg_upgrade/check.c --- b/contrib/pg_upgrade/check.c *** check_for_reg_data_type_usage(ClusterInf *** 644,649 --- 644,654 DbInfo *active_db = cluster-dbarr.dbs[dbnum]; PGconn *conn = connectToServer(cluster, active_db-db_name); + /* + * While several relkinds don't store any data, e.g. views, they + * can be used to define data types of other columns, so we + * check all relkinds. + */ res = executeQueryOrDie(conn, SELECT n.nspname, c.relname, a.attname FROM pg_catalog.pg_class c, -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and relkind filtering
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(). -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and relkind filtering
On Mon, Dec 5, 2011 at 5:06 PM, Bruce Momjian br...@momjian.us 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? It's not obvious to me that anything other than a table or index would matter. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and relkind filtering
Robert Haas wrote: On Mon, Dec 5, 2011 at 5:06 PM, Bruce Momjian br...@momjian.us 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? It's not obvious to me that anything other than a table or index would matter. Well, I assume the composite type could be referenced by another table, and the foreign table might have data stored in it that is now invalid. Toast and sequences are probably safely skipped, but also probably never a problem to check. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and relkind filtering
Robert Haas robertmh...@gmail.com writes: On Mon, Dec 5, 2011 at 5:06 PM, Bruce Momjian br...@momjian.us wrote: Pg_upgrade has the following check to make sure the cluster is safe for upgrading: What types, other than views, can we skip in this query? It's not obvious to me that anything other than a table or index would matter. You'd better complain about composite types too, since one of them could be a column in a table. (Unless you want to test to see whether it actually is stored anywhere, but that seems like way overkill for this.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_upgrade and relkind filtering
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? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
