From 2cb1272445d2a6616991fc6ede274d9f1f62ff73 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 21 Apr 2022 16:23:12 -0400 Subject: [PATCH] Rethink method for assigning OIDs to the template0 and postgres DBs. 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 --- doc/src/sgml/bki.sgml | 7 ++++--- src/backend/access/transam/xlog.c | 4 ++-- src/backend/catalog/Catalog.pm | 14 ++++++++++++++ src/backend/catalog/catalog.c | 14 +++++++++----- src/backend/catalog/genbki.pl | 8 +++++++- src/backend/utils/init/postinit.c | 2 +- src/bin/initdb/initdb.c | 9 +++++---- src/bin/pg_dump/pg_dump.c | 9 +++++---- src/include/access/transam.h | 4 ---- src/include/catalog/genbki.h | 8 ++++++++ src/include/catalog/pg_database.dat | 2 +- src/include/catalog/pg_database.h | 9 +++++++++ src/include/catalog/renumber_oids.pl | 10 ++++++++++ src/include/catalog/unused_oids | 9 --------- 14 files changed, 75 insertions(+), 34 deletions(-) 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_' }, ] ]]> 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.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..e784538aae 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -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 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/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 1cb4a5b0d2..fcef651c2f 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" @@ -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/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.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 e10e91c0af..611c95656a 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(Template0DbOid, 4); +DECLARE_OID_DEFINING_MACRO(PostgresDbOid, 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');