Thread: reg* checks in pg_upgrade are out of date
Hi, It seems the list of reg* types and the check for them in pg_upgrade have gone out of sync. We have the following reg* types: SELECT typname FROM pg_type WHERE typname LIKE 'reg%' order by typname; ┌───────────────┐ │ typname │ ├───────────────┤ │ regclass │ │ regconfig │ │ regdictionary │ │ regnamespace │ │ regoper │ │ regoperator │ │ regproc │ │ regprocedure │ │ regrole │ │ regtype │ └───────────────┘ (10 rows) but pg_upgrade doesn't consider all of them: /* * 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, " " 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 NOT IN ('pg_catalog', 'information_schema')"); (I don't get the order here btw) ISTM when regrole and regnamespace were added, pg_upgrade wasn't considered. It turns out that regrole is safe, because we preserve user oids, but regnamespace isn't afaict. I don't think it's extremely likely that users store such reg* columns in tables, but we probably still should fix this. Greetings, Andres Freund
On 11/21/18 7:12 PM, Andres Freund wrote: > Hi, > > It seems the list of reg* types and the check for them in pg_upgrade > have gone out of sync. We have the following reg* types: > > SELECT typname FROM pg_type WHERE typname LIKE 'reg%' order by typname; > ┌───────────────┐ > │ typname │ > ├───────────────┤ > │ regclass │ > │ regconfig │ > │ regdictionary │ > │ regnamespace │ > │ regoper │ > │ regoperator │ > │ regproc │ > │ regprocedure │ > │ regrole │ > │ regtype │ > └───────────────┘ > (10 rows) > > but pg_upgrade doesn't consider all of them: > > /* > * 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, " > " 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 NOT IN ('pg_catalog', 'information_schema')"); > > (I don't get the order here btw) > > ISTM when regrole and regnamespace were added, pg_upgrade wasn't > considered. It turns out that regrole is safe, because we preserve user > oids, but regnamespace isn't afaict. I don't think it's extremely > likely that users store such reg* columns in tables, but we probably > still should fix this. > yeah, I think you're right, both about the need to fix it and the unlikelihood of it occurring in the wild. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-11-22 08:49:23 -0500, Andrew Dunstan wrote: > > On 11/21/18 7:12 PM, Andres Freund wrote: > > Hi, > > > > It seems the list of reg* types and the check for them in pg_upgrade > > have gone out of sync. We have the following reg* types: > > > > SELECT typname FROM pg_type WHERE typname LIKE 'reg%' order by typname; > > ┌───────────────┐ > > │ typname │ > > ├───────────────┤ > > │ regclass │ > > │ regconfig │ > > │ regdictionary │ > > │ regnamespace │ > > │ regoper │ > > │ regoperator │ > > │ regproc │ > > │ regprocedure │ > > │ regrole │ > > │ regtype │ > > └───────────────┘ > > (10 rows) > > > > but pg_upgrade doesn't consider all of them: > > > > /* > > * 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, " > > " 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 NOT IN ('pg_catalog', 'information_schema')"); > > > > (I don't get the order here btw) > > > > ISTM when regrole and regnamespace were added, pg_upgrade wasn't > > considered. It turns out that regrole is safe, because we preserve user > > oids, but regnamespace isn't afaict. I don't think it's extremely > > likely that users store such reg* columns in tables, but we probably > > still should fix this. > > > > yeah, I think you're right, both about the need to fix it and the > unlikelihood of it occurring in the wild. I've done so, and backpatched to 9.5, where these types where added. Greetings, Andres Freund