Rethink method for assigning OIDs to the template0 and postgres DBs.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 21 Apr 2022 20:23:12 +0000 (16:23 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 21 Apr 2022 20:23:15 +0000 (16:23 -0400)
Commit aa0105141 assigned fixed OIDs to template0 and postgres
in a very ad-hoc way.  Notably, instead of teaching Catalog.pm
about these OIDs, the unused_oids script was just hacked to
not show them as unused.  That's problematic since, for example,
duplicate_oids wouldn't report any future conflict.  Hence,
invent a macro DECLARE_OID_DEFINING_MACRO() that can be used to
define an OID that is known to Catalog.pm and will participate
in duplicate-detection as well as renumbering by renumber_oids.pl.
(We don't anticipate renumbering these particular OIDs, but we
might as well build out all the Catalog.pm infrastructure while
we're here.)

Another issue is that aa0105141 neglected to touch IsPinnedObject,
with the result that it now claimed template0 and postgres are
pinned.  The right thing to do there seems to be to teach it that
no database is pinned, since in fact DROP DATABASE doesn't check
for pinned-ness (and at least for these cases, that is an
intentional choice).  It's not clear whether this wrong answer
had any visible effect, but perhaps it could have resulted in
erroneous management of dependency entries.

In passing, rename the TemplateDbOid macro to Template1DbOid
to reduce confusion (likely we should have done that way back
when we invented template0, but we didn't), and rename the
OID macros for template0 and postgres to have a similar style.

There are no changes to postgres.bki here, so no need for a
catversion bump.

Discussion: https://postgr.es/m/2935358.1650479692@sss.pgh.pa.us

14 files changed:
doc/src/sgml/bki.sgml
src/backend/access/transam/xlog.c
src/backend/catalog/Catalog.pm
src/backend/catalog/catalog.c
src/backend/catalog/genbki.pl
src/backend/utils/init/postinit.c
src/bin/initdb/initdb.c
src/bin/pg_dump/pg_dump.c
src/include/access/transam.h
src/include/catalog/genbki.h
src/include/catalog/pg_database.dat
src/include/catalog/pg_database.h
src/include/catalog/renumber_oids.pl
src/include/catalog/unused_oids

index 33955494c632ad8becc272c35402c4896f5b2b96..20894baf185683ced4fadced26be3cf5b2d2fc9a 100644 (file)
 [
 
 # 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>
index 5eabd32cf6ae0fbd49fdc0ea906dfb4d227d7129..61cda56c6ff26f3de1ba1459feea04e00b8cd468 100644 (file)
@@ -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);
index 0275795dea44f1b75cb772f96a1c3ce54fba5712..ece0a934f05be8a0c2ca9682e2a3d89312519ebe 100644 (file)
@@ -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;
index 520f77971b396196eff2abca54b8d72bfbd93234..e784538aaea49490be489ca6496cbcd0a30b9529 100644 (file)
@@ -339,16 +339,20 @@ IsPinnedObject(Oid classId, Oid objectId)
     * robustness.
     */
 
-   /* template1 is not pinned */
-   if (classId == DatabaseRelationId &&
-       objectId == TemplateDbOid)
-       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
index 2d02b0226707275efbabe003f3f6eee4bd2edd1e..f4ec6d6d40cb061f6601ae15e4b6faaf54d03fbf 100644 (file)
@@ -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";
 
index 9139fe895c0efad39db240bea0e703e0877453af..5dbc7379e36864ea06f4383edf29fd2ffd8599a5 100644 (file)
@@ -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)
index 1cb4a5b0d21430756c9578c5d81c71134082d4da..fcef651c2fc6e13c18eb993ea676be7e2c717036 100644 (file)
 #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"
@@ -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
    };
index d3588607e744b113fae70e67d047f2aef69c6286..786d592e2ba0a8ea1e2940e11432741c5a5931f9 100644 (file)
@@ -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)
    {
index 9a2816de51529f1c51b367a1e60722ee1418a846..338dfca5a0bb9772094db4a24a236705373f0c17 100644 (file)
@@ -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
index 4ecd76f4be7e1312e4b88782a2f2799780085e7f..992b7842363ae2a619fe0510e802e4ebb2b06f65 100644 (file)
 #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
index 5feedff7bff55c16d039c604cbb984b45139c6e0..05873f74f68446c122e4ee3aa607d39f6e9c571b 100644 (file)
@@ -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',
index e10e91c0af1b6550d25fa388d951c9820f631e82..611c95656a92311739bbf58f7032498e209e8e18 100644 (file)
@@ -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(Template0DbOid, 4);
+DECLARE_OID_DEFINING_MACRO(PostgresDbOid, 5);
+
 #endif                         /* PG_DATABASE_H */
index 7de13da4bdd2afba94368263056c5b352afab4be..ba8c69c87e9af0ca294485717f6856fd1b041660 100755 (executable)
@@ -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})
index 61d41e7561865df23b761e3fd52235f9f5af26d7..e55bc6fa3c31d4da18526152ba059c0476fa09b6 100755 (executable)
@@ -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');