Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce) - Mailing list pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Apr 20, 2022 at 4:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Having just had to bury my nose in renumber_oids.pl, I thought of a
>> different approach we could take to expose these OIDs to Catalog.pm.
>> That's to invent a new macro that Catalog.pm recognizes, and write
>> something about like this in pg_database.h:
>> DECLARE_OID_DEFINING_MACRO(Template0ObjectId, 4);
>> DECLARE_OID_DEFINING_MACRO(PostgresObjectId, 5);

> I like it!

0001 attached is a revised patch that does it that way.  This seems
like a clearly better answer.

0002 contains the perhaps-slightly-more-controversial changes of
changing the macro names and explicitly pinning no databases.

            regards, tom lane

diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 0275795dea..ece0a934f0 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -44,6 +44,8 @@ sub ParseHeader
     $catalog{columns}     = [];
     $catalog{toasting}    = [];
     $catalog{indexing}    = [];
+    $catalog{other_oids}  = [];
+    $catalog{foreign_keys} = [];
     $catalog{client_code} = [];

     open(my $ifh, '<', $input_file) || die "$input_file: $!";
@@ -118,6 +120,14 @@ sub ParseHeader
                 index_decl => $6
               };
         }
+        elsif (/^DECLARE_OID_DEFINING_MACRO\(\s*(\w+),\s*(\d+)\)/)
+        {
+            push @{ $catalog{other_oids} },
+              {
+                other_name => $1,
+                other_oid  => $2
+              };
+        }
         elsif (
             /^DECLARE_(ARRAY_)?FOREIGN_KEY(_OPT)?\(\s*\(([^)]+)\),\s*(\w+),\s*\(([^)]+)\)\)/
           )
@@ -572,6 +582,10 @@ sub FindAllOidsFromHeaders
         {
             push @oids, $index->{index_oid};
         }
+        foreach my $other (@{ $catalog->{other_oids} })
+        {
+            push @oids, $other->{other_oid};
+        }
     }

     return \@oids;
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index 520f77971b..d7e5c02f95 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -339,9 +339,11 @@ IsPinnedObject(Oid classId, Oid objectId)
      * robustness.
      */

-    /* template1 is not pinned */
+    /* template1, template0, postgres are not pinned */
     if (classId == DatabaseRelationId &&
-        objectId == TemplateDbOid)
+        (objectId == TemplateDbOid ||
+         objectId == Template0ObjectId ||
+         objectId == PostgresObjectId))
         return false;

     /* the public namespace is not pinned */
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 2d02b02267..f4ec6d6d40 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -472,7 +472,7 @@ EOM
       $catalog->{rowtype_oid_macro}, $catalog->{rowtype_oid}
       if $catalog->{rowtype_oid_macro};

-    # Likewise for macros for toast and index OIDs
+    # Likewise for macros for toast, index, and other OIDs
     foreach my $toast (@{ $catalog->{toasting} })
     {
         printf $def "#define %s %s\n",
@@ -488,6 +488,12 @@ EOM
           $index->{index_oid_macro}, $index->{index_oid}
           if $index->{index_oid_macro};
     }
+    foreach my $other (@{ $catalog->{other_oids} })
+    {
+        printf $def "#define %s %s\n",
+          $other->{other_name}, $other->{other_oid}
+          if $other->{other_name};
+    }

     print $def "\n";

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 1cb4a5b0d2..5e2eeefc4c 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -59,11 +59,11 @@
 #include "sys/mman.h"
 #endif

-#include "access/transam.h"
 #include "access/xlog_internal.h"
 #include "catalog/pg_authid_d.h"
 #include "catalog/pg_class_d.h" /* pgrminclude ignore */
 #include "catalog/pg_collation_d.h"
