Re: Removing pg_migrator limitations - Mailing list pgsql-hackers

Bruce Momjian wrote:
> Bruce Momjian wrote:
> > Tom Lane wrote:
> > > Bruce Momjian <bruce@momjian.us> writes:
> > > > Tom Lane wrote:
> > > >> The reason I don't want to do it that way is that then you need two
> > > >> ugly kluges in the backend, not just one.  With the zero-and-add-one
> > > >> approach there is no need to have a "next enum oid" variable at all.
> > >
> > > > Uh, I still need that variable because that is how we are going to set
> > > > the oid in EnumValuesCreate(), unless we want to add dummy oid-value
> > > > arguments to that function for use only by the binary upgrade
> > > > server-side function.
> > >
> > > Please go back and re-read what I suggested: you need a function along
> > > the lines of
> > >     add_enum_member(enum-type, 'value name', value-oid)
> > > and then there's no need for any saved state.  So what if it has a
> > > different signature from the other pg_migrator special functions?
> > > It's not doing the same thing.
> >
> > OK, right, I can get rid of the enum function that just sets the next
> > oid value if I do all the enum value creation via function calls.  I
> > will work in that direction then.
>
> There is only one call to EnumValuesCreate() so maybe adding a
> binary-upgrade-only parameter to the function will be the cleanest
> approach.

Here is a patch to allow EnumValuesCreate() to create labels with
specified oids, with pg_dump support.  This is done cleanly now that we
allow zero-label enums.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/catalog/pg_enum.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/pg_enum.c,v
retrieving revision 1.11
diff -c -c -r1.11 pg_enum.c
*** src/backend/catalog/pg_enum.c    24 Dec 2009 22:17:58 -0000    1.11
--- src/backend/catalog/pg_enum.c    26 Dec 2009 18:48:55 -0000
***************
*** 33,39 ****
   * vals is a list of Value strings.
   */
  void
