Fix for pg_upgrade migrating pg_largeobject_metadata - Mailing list pgsql-hackers

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > Tom Lane wrote:
> > >> That isn't going to work.  At least not unless you start trying to force
> > >> roles to have the same OIDs in the new installation.
> >
> > > If so I can use the CREATE ROLE ... SYSID clause when doing a binary
> > > upgrade.
> >
> > Oh, I had forgotten we still had that wart in the grammar.
> > It doesn't actually work:
> >
> >         else if (strcmp(defel->defname, "sysid") == 0)
> >         {
> >             ereport(NOTICE,
> >                     (errmsg("SYSID can no longer be specified")));
> >         }
> >
> > Not sure if it's better to try to make that work again than to add
> > another hack in pg_upgrade_support.  On the whole that's a keyword
> > I'd rather see us drop someday soon.
>
> OK, let me work on adding it to pg_upgrade_support.  Glad you saw this.

I have fixed the bug by using pg_upgrade_support.  It was a little
complicated because you need to install the pg_upgrade_support functions
in the super-user database so it is available when you create the users
in the first step of restoring the pg_dumpall file.

I am afraid we have to batckpatch this to fix to 9.0 for 9.0 to 9.0
upgrades.  It does not apply when coming from pre-9.0 because there was
no pg_largeobject_metadata.

For testing I did this:

    CREATE DATABASE lo;
    \c lo
    SELECT lo_import('/etc/motd');
    \set loid `psql -qt -c 'select loid from pg_largeobject' lo`
    CREATE ROLE user1;
    CREATE ROLE user2;
    -- force user2 to have a different user id on restore
    DROP ROLE user1;
    GRANT ALL ON LARGE OBJECT :loid TO user2;

The fixed version shows:

    lo=> select * from pg_largeobject_metadata;
     lomowner |                  lomacl
    ----------+------------------------------------------
           10 | {postgres=rw/postgres,user2=rw/postgres}
    (1 row)

In the broken version, 'user2' was a raw oid, obviously wrong.

Fortunately this was found during my testing and not reported as a bug
by a pg_upgrade user.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/dump.c b/contrib/pg_upgrade/dump.c
index 52ab481..aba95f4 100644
*** /tmp/pgdiff.20347/YPWaqb_dump.c    Wed Jan  5 20:52:07 2011
--- contrib/pg_upgrade/dump.c    Wed Jan  5 20:43:29 2011
*************** generate_old_dump(void)
*** 33,39 ****
   *
   *    This function splits pg_dumpall output into global values and
   *    database creation, and per-db schemas.    This allows us to create
!  *    the toast place holders between restoring these two parts of the
   *    dump.  We split on the first "\connect " after a CREATE ROLE
   *    username match;  this is where the per-db restore starts.
   *
--- 33,39 ----
   *
   *    This function splits pg_dumpall output into global values and
   *    database creation, and per-db schemas.    This allows us to create
!  *    the support functions between restoring these two parts of the
   *    dump.  We split on the first "\connect " after a CREATE ROLE
   *    username match;  this is where the per-db restore starts.
   *
diff --git a/contrib/pg_upgrade/function.c b/contrib/pg_upgrade/function.c
index 2ab8e4f..5675551 100644
*** /tmp/pgdiff.20347/QpVBHa_function.c    Wed Jan  5 20:52:07 2011
--- contrib/pg_upgrade/function.c    Wed Jan  5 20:26:33 2011
***************
*** 13,35 ****


  /*
!  * install_support_functions()
   *
   * pg_upgrade requires some support functions that enable it to modify
   * backend behavior.
   */
  void