+#include "catalog/pg_database_d.h"    /* pgrminclude ignore */
 #include "common/file_perm.h"
 #include "common/file_utils.h"
 #include "common/logging.h"
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d3588607e7..786d592e2b 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2901,10 +2901,11 @@ dumpDatabase(Archive *fout)
     qdatname = pg_strdup(fmtId(datname));

     /*
-     * Prepare the CREATE DATABASE command.  We must specify encoding, locale,
-     * and tablespace since those can't be altered later.  Other DB properties
-     * are left to the DATABASE PROPERTIES entry, so that they can be applied
-     * after reconnecting to the target DB.
+     * Prepare the CREATE DATABASE command.  We must specify OID (if we want
+     * to preserve that), as well as the encoding, locale, and tablespace
+     * since those can't be altered later.  Other DB properties are left to
+     * the DATABASE PROPERTIES entry, so that they can be applied after
+     * reconnecting to the target DB.
      */
     if (dopt->binary_upgrade)
     {
diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index 9a2816de51..338dfca5a0 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -196,10 +196,6 @@ FullTransactionIdAdvance(FullTransactionId *dest)
 #define FirstUnpinnedObjectId    12000
 #define FirstNormalObjectId        16384

-/* OIDs of Template0 and Postgres database are fixed */
-#define Template0ObjectId        4
-#define PostgresObjectId        5
-
 /*
  * VariableCache is a data structure in shared memory that is used to track
  * OID and XID assignment state.  For largely historical reasons, there is
diff --git a/src/include/catalog/genbki.h b/src/include/catalog/genbki.h
index 4ecd76f4be..992b784236 100644
--- a/src/include/catalog/genbki.h
+++ b/src/include/catalog/genbki.h
@@ -84,6 +84,14 @@
 #define DECLARE_UNIQUE_INDEX(name,oid,oidmacro,decl) extern int no_such_variable
 #define DECLARE_UNIQUE_INDEX_PKEY(name,oid,oidmacro,decl) extern int no_such_variable

+/*
+ * These lines inform genbki.pl about manually-assigned OIDs that do not
+ * correspond to any entry in the catalog *.dat files, but should be subject
+ * to uniqueness verification and renumber_oids.pl renumbering.  A C macro
+ * to #define the given name is emitted into the corresponding *_d.h file.
+ */
+#define DECLARE_OID_DEFINING_MACRO(name,oid) extern int no_such_variable
+
 /*
  * These lines are processed by genbki.pl to create a table for use
  * by the pg_get_catalog_foreign_keys() function.  We do not have any
diff --git a/src/include/catalog/pg_database.h b/src/include/catalog/pg_database.h
index e10e91c0af..96be9e9729 100644
--- a/src/include/catalog/pg_database.h
+++ b/src/include/catalog/pg_database.h
@@ -91,4 +91,13 @@ DECLARE_TOAST_WITH_MACRO(pg_database, 4177, 4178, PgDatabaseToastTable, PgDataba
 DECLARE_UNIQUE_INDEX(pg_database_datname_index, 2671, DatabaseNameIndexId, on pg_database using btree(datname
name_ops));
 DECLARE_UNIQUE_INDEX_PKEY(pg_database_oid_index, 2672, DatabaseOidIndexId, on pg_database using btree(oid oid_ops));

+/*
+ * pg_database.dat contains an entry for template1, but not for the template0
+ * or postgres databases, because those are created later in initdb.
+ * However, we still want to manually assign the OIDs for template0 and
+ * postgres, so declare those here.
+ */
+DECLARE_OID_DEFINING_MACRO(Template0ObjectId, 4);
+DECLARE_OID_DEFINING_MACRO(PostgresObjectId, 5);
+
 #endif                            /* PG_DATABASE_H */
diff --git a/src/include/catalog/renumber_oids.pl b/src/include/catalog/renumber_oids.pl
index 7de13da4bd..ba8c69c87e 100755
--- a/src/include/catalog/renumber_oids.pl
+++ b/src/include/catalog/renumber_oids.pl
@@ -170,6 +170,16 @@ foreach my $input_file (@header_files)
                 $changed = 1;
             }
         }
