From aa01051418f10afbdfa781b8dc109615ca785ff9 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 24 Jan 2022 14:23:15 -0500 Subject: [PATCH] pg_upgrade: Preserve database OIDs. Commit 9a974cbcba005256a19991203583a94b4f9a21a9 arranged to preserve relfilenodes and tablespace OIDs. For similar reasons, also arrange to preserve database OIDs. One problem is that, up until now, the OIDs assigned to the template0 and postgres databases have not been fixed. This could be a problem when upgrading, because pg_upgrade might try to migrate a database from the old cluster to the new cluster while keeping the OID and find a different database with that OID, resulting in a failure. If it finds a database with the same name and the same OID that's OK: it will be dropped and recreated. But the same OID and a different name is a problem. To prevent that, fix the OIDs for postgres and template0 to specific values less than 16384. To avoid running afoul of this rule, these values should not be changed in future releases. It's not a problem that these OIDs aren't fixed in existing releases, because the OIDs that we're assigning here weren't used for either of these databases in any previous release. Thus, there's no chance that an upgrade of a cluster from any previous release will collide with the OIDs we're assigning here. And going forward, the OIDs will always be fixed, so the only potential collision is with a system database having the same name and the same OID, which is OK. This patch lets users assign a specific OID to a database as well, provided however that it can't be less than 16384. I (rhaas) thought it might be better not to expose this capability to users, but the consensus was otherwise, so the syntax is documented. Letting users assign OIDs below 16384 would not be OK, though, because a user-created database with a low-numbered OID might collide with a system-created database in a future release. We therefore prohibit that. Shruthi KC, based on an earlier patch from Antonin Houska, reviewed and with some adjustments by me. Discussion: http://postgr.es/m/CA+TgmoYgTwYcUmB=e8+hRHOFA0kkS6Kde85+UNdon6q7bt1niQ@mail.gmail.com Discussion: http://postgr.es/m/CAASxf_Mnwm1Dh2vd5FAhVX6S1nwNSZUB1z12VddYtM++H2+p7w@mail.gmail.com --- doc/src/sgml/ref/create_database.sgml | 18 ++++++++- src/backend/commands/dbcommands.c | 57 ++++++++++++++++++++++++--- src/bin/initdb/initdb.c | 22 ++++++++++- src/bin/pg_dump/pg_dump.c | 12 +++++- src/bin/pg_upgrade/IMPLEMENTATION | 2 +- src/bin/pg_upgrade/info.c | 9 ++--- src/bin/pg_upgrade/pg_upgrade.h | 3 +- src/bin/pg_upgrade/relfilenode.c | 4 +- src/bin/psql/tab-complete.c | 2 +- src/include/access/transam.h | 4 ++ src/include/catalog/unused_oids | 9 +++++ 11 files changed, 120 insertions(+), 22 deletions(-) diff --git a/doc/src/sgml/ref/create_database.sgml b/doc/src/sgml/ref/create_database.sgml index 41cb4068ec..f22e28dc81 100644 --- a/doc/src/sgml/ref/create_database.sgml +++ b/doc/src/sgml/ref/create_database.sgml @@ -31,7 +31,8 @@ CREATE DATABASE name [ TABLESPACE [=] tablespace_name ] [ ALLOW_CONNECTIONS [=] allowconn ] [ CONNECTION LIMIT [=] connlimit ] - [ IS_TEMPLATE [=] istemplate ] ] + [ IS_TEMPLATE [=] istemplate ] + [ OID [=] oid ] ] @@ -203,6 +204,21 @@ CREATE DATABASE name + + + oid + + + The object identifier to be used for the new database. If this + parameter is not specified, the database will choose a suitable + OID automatically. This parameter is primarily intended for internal + use by pg_upgrade, and only + pg_upgrade can specify a value less + than 16384. + + + + diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index da8345561d..e950e4c458 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -115,7 +115,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) HeapTuple tuple; Datum new_record[Natts_pg_database]; bool new_record_nulls[Natts_pg_database]; - Oid dboid; + Oid dboid = InvalidOid; Oid datdba; ListCell *option; DefElem *dtablespacename = NULL; @@ -215,6 +215,30 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) errhint("Consider using tablespaces instead."), parser_errposition(pstate, defel->location))); } + else if (strcmp(defel->defname, "oid") == 0) + { + dboid = defGetInt32(defel); + + /* + * We don't normally permit new databases to be created with + * system-assigned OIDs. pg_upgrade tries to preserve database + * OIDs, so we can't allow any database to be created with an + * OID that might be in use in a freshly-initialized cluster + * created by some future version. We assume all such OIDs will + * be from the system-managed OID range. + * + * As an exception, however, we permit any OID to be assigned when + * allow_system_table_mods=on (so that initdb can assign system + * OIDs to template0 and postgres) or when performing a binary + * upgrade (so that pg_upgrade can preserve whatever OIDs it finds + * in the source cluster). + */ + if (dboid < FirstNormalObjectId && + !allowSystemTableMods && !IsBinaryUpgrade) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)), + errmsg("OIDs less than %u are reserved for system objects", FirstNormalObjectId)); + } else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -502,11 +526,34 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) */ pg_database_rel = table_open(DatabaseRelationId, RowExclusiveLock); - do + /* + * If database OID is configured, check if the OID is already in use or + * data directory already exists. + */ + if (OidIsValid(dboid)) { - dboid = GetNewOidWithIndex(pg_database_rel, DatabaseOidIndexId, - Anum_pg_database_oid); - } while (check_db_file_conflict(dboid)); + char *existing_dbname = get_database_name(dboid); + + if (existing_dbname != NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)), + errmsg("database OID %u is already in use by database \"%s\"", + dboid, existing_dbname)); + + if (check_db_file_conflict(dboid)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)), + errmsg("data directory with the specified OID %u already exists", dboid)); + } + else + { + /* Select an OID for the new database if is not explicitly configured. */ + do + { + dboid = GetNewOidWithIndex(pg_database_rel, DatabaseOidIndexId, + Anum_pg_database_oid); + } while (check_db_file_conflict(dboid)); + } /* * Insert a new tuple into pg_database. This establishes our ownership of diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 0a2dba7d18..d78e8e67b8 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -59,6 +59,7 @@ #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 */ @@ -1838,8 +1839,23 @@ static void make_template0(FILE *cmdfd) { const char *const *line; + + /* + * pg_upgrade tries to preserve database OIDs across upgrades. It's smart + * enough to drop and recreate a conflicting database with the same name, + * but if the same OID were used for one system-created database in the + * old cluster and a different system-created database in the new cluster, + * it would fail. To avoid that, assign a fixed OID to template0 rather + * than letting the server choose one. + * + * (Note that, while the user could have dropped and recreated these + * objects in the old cluster, the problem scenario only exists if the OID + * that is in use in the old cluster is also used in the new cluster - and + * the new cluster should be the result of a fresh initdb.) + */ static const char *const template0_setup[] = { - "CREATE DATABASE template0 IS_TEMPLATE = true ALLOW_CONNECTIONS = false;\n\n", + "CREATE DATABASE template0 IS_TEMPLATE = true ALLOW_CONNECTIONS = false OID = " + CppAsString2(Template0ObjectId) ";\n\n", /* * Explicitly revoke public create-schema and create-temp-table @@ -1869,8 +1885,10 @@ static void make_postgres(FILE *cmdfd) { const char *const *line; + + /* Assign a fixed OID to postgres, for the same reasons as template0 */ static const char *const postgres_setup[] = { - "CREATE DATABASE postgres;\n\n", + "CREATE DATABASE postgres OID = " CppAsString2(PostgresObjectId) ";\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 f1e8b0b5c2..e3ddf19959 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -2838,8 +2838,16 @@ dumpDatabase(Archive *fout) * are left to the DATABASE PROPERTIES entry, so that they can be applied * after reconnecting to the target DB. */ - appendPQExpBuffer(creaQry, "CREATE DATABASE %s WITH TEMPLATE = template0", - qdatname); + if (dopt->binary_upgrade) + { + appendPQExpBuffer(creaQry, "CREATE DATABASE %s WITH TEMPLATE = template0 OID = %u", + qdatname, dbCatId.oid); + } + else + { + appendPQExpBuffer(creaQry, "CREATE DATABASE %s WITH TEMPLATE = template0", + qdatname); + } if (strlen(encoding) > 0) { appendPQExpBufferStr(creaQry, " ENCODING = "); diff --git a/src/bin/pg_upgrade/IMPLEMENTATION b/src/bin/pg_upgrade/IMPLEMENTATION index 69fcd70a7c..229399a45a 100644 --- a/src/bin/pg_upgrade/IMPLEMENTATION +++ b/src/bin/pg_upgrade/IMPLEMENTATION @@ -86,7 +86,7 @@ by pg_dumpall --- this script effectively creates the complete user-defined metadata from the old cluster to the new cluster. It preserves the relfilenode numbers so TOAST and other references to relfilenodes in user data is preserved. (See binary-upgrade usage -in pg_dump). +in pg_dump). We choose to preserve tablespace and database OIDs as well. Finally, pg_upgrade links or copies each user-defined table and its supporting indexes and toast tables from the old cluster to the new diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c index f7fa0820d6..69ef23119f 100644 --- a/src/bin/pg_upgrade/info.c +++ b/src/bin/pg_upgrade/info.c @@ -190,10 +190,8 @@ create_rel_filename_map(const char *old_data, const char *new_data, map->new_tablespace_suffix = new_cluster.tablespace_suffix; } - map->old_db_oid = old_db->db_oid; - map->new_db_oid = new_db->db_oid; - - /* relfilenode is preserved across old and new cluster */ + /* DB oid and relfilenodes are preserved between old and new cluster */ + map->db_oid = old_db->db_oid; map->relfilenode = old_rel->relfilenode; /* used only for logging and error reporting, old/new are identical */ @@ -324,8 +322,7 @@ get_db_infos(ClusterInfo *cluster) " LEFT OUTER JOIN pg_catalog.pg_tablespace t " " ON d.dattablespace = t.oid " "WHERE d.datallowconn = true " - /* we don't preserve pg_database.oid so we sort by name */ - "ORDER BY 2"); + "ORDER BY 1"); res = executeQueryOrDie(conn, "%s", query); diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index da6770d0f8..1db8e3f0fb 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -145,8 +145,7 @@ typedef struct const char *new_tablespace; const char *old_tablespace_suffix; const char *new_tablespace_suffix; - Oid old_db_oid; - Oid new_db_oid; + Oid db_oid; Oid relfilenode; /* the rest are used only for logging and error reporting */ char *nspname; /* namespaces */ diff --git a/src/bin/pg_upgrade/relfilenode.c b/src/bin/pg_upgrade/relfilenode.c index 6e0253cc3e..2f4deb3416 100644 --- a/src/bin/pg_upgrade/relfilenode.c +++ b/src/bin/pg_upgrade/relfilenode.c @@ -193,14 +193,14 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro snprintf(old_file, sizeof(old_file), "%s%s/%u/%u%s%s", map->old_tablespace, map->old_tablespace_suffix, - map->old_db_oid, + map->db_oid, map->relfilenode, type_suffix, extent_suffix); snprintf(new_file, sizeof(new_file), "%s%s/%u/%u%s%s", map->new_tablespace, map->new_tablespace_suffix, - map->new_db_oid, + map->db_oid, map->relfilenode, type_suffix, extent_suffix); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 6bd33a06cb..502b5c5751 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2633,7 +2633,7 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("OWNER", "TEMPLATE", "ENCODING", "TABLESPACE", "IS_TEMPLATE", "ALLOW_CONNECTIONS", "CONNECTION LIMIT", - "LC_COLLATE", "LC_CTYPE", "LOCALE"); + "LC_COLLATE", "LC_CTYPE", "LOCALE", "OID"); else if (Matches("CREATE", "DATABASE", MatchAny, "TEMPLATE")) COMPLETE_WITH_QUERY(Query_for_list_of_template_databases); diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 338dfca5a0..9a2816de51 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -196,6 +196,10 @@ 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/unused_oids b/src/include/catalog/unused_oids index e55bc6fa3c..61d41e7561 100755 --- a/src/include/catalog/unused_oids +++ b/src/include/catalog/unused_oids @@ -32,6 +32,15 @@ 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');