! install_support_functions(void)
  {
!     int            dbnum;
!
!     prep_status("Adding support functions to new cluster");
!
!     for (dbnum = 0; dbnum < new_cluster.dbarr.ndbs; dbnum++)
!     {
!         DbInfo       *new_db = &new_cluster.dbarr.dbs[dbnum];
!         PGconn       *conn = connectToServer(&new_cluster, new_db->db_name);
!
          /* suppress NOTICE of dropped objects */
          PQclear(executeQueryOrDie(conn,
                                    "SET client_min_messages = warning;"));
--- 13,28 ----


  /*
!  * install_db_support_functions()
   *
   * pg_upgrade requires some support functions that enable it to modify
   * backend behavior.
   */
  void
! install_db_support_functions(const char *db_name)
  {
!         PGconn *conn = connectToServer(&new_cluster, db_name);
!
          /* suppress NOTICE of dropped objects */
          PQclear(executeQueryOrDie(conn,
                                    "SET client_min_messages = warning;"));
*************** install_support_functions(void)
*** 83,91 ****
                                    "RETURNS VOID "
                                    "AS '$libdir/pg_upgrade_support' "
                                    "LANGUAGE C STRICT;"));
          PQfinish(conn);
-     }
-     check_ok();
  }


--- 76,88 ----
                                    "RETURNS VOID "
                                    "AS '$libdir/pg_upgrade_support' "
                                    "LANGUAGE C STRICT;"));
+         PQclear(executeQueryOrDie(conn,
+                                   "CREATE OR REPLACE FUNCTION "
+              "        binary_upgrade.set_next_pg_authid_oid(OID) "
+                                   "RETURNS VOID "
+                                   "AS '$libdir/pg_upgrade_support' "
+                                   "LANGUAGE C STRICT;"));
          PQfinish(conn);
  }


diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
index 578bdb9..339849d 100644
*** /tmp/pgdiff.20347/6HSV7d_info.c    Wed Jan  5 20:52:07 2011
--- contrib/pg_upgrade/info.c    Wed Jan  5 20:17:46 2011
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 282,288 ****
               "    ON c.relnamespace = n.oid "
               "   LEFT OUTER JOIN pg_catalog.pg_tablespace t "
               "    ON c.reltablespace = t.oid "
!              "WHERE (( n.nspname NOT IN ('pg_catalog', 'information_schema') "
               "    AND c.oid >= %u "
               "    ) OR ( "
               "    n.nspname = 'pg_catalog' "
--- 282,288 ----
               "    ON c.relnamespace = n.oid "
               "   LEFT OUTER JOIN pg_catalog.pg_tablespace t "
               "    ON c.reltablespace = t.oid "
!              "WHERE (( n.nspname NOT IN ('pg_catalog', 'information_schema', 'binary_upgrade') "
               "    AND c.oid >= %u "
               "    ) OR ( "
               "    n.nspname = 'pg_catalog' "
diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c
index 64fb8f8..fe7b3db 100644
*** /tmp/pgdiff.20347/2pEOVc_pg_upgrade.c    Wed Jan  5 20:52:07 2011
--- contrib/pg_upgrade/pg_upgrade.c    Wed Jan  5 20:49:07 2011
*************** prepare_new_databases(void)
*** 195,205 ****

      set_frozenxids();

      /*
!      * We have to create the databases first so we can create the toast table
!      * placeholder relfiles.
       */