+        elsif (/^(DECLARE_OID_DEFINING_MACRO\(\s*\w+,\s*)(\d+)\)/)
+        {
+            if (exists $maphash{$2})
+            {
+                my $repl = $1 . $maphash{$2} . ")";
+                $line =~
+                  s/^DECLARE_OID_DEFINING_MACRO\(\s*\w+,\s*\d+\)/$repl/;
+                $changed = 1;
+            }
+        }
         elsif ($line =~ m/^CATALOG\((\w+),(\d+),(\w+)\)/)
         {
             if (exists $maphash{$2})
diff --git a/src/include/catalog/unused_oids b/src/include/catalog/unused_oids
index 61d41e7561..e55bc6fa3c 100755
--- a/src/include/catalog/unused_oids
+++ b/src/include/catalog/unused_oids
@@ -32,15 +32,6 @@ my @input_files = glob("pg_*.h");

 my $oids = Catalog::FindAllOidsFromHeaders(@input_files);

-# Push the template0 and postgres database OIDs.
-my $Template0ObjectId =
-  Catalog::FindDefinedSymbol('access/transam.h', '..', 'Template0ObjectId');
-push @{$oids}, $Template0ObjectId;
-
-my $PostgresObjectId =
-  Catalog::FindDefinedSymbol('access/transam.h', '..', 'PostgresObjectId');
-push @{$oids}, $PostgresObjectId;
-
 # Also push FirstGenbkiObjectId to serve as a terminator for the last gap.
 my $FirstGenbkiObjectId =
   Catalog::FindDefinedSymbol('access/transam.h', '..', 'FirstGenbkiObjectId');
diff --git a/doc/src/sgml/bki.sgml b/doc/src/sgml/bki.sgml
index 33955494c6..20894baf18 100644
--- a/doc/src/sgml/bki.sgml
+++ b/doc/src/sgml/bki.sgml
@@ -180,12 +180,13 @@
 [

 # A comment could appear here.
-{ oid => '1', oid_symbol => 'TemplateDbOid',
+{ oid => '1', oid_symbol => 'Template1DbOid',
   descr => 'database\'s default template',
-  datname => 'template1', encoding => 'ENCODING', datistemplate => 't',
+  datname => 'template1', encoding => 'ENCODING',
+  datlocprovider => 'LOCALE_PROVIDER', datistemplate => 't',
   datallowconn => 't', datconnlimit => '-1', datfrozenxid => '0',
   datminmxid => '1', dattablespace => 'pg_default', datcollate => 'LC_COLLATE',
-  datctype => 'LC_CTYPE', datacl => '_null_' },
+  datctype => 'LC_CTYPE', daticulocale => 'ICU_LOCALE', datacl => '_null_' },

 ]
 ]]></programlisting>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5eabd32cf6..61cda56c6f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4540,9 +4540,9 @@ BootStrapXLOG(void)
     checkPoint.nextMulti = FirstMultiXactId;
     checkPoint.nextMultiOffset = 0;
     checkPoint.oldestXid = FirstNormalTransactionId;
-    checkPoint.oldestXidDB = TemplateDbOid;
+    checkPoint.oldestXidDB = Template1DbOid;
     checkPoint.oldestMulti = FirstMultiXactId;
-    checkPoint.oldestMultiDB = TemplateDbOid;
+    checkPoint.oldestMultiDB = Template1DbOid;
     checkPoint.oldestCommitTsXid = InvalidTransactionId;
     checkPoint.newestCommitTsXid = InvalidTransactionId;
     checkPoint.time = (pg_time_t) time(NULL);
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index d7e5c02f95..e784538aae 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -339,18 +339,20 @@ IsPinnedObject(Oid classId, Oid objectId)
      * robustness.
      */

