Horton

Thread

Commits

  1. Update pg_upgrade test for reg* to include regrole and regnamespace.

  1. reg* checks in pg_upgrade are out of date

    Andres Freund <andres@anarazel.de> — 2018-11-22T00:12:14Z

    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

  2. Re: reg* checks in pg_upgrade are out of date

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-11-22T13:49:23Z

    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

  3. Re: reg* checks in pg_upgrade are out of date

    Andres Freund <andres@anarazel.de> — 2018-11-27T01:19:04Z

    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