-     prep_status("Creating databases in the new cluster");
      exec_prog(true,
                SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on "
      /* --no-psqlrc prevents AUTOCOMMIT=off */
--- 195,209 ----

      set_frozenxids();

+     prep_status("Creating databases in the new cluster");
+
+     /* install support functions in the database used by GLOBALS_DUMP_FILE */
+     install_db_support_functions(os_info.user);
+
      /*
!      * We have to create the databases first so we can install support
!      * functions in all the other databases.
       */
      exec_prog(true,
                SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on "
      /* --no-psqlrc prevents AUTOCOMMIT=off */
*************** prepare_new_databases(void)
*** 218,227 ****
  static void
  create_new_objects(void)
  {
      /* -- NEW -- */
      start_postmaster(&new_cluster, false);

!     install_support_functions();

      prep_status("Restoring database schema to new cluster");
      exec_prog(true,
--- 222,241 ----
  static void
  create_new_objects(void)
  {
+     int            dbnum;
+
      /* -- NEW -- */
      start_postmaster(&new_cluster, false);

!     prep_status("Adding support functions to new cluster");
!
!     for (dbnum = 0; dbnum < new_cluster.dbarr.ndbs; dbnum++)
!     {
!         DbInfo       *new_db = &new_cluster.dbarr.dbs[dbnum];
!
!         install_db_support_functions(new_db->db_name);
!     }
!     check_ok();

      prep_status("Restoring database schema to new cluster");
      exec_prog(true,
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
index d863155..ef2cdff 100644
*** /tmp/pgdiff.20347/gzDtFb_pg_upgrade.h    Wed Jan  5 20:52:07 2011
--- contrib/pg_upgrade/pg_upgrade.h    Wed Jan  5 20:32:45 2011
*************** void        check_hard_link(void);
*** 324,330 ****

  /* function.c */

! void        install_support_functions(void);
  void        uninstall_support_functions(void);
  void        get_loadable_libraries(void);
  void        check_loadable_libraries(void);
--- 324,330 ----

  /* function.c */

! void        install_db_support_functions(const char *db_name);
  void        uninstall_support_functions(void);
  void        get_loadable_libraries(void);
  void        check_loadable_libraries(void);
diff --git a/contrib/pg_upgrade_support/pg_upgrade_support.c b/contrib/pg_upgrade_support/pg_upgrade_support.c
index e55e139..31c2b4e 100644
*** /tmp/pgdiff.20347/szwF0a_pg_upgrade_support.c    Wed Jan  5 20:52:07 2011
--- contrib/pg_upgrade_support/pg_upgrade_support.c    Wed Jan  5 20:17:46 2011
*************** extern PGDLLIMPORT Oid binary_upgrade_ne
*** 27,32 ****
--- 27,33 ----
  extern PGDLLIMPORT Oid binary_upgrade_next_toast_relfilenode;
  extern PGDLLIMPORT Oid binary_upgrade_next_index_relfilenode;
  extern PGDLLIMPORT Oid binary_upgrade_next_pg_enum_oid;
+ extern PGDLLIMPORT Oid binary_upgrade_next_pg_authid_oid;

  Datum        set_next_pg_type_oid(PG_FUNCTION_ARGS);
  Datum        set_next_pg_type_array_oid(PG_FUNCTION_ARGS);
*************** Datum        set_next_heap_relfilenode(PG_FUNC
*** 35,40 ****
--- 36,42 ----
  Datum        set_next_toast_relfilenode(PG_FUNCTION_ARGS);
  Datum        set_next_index_relfilenode(PG_FUNCTION_ARGS);
  Datum        set_next_pg_enum_oid(PG_FUNCTION_ARGS);
+ Datum        set_next_pg_authid_oid(PG_FUNCTION_ARGS);

  PG_FUNCTION_INFO_V1(set_next_pg_type_oid);
  PG_FUNCTION_INFO_V1(set_next_pg_type_array_oid);
*************** PG_FUNCTION_INFO_V1(set_next_heap_relfil
*** 43,48 ****
--- 45,51 ----
  PG_FUNCTION_INFO_V1(set_next_toast_relfilenode);
  PG_FUNCTION_INFO_V1(set_next_index_relfilenode);
  PG_FUNCTION_INFO_V1(set_next_pg_enum_oid);
+ PG_FUNCTION_INFO_V1(set_next_pg_authid_oid);

  Datum
  set_next_pg_type_oid(PG_FUNCTION_ARGS)
*************** set_next_pg_enum_oid(PG_FUNCTION_ARGS)
*** 113,115 ****
--- 116,128 ----

      PG_RETURN_VOID();
  }
+
+ Datum
+ set_next_pg_authid_oid(PG_FUNCTION_ARGS)
+ {
+     Oid            authoid = PG_GETARG_OID(0);
+
+     binary_upgrade_next_pg_authid_oid = authoid;
+     PG_RETURN_VOID();
+ }
+
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index be049cb..93bfd40 100644
*** /tmp/pgdiff.20347/AvSkle_user.c    Wed Jan  5 20:52:07 2011
--- src/backend/commands/user.c    Wed Jan  5 20:17:46 2011
***************
*** 35,40 ****
--- 35,43 ----
  #include "utils/syscache.h"
  #include "utils/tqual.h"

+ /* Kluge for upgrade-in-place support */
+ Oid            binary_upgrade_next_pg_authid_oid = InvalidOid;
+

  /* GUC parameter */
  extern bool Password_encryption;
*************** CreateRole(CreateRoleStmt *stmt)
*** 393,398 ****
--- 396,408 ----

      tuple = heap_form_tuple(pg_authid_dsc, new_record, new_record_nulls);

+     /* Use binary-upgrade override if applicable */
+     if (OidIsValid(binary_upgrade_next_pg_authid_oid))
+     {
+         HeapTupleSetOid(tuple, binary_upgrade_next_pg_authid_oid);
+         binary_upgrade_next_pg_authid_oid = InvalidOid;
+     }
+
      /*
       * Insert new record in the pg_authid table
       */
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 17a73b8..2928232 100644
*** /tmp/pgdiff.20347/aQpZrb_pg_dumpall.c    Wed Jan  5 20:52:07 2011
--- src/bin/pg_dump/pg_dumpall.c    Wed Jan  5 20:17:46 2011
*************** dumpRoles(PGconn *conn)
*** 650,656 ****
  {
      PQExpBuffer buf = createPQExpBuffer();
      PGresult   *res;
!     int            i_rolname,
                  i_rolsuper,
                  i_rolinherit,
                  i_rolcreaterole,
--- 650,657 ----
  {
      PQExpBuffer buf = createPQExpBuffer();
      PGresult   *res;
!     int            i_oid,
!                 i_rolname,
                  i_rolsuper,
                  i_rolinherit,
                  i_rolcreaterole,
*************** dumpRoles(PGconn *conn)
*** 667,700 ****
      /* note: rolconfig is dumped later */
      if (server_version >= 90100)
          printfPQExpBuffer(buf,
!                           "SELECT rolname, rolsuper, rolinherit, "
                            "rolcreaterole, rolcreatedb, rolcatupdate, "
                            "rolcanlogin, rolconnlimit, rolpassword, "
                            "rolvaliduntil, rolreplication, "
                "pg_catalog.shobj_description(oid, 'pg_authid') as rolcomment "
                            "FROM pg_authid "
!                           "ORDER BY 1");
      else if (server_version >= 80200)
          printfPQExpBuffer(buf,
!                           "SELECT rolname, rolsuper, rolinherit, "
                            "rolcreaterole, rolcreatedb, rolcatupdate, "
                            "rolcanlogin, rolconnlimit, rolpassword, "
                            "rolvaliduntil, false as rolreplication, "
                "pg_catalog.shobj_description(oid, 'pg_authid') as rolcomment "
                            "FROM pg_authid "
!                           "ORDER BY 1");
      else if (server_version >= 80100)
          printfPQExpBuffer(buf,
!                           "SELECT rolname, rolsuper, rolinherit, "
                            "rolcreaterole, rolcreatedb, rolcatupdate, "
                            "rolcanlogin, rolconnlimit, rolpassword, "
                            "rolvaliduntil, false as rolreplication, "
                            "null as rolcomment "
                            "FROM pg_authid "
!                           "ORDER BY 1");
      else
          printfPQExpBuffer(buf,
!                           "SELECT usename as rolname, "
                            "usesuper as rolsuper, "
                            "true as rolinherit, "
                            "usesuper as rolcreaterole, "
--- 668,701 ----
      /* note: rolconfig is dumped later */
      if (server_version >= 90100)
          printfPQExpBuffer(buf,
!                           "SELECT oid, rolname, rolsuper, rolinherit, "
                            "rolcreaterole, rolcreatedb, rolcatupdate, "
                            "rolcanlogin, rolconnlimit, rolpassword, "
                            "rolvaliduntil, rolreplication, "
                "pg_catalog.shobj_description(oid, 'pg_authid') as rolcomment "
                            "FROM pg_authid "
!                           "ORDER BY 2");
      else if (server_version >= 80200)
          printfPQExpBuffer(buf,
!                           "SELECT oid, rolname, rolsuper, rolinherit, "
                            "rolcreaterole, rolcreatedb, rolcatupdate, "
                            "rolcanlogin, rolconnlimit, rolpassword, "
                            "rolvaliduntil, false as rolreplication, "
                "pg_catalog.shobj_description(oid, 'pg_authid') as rolcomment "
                            "FROM pg_authid "
!                           "ORDER BY 2");
      else if (server_version >= 80100)
          printfPQExpBuffer(buf,
!                           "SELECT oid, rolname, rolsuper, rolinherit, "
                            "rolcreaterole, rolcreatedb, rolcatupdate, "
                            "rolcanlogin, rolconnlimit, rolpassword, "
                            "rolvaliduntil, false as rolreplication, "
                            "null as rolcomment "
                            "FROM pg_authid "
!                           "ORDER BY 2");
      else
          printfPQExpBuffer(buf,
!                           "SELECT 0, usename as rolname, "
                            "usesuper as rolsuper, "
                            "true as rolinherit, "
                            "usesuper as rolcreaterole, "
*************** dumpRoles(PGconn *conn)
*** 708,714 ****
                            "null as rolcomment "
                            "FROM pg_shadow "
                            "UNION ALL "
!                           "SELECT groname as rolname, "
                            "false as rolsuper, "
                            "true as rolinherit, "
                            "false as rolcreaterole, "
--- 709,715 ----
                            "null as rolcomment "
                            "FROM pg_shadow "
                            "UNION ALL "
!                           "SELECT 0, groname as rolname, "
                            "false as rolsuper, "
                            "true as rolinherit, "
                            "false as rolcreaterole, "
*************** dumpRoles(PGconn *conn)
*** 723,732 ****
                            "FROM pg_group "
                            "WHERE NOT EXISTS (SELECT 1 FROM pg_shadow "
                            " WHERE usename = groname) "
!                           "ORDER BY 1");

      res = executeQuery(conn, buf->data);

      i_rolname = PQfnumber(res, "rolname");
      i_rolsuper = PQfnumber(res, "rolsuper");
      i_rolinherit = PQfnumber(res, "rolinherit");
--- 724,734 ----
                            "FROM pg_group "
                            "WHERE NOT EXISTS (SELECT 1 FROM pg_shadow "
                            " WHERE usename = groname) "
!                           "ORDER BY 2");

      res = executeQuery(conn, buf->data);

+     i_oid = PQfnumber(res, "oid");
      i_rolname = PQfnumber(res, "rolname");
      i_rolsuper = PQfnumber(res, "rolsuper");
      i_rolinherit = PQfnumber(res, "rolinherit");
*************** dumpRoles(PGconn *conn)
*** 751,756 ****
--- 753,768 ----

          resetPQExpBuffer(buf);

+         if (binary_upgrade)
+         {
+             Oid            auth_oid = atooid(PQgetvalue(res, i, i_oid));
+
+             appendPQExpBuffer(buf, "\n-- For binary upgrade, must preserve pg_authid.oid\n");
+             appendPQExpBuffer(buf,
+              "SELECT binary_upgrade.set_next_pg_authid_oid('%u'::pg_catalog.oid);\n\n",
+                               auth_oid);
+         }
+
          /*
           * We dump CREATE ROLE followed by ALTER ROLE to ensure that the role
           * will acquire the right properties even if it already exists (ie, it