! EnumValuesCreate(Oid enumTypeOid, List *vals)
  {
      Relation    pg_enum;
      TupleDesc    tupDesc;
--- 33,40 ----
   * vals is a list of Value strings.
   */
  void
! EnumValuesCreate(Oid enumTypeOid, List *vals,
!                  Oid binary_upgrade_next_pg_enum_oid)
  {
      Relation    pg_enum;
      TupleDesc    tupDesc;
***************
*** 58,82 ****
      tupDesc = pg_enum->rd_att;

      /*
!      * Allocate oids.  While this method does not absolutely guarantee that we
!      * generate no duplicate oids (since we haven't entered each oid into the
!      * table before allocating the next), trouble could only occur if the oid
!      * counter wraps all the way around before we finish. Which seems
!      * unlikely.
       */
      oids = (Oid *) palloc(num_elems * sizeof(Oid));
!     for (elemno = 0; elemno < num_elems; elemno++)
      {
          /*
!          *    The pg_enum.oid is stored in user tables.  This oid must be
!          *    preserved by binary upgrades.
           */
!         oids[elemno] = GetNewOid(pg_enum);
      }

-     /* sort them, just in case counter wrapped from high to low */
-     qsort(oids, num_elems, sizeof(Oid), oid_cmp);
-
      /* and make the entries */
      memset(nulls, false, sizeof(nulls));

--- 59,97 ----
      tupDesc = pg_enum->rd_att;

      /*
!      *    Allocate oids
       */
      oids = (Oid *) palloc(num_elems * sizeof(Oid));
!     if (OidIsValid(binary_upgrade_next_pg_enum_oid))
!     {
!             if (num_elems != 1)
!                 ereport(ERROR,
!                         (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
!                          errmsg("EnumValuesCreate() can only set a single OID")));
!             oids[0] = binary_upgrade_next_pg_enum_oid;
!             binary_upgrade_next_pg_enum_oid = InvalidOid;
!     }
!     else
      {
          /*
!          * While this method does not absolutely guarantee that we generate
!          * no duplicate oids (since we haven't entered each oid into the
!          * table before allocating the next), trouble could only occur if
!          * the oid counter wraps all the way around before we finish. Which
!          * seems unlikely.
           */
!         for (elemno = 0; elemno < num_elems; elemno++)
!         {
!             /*
!              *    The pg_enum.oid is stored in user tables.  This oid must be
!              *    preserved by binary upgrades.
!              */
!             oids[elemno] = GetNewOid(pg_enum);
!         }
!         /* sort them, just in case counter wrapped from high to low */
!         qsort(oids, num_elems, sizeof(Oid), oid_cmp);
      }

      /* and make the entries */
      memset(nulls, false, sizeof(nulls));

Index: src/backend/commands/typecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/typecmds.c,v
retrieving revision 1.141
diff -c -c -r1.141 typecmds.c
*** src/backend/commands/typecmds.c    24 Dec 2009 22:09:23 -0000    1.141
--- src/backend/commands/typecmds.c    26 Dec 2009 18:48:56 -0000
***************
*** 1161,1167 ****
                     false);        /* Type NOT NULL */

      /* Enter the enum's values into pg_enum */
!     EnumValuesCreate(enumTypeOid, stmt->vals);

      /*
       * Create the array type that goes with it.
--- 1161,1167 ----
                     false);        /* Type NOT NULL */

      /* Enter the enum's values into pg_enum */
!     EnumValuesCreate(enumTypeOid, stmt->vals, InvalidOid);

      /*
       * Create the array type that goes with it.
Index: src/bin/pg_dump/pg_dump.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.562
diff -c -c -r1.562 pg_dump.c
*** src/bin/pg_dump/pg_dump.c    26 Dec 2009 16:55:21 -0000    1.562
--- src/bin/pg_dump/pg_dump.c    26 Dec 2009 18:48:57 -0000
***************
*** 6528,6539 ****
      PGresult   *res;
      int            num,
                  i;
      char       *label;

      /* Set proper schema search path so regproc references list correctly */
      selectSourceSchema(tyinfo->dobj.namespace->dobj.name);

!     appendPQExpBuffer(query, "SELECT enumlabel FROM pg_catalog.pg_enum "
                        "WHERE enumtypid = '%u'"
                        "ORDER BY oid",
                        tyinfo->dobj.catId.oid);
--- 6528,6541 ----
      PGresult   *res;
      int            num,
                  i;
+     Oid            enum_oid;
      char       *label;

      /* Set proper schema search path so regproc references list correctly */
      selectSourceSchema(tyinfo->dobj.namespace->dobj.name);

!     appendPQExpBuffer(query, "SELECT oid, enumlabel "
!                       "FROM pg_catalog.pg_enum "
                        "WHERE enumtypid = '%u'"
                        "ORDER BY oid",
                        tyinfo->dobj.catId.oid);
***************
*** 6556,6573 ****
      if (binary_upgrade)
          binary_upgrade_set_type_oids_by_type_oid(q, tyinfo->dobj.catId.oid);

!     appendPQExpBuffer(q, "CREATE TYPE %s AS ENUM (\n",
                        fmtId(tyinfo->dobj.name));
!     for (i = 0; i < num; i++)
      {
!         label = PQgetvalue(res, i, 0);
!         if (i > 0)
!             appendPQExpBuffer(q, ",\n");
!         appendPQExpBuffer(q, "    ");
!         appendStringLiteralAH(q, label, fout);
      }
      appendPQExpBuffer(q, "\n);\n");

      ArchiveEntry(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId,
                   tyinfo->dobj.name,
                   tyinfo->dobj.namespace->dobj.name,
--- 6558,6601 ----
      if (binary_upgrade)
          binary_upgrade_set_type_oids_by_type_oid(q, tyinfo->dobj.catId.oid);

!     appendPQExpBuffer(q, "CREATE TYPE %s AS ENUM (",
                        fmtId(tyinfo->dobj.name));
!
!     if (!binary_upgrade)
      {
!         /* Labels with server-assigned oids */
!         for (i = 0; i < num; i++)
!         {
!             label = PQgetvalue(res, i, PQfnumber(res, "enumlabel"));
!             if (i > 0)
!                 appendPQExpBuffer(q, ",");
!             appendPQExpBuffer(q, "\n    ");
!             appendStringLiteralAH(q, label, fout);
!         }
      }
+
      appendPQExpBuffer(q, "\n);\n");

+     if (binary_upgrade)
+     {
+         /* Labels with dump-assigned (preserved) oids */
+         for (i = 0; i < num; i++)
+         {
+             enum_oid = atooid(PQgetvalue(res, i, PQfnumber(res, "oid")));
+             label = PQgetvalue(res, i, PQfnumber(res, "enumlabel"));
+
+             if (i == 0)
+                 appendPQExpBuffer(q, "\n-- For binary upgrade, must preserve pg_enum oids\n");
+             appendPQExpBuffer(q,
+                 "SELECT binary_upgrade.add_pg_enum_label('%u'::pg_catalog.oid, "
+                 "'%u'::pg_catalog.oid, ",
+                 enum_oid, tyinfo->dobj.catId.oid);
+             appendStringLiteralAH(q, label, fout);
+             appendPQExpBuffer(q, ");\n");
+         }
+         appendPQExpBuffer(q, "\n");
+     }
+
      ArchiveEntry(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId,
                   tyinfo->dobj.name,
                   tyinfo->dobj.namespace->dobj.name,
Index: src/include/catalog/pg_enum.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_enum.h,v
retrieving revision 1.5
diff -c -c -r1.5 pg_enum.h
*** src/include/catalog/pg_enum.h    1 Jan 2009 17:23:57 -0000    1.5
--- src/include/catalog/pg_enum.h    26 Dec 2009 18:49:00 -0000
***************
*** 60,66 ****
  /*
   * prototypes for functions in pg_enum.c
   */
! extern void EnumValuesCreate(Oid enumTypeOid, List *vals);
  extern void EnumValuesDelete(Oid enumTypeOid);

  #endif   /* PG_ENUM_H */
--- 60,67 ----
  /*
   * prototypes for functions in pg_enum.c
   */
! extern void EnumValuesCreate(Oid enumTypeOid, List *vals,
!             Oid binary_upgrade_next_pg_enum_oid);
  extern void EnumValuesDelete(Oid enumTypeOid);

  #endif   /* PG_ENUM_H */