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

diff --git a/doc/src/sgml/ref/create_database.sgml b/doc/src/sgml/ref/create_database.sgml
index 41cb4068ec2fcbcfd4ce4ddb030a2ac068e55685..f22e28dc81b8315867fe573c1ffdddaab56622e1 100644 (file)
--- a/doc/src/sgml/ref/create_database.sgml
+++ b/doc/src/sgml/ref/create_database.sgml
@@ -31,7 +31,8 @@ CREATE DATABASE <replaceable class="parameter">name</replaceable>
            [ TABLESPACE [=] <replaceable class="parameter">tablespace_name</replaceable> ]
            [ ALLOW_CONNECTIONS [=] <replaceable class="parameter">allowconn</replaceable> ]
            [ CONNECTION LIMIT [=] <replaceable class="parameter">connlimit</replaceable> ]
-           [ IS_TEMPLATE [=] <replaceable class="parameter">istemplate</replaceable> ] ]
+           [ IS_TEMPLATE [=] <replaceable class="parameter">istemplate</replaceable> ]
+           [ OID [=] <replaceable class="parameter">oid</replaceable> ] ]
 </synopsis>
  </refsynopsisdiv>
 
@@ -203,6 +204,21 @@ CREATE DATABASE <replaceable class="parameter">name</replaceable>
         </para>
        </listitem>
       </varlistentry>
+
+      <varlistentry>
+       <term><replaceable class="parameter">oid</replaceable></term>
+       <listitem>
+        <para>
+         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 <application>pg_upgrade</application>, and only
+         <application>pg_upgrade</application> can specify a value less
+         than 16384.
+        </para>
+       </listitem>
+      </varlistentry>
+
     </variablelist>
 
   <para>

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index da8345561d8f6536b187f8c8ebfd11cc12c1b9d9..e950e4c458a1543532c4b02bea5867ec3afca814 100644 (file)
--- 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 0a2dba7d18e6d8a6c6846ce91367b4baee3301f5..d78e8e67b8d2f95f661db8a2009d66598c252af6 100644 (file)
--- 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 f1e8b0b5c2cca2639dac67e897222ffe24a8c961..e3ddf19959af56a80c57a87cb564a5ff8d8b88c0 100644 (file)
--- 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 69fcd70a7c526d9f27b43a49f71e9b591a6b889f..229399a45ae0587c345e618dd5e2e9507d20656c 100644 (file)
--- 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 f7fa0820d69ad5c84254cf6ce3f6c479938c7819..69ef23119f5b427994a8f172399477f248d3bb3e 100644 (file)
--- 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 da6770d0f834b693ce7bed0dc63e81ad5d6eb831..1db8e3f0fbe023a0ed29ee3540a274246f710dae 100644 (file)
--- 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 6e0253cc3e61cf78ba4444139b308f060230f7db..2f4deb3416320c31636558e2ee40b010e302a1c3 100644 (file)
--- 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 6bd33a06cb19c808d1335c7a3c3f234bb69e2a6f..502b5c575159391dedb20bc47ad3eaddecd25385 100644 (file)
--- 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 338dfca5a0bb9772094db4a24a236705373f0c17..9a2816de51529f1c51b367a1e60722ee1418a846 100644 (file)
--- 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 e55bc6fa3c31d4da18526152ba059c0476fa09b6..61d41e7561865df23b761e3fd52235f9f5af26d7 100755 (executable)
--- 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');