-    /* template1, template0, postgres are not pinned */
-    if (classId == DatabaseRelationId &&
-        (objectId == TemplateDbOid ||
-         objectId == Template0ObjectId ||
-         objectId == PostgresObjectId))
-        return false;
-
     /* the public namespace is not pinned */
     if (classId == NamespaceRelationId &&
         objectId == PG_PUBLIC_NAMESPACE)
         return false;

+    /*
+     * Databases are never pinned.  It might seem that it'd be prudent to pin
+     * at least template0; but we do this intentionally so that template0 and
+     * template1 can be rebuilt from each other, thus letting them serve as
+     * mutual backups (as long as you've not modified template1, anyway).
+     */
+    if (classId == DatabaseRelationId)
+        return false;
+
     /*
      * All other initdb-created objects are pinned.  This is overkill (the
      * system doesn't really depend on having every last weird datatype, for
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 9139fe895c..5dbc7379e3 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -908,7 +908,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
      */
     if (bootstrap)
     {
-        MyDatabaseId = TemplateDbOid;
+        MyDatabaseId = Template1DbOid;
         MyDatabaseTableSpace = DEFAULTTABLESPACE_OID;
     }
     else if (in_dbname != NULL)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 5e2eeefc4c..fcef651c2f 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1812,8 +1812,8 @@ make_template0(FILE *cmdfd)
      * be a little bit slower and make the new cluster a little bit bigger.
      */
     static const char *const template0_setup[] = {
-        "CREATE DATABASE template0 IS_TEMPLATE = true ALLOW_CONNECTIONS = false OID = "
-        CppAsString2(Template0ObjectId)
+        "CREATE DATABASE template0 IS_TEMPLATE = true ALLOW_CONNECTIONS = false"
+        " OID = " CppAsString2(Template0DbOid)
         " STRATEGY = file_copy;\n\n",

         /*
@@ -1862,7 +1862,8 @@ make_postgres(FILE *cmdfd)
      * OID to postgres and select the file_copy strategy.
      */
     static const char *const postgres_setup[] = {
-        "CREATE DATABASE postgres OID = " CppAsString2(PostgresObjectId) " STRATEGY = file_copy;\n\n",
+        "CREATE DATABASE postgres OID = " CppAsString2(PostgresDbOid)
+        " STRATEGY = file_copy;\n\n",
         "COMMENT ON DATABASE postgres IS 'default administrative connection database';\n\n",
         NULL
     };
diff --git a/src/include/catalog/pg_database.dat b/src/include/catalog/pg_database.dat
index 5feedff7bf..05873f74f6 100644
--- a/src/include/catalog/pg_database.dat
+++ b/src/include/catalog/pg_database.dat
@@ -12,7 +12,7 @@

 [

-{ oid => '1', oid_symbol => 'TemplateDbOid',
+{ oid => '1', oid_symbol => 'Template1DbOid',
   descr => 'default template for new databases',
   datname => 'template1', encoding => 'ENCODING', datlocprovider => 'LOCALE_PROVIDER', datistemplate => 't',
   datallowconn => 't', datconnlimit => '-1', datfrozenxid => '0',
diff --git a/src/include/catalog/pg_database.h b/src/include/catalog/pg_database.h
index 96be9e9729..611c95656a 100644
--- a/src/include/catalog/pg_database.h
+++ b/src/include/catalog/pg_database.h
@@ -97,7 +97,7 @@ DECLARE_UNIQUE_INDEX_PKEY(pg_database_oid_index, 2672, DatabaseOidIndexId, on pg
  * However, we still want to manually assign the OIDs for template0 and
  * postgres, so declare those here.
  */
-DECLARE_OID_DEFINING_MACRO(Template0ObjectId, 4);
-DECLARE_OID_DEFINING_MACRO(PostgresObjectId, 5);
+DECLARE_OID_DEFINING_MACRO(Template0DbOid, 4);
+DECLARE_OID_DEFINING_MACRO(PostgresDbOid, 5);

 #endif                            /* PG_DATABASE_H */