rebased and updated

Robert thought that it might be reasonable for someone to initdb, and
then connect and make some modifications, and then pg_upgrade.
https://www.postgresql.org/message-id/CA%2BTgmoYwaXh_wRRa2CqL4XpM4r6YEbq1%2Bec%3D%2B8b7Dr7-T_tT%2BQ%40mail.gmail.com

But the DBs are dropped by pg_upgrade, so that seems to serve no
purpose, except for shared relations (and global objects?).  In the case
of shared relations, it seems unsafe (even though my test didn't cause
an immediate explosion).

So rather than continuing to allow arbitrary changes between initdb and
pg_upgrade, I propose to prohibit all changes.  I'd consider allowing an
"inclusive" list of allowable changes, if someone were to propose such a
thing - but since DBs are dropped, I'm not sure what it could include.
>From 7958ec4b13e9dc581f69df65933789274de499b4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 6 Jul 2022 08:55:11 -0500
Subject: [PATCH v2] wip: initdb should advance the OID counter to FirstNormal

Otherwise, a cluster started in single-user mode immediately after
initdb can create objects in the range of system OIDs, and pg_upgrade
might be able to be run twice (if the cluster has no relations/objects).
---
 src/backend/access/transam/varsup.c |  5 ++---
 src/bin/initdb/initdb.c             |  9 +++++++++
 src/bin/pg_upgrade/check.c          |  5 +++++
 src/bin/pg_upgrade/pg_upgrade.c     | 13 -------------
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c
index 849a7ce9d6d..d93974fcaa5 100644
--- a/src/backend/access/transam/varsup.c
+++ b/src/backend/access/transam/varsup.c
@@ -543,8 +543,7 @@ GetNewObjectId(void)
 	 *
 	 * During initdb, we start the OID generator at FirstGenbkiObjectId, so we
 	 * only wrap if before that point when in bootstrap or standalone mode.
-	 * The first time through this routine after normal postmaster start, the
-	 * counter will be forced up to FirstNormalObjectId.  This mechanism
+	 * This mechanism
 	 * leaves the OIDs between FirstGenbkiObjectId and FirstNormalObjectId
 	 * available for automatic assignment during initdb, while ensuring they
 	 * will never conflict with user-assigned OIDs.
@@ -553,7 +552,7 @@ GetNewObjectId(void)
 	{
 		if (IsPostmasterEnvironment)
 		{
-			/* wraparound, or first post-initdb assignment, in normal mode */
+			/* wraparound */
 			ShmemVariableCache->nextOid = FirstNormalObjectId;
 			ShmemVariableCache->oidCount = 0;
 		}
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index e00837ecacf..1aad2b335b8 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -61,6 +61,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 */
@@ -2735,6 +2736,14 @@ initialize_data_directory(void)
 
 	PG_CMD_CLOSE;
 
+	/* Set FirstNormal OID */
+	snprintf(cmd, sizeof(cmd),
+			 "\"%s/pg_resetwal\" -o %u %s",
+			 bin_path, FirstNormalObjectId, pg_data);
+
+	PG_CMD_OPEN;
+	PG_CMD_CLOSE;
+
 	check_ok();
 }
 
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index f4969bcdad7..f7ff9b5f65b 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -9,6 +9,7 @@
 
 #include "postgres_fe.h"
 
+#include "access/transam.h"
 #include "catalog/pg_authid_d.h"
 #include "catalog/pg_collation.h"
 #include "fe_utils/string_utils.h"
@@ -434,6 +435,10 @@ check_new_cluster_is_empty(void)
 {
 	int			dbnum;
 
+	if (new_cluster.controldata.chkpnt_nxtoid != FirstNormalObjectId)
+		pg_fatal("New cluster is not pristine: OIDs have been assigned since initdb (%u != %u)\n",
+			new_cluster.controldata.chkpnt_nxtoid, FirstNormalObjectId);
+
 	for (dbnum = 0; dbnum < new_cluster.dbarr.ndbs; dbnum++)
 	{
 		int			relnum;
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 115faa222e3..ea7d93d9cbe 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -172,19 +172,6 @@ main(int argc, char **argv)
 	transfer_all_new_tablespaces(&old_cluster.dbarr, &new_cluster.dbarr,
 								 old_cluster.pgdata, new_cluster.pgdata);
 
-	/*
-	 * Assuming OIDs are only used in system tables, there is no need to
-	 * restore the OID counter because we have not transferred any OIDs from
-	 * the old system, but we do it anyway just in case.  We do it late here
-	 * because there is no need to have the schema load use new oids.
-	 */
-	prep_status("Setting next OID for new cluster");
-	exec_prog(UTILITY_LOG_FILE, NULL, true, true,
-			  "\"%s/pg_resetwal\" -o %u \"%s\"",
-			  new_cluster.bindir, old_cluster.controldata.chkpnt_nxtoid,
-			  new_cluster.pgdata);
-	check_ok();
-
 	if (user_opts.do_sync)
 	{
 		prep_status("Sync data directory to disk");
-- 
2.17.1

Reply via email to