Hi,

Recently we have supported upgrade of subscriptions,but currently
subscription OIDs can be changed when a cluster is upgraded using
pg_upgrade. It will be better to preserve them as it will be easier to
compare subscription related objects in pg_subscription and
pg_subscription_rel in the old and new clusters.

Attached patch has the changes for the same.

Regards,
Vignesh
From c76b8b6cbdc7b1f29c0c73290651673cf06fd137 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Sun, 25 Feb 2024 07:22:27 +0530
Subject: [PATCH v1] Preserve subscription OIDs during pg_upgrade.

Currently subscription OIDs can be changed when a cluster is upgraded
using pg_upgrade. It will be better to preserve them as it will be
easier to match subscription related objects in pg_subscription and
pg_subscription_rel in the old and new clusters.

Bump catalog version.
---
 src/backend/commands/subscriptioncmds.c       | 25 +++++++++++++++++--
 src/backend/utils/adt/pg_upgrade_support.c    | 10 ++++++++
 src/bin/pg_dump/pg_dump.c                     |  8 ++++++
 src/bin/pg_upgrade/pg_upgrade.c               |  3 +++
 src/bin/pg_upgrade/t/004_subscription.pl      |  7 ++++++
 src/include/catalog/binary_upgrade.h          |  1 +
 src/include/catalog/pg_proc.dat               |  4 +++
 .../expected/spgist_name_ops.out              |  6 +++--
 8 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index a05d69922d..a12e680321 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -76,6 +76,12 @@
 /* check if the 'val' has 'bits' set */
 #define IsSet(val, bits)  (((val) & (bits)) == (bits))
 
+/*
+ * This will be set by the pg_upgrade_support function --
+ * binary_upgrade_set_next_pg_subscription_oid().
+ */
+Oid			binary_upgrade_next_pg_subscription_oid = InvalidOid;
+
 /*
  * Structure to hold a bitmap representing the user-provided CREATE/ALTER
  * SUBSCRIPTION command options and the parsed/default values of each of them.
@@ -699,8 +705,23 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
 	memset(values, 0, sizeof(values));
 	memset(nulls, false, sizeof(nulls));
 
-	subid = GetNewOidWithIndex(rel, SubscriptionObjectIndexId,
-							   Anum_pg_subscription_oid);
+	/* Use binary-upgrade override for pg_subscription.oid? */
+	if (IsBinaryUpgrade)
+	{
+		if (!OidIsValid(binary_upgrade_next_pg_subscription_oid))
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("pg_subscription OID value not set when in binary upgrade mode")));
+
+		subid = binary_upgrade_next_pg_subscription_oid;
+		binary_upgrade_next_pg_subscription_oid = InvalidOid;
+	}
+	else
+	{
+		subid = GetNewOidWithIndex(rel, SubscriptionObjectIndexId,
+								   Anum_pg_subscription_oid);
+	}
+
 	values[Anum_pg_subscription_oid - 1] = ObjectIdGetDatum(subid);
 	values[Anum_pg_subscription_subdbid - 1] = ObjectIdGetDatum(MyDatabaseId);
 	values[Anum_pg_subscription_subskiplsn - 1] = LSNGetDatum(InvalidXLogRecPtr);
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index c54b08fe18..e78cd478b0 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -181,6 +181,16 @@ binary_upgrade_set_next_pg_authid_oid(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+Datum
+binary_upgrade_set_next_pg_subscription_oid(PG_FUNCTION_ARGS)
+{
+	Oid			subid = PG_GETARG_OID(0);
+
+	CHECK_IS_BINARY_UPGRADE;
+	binary_upgrade_next_pg_subscription_oid = subid;
+	PG_RETURN_VOID();
+}
+
 Datum
 binary_upgrade_create_empty_extension(PG_FUNCTION_ARGS)
 {
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2225a12718..c77cda4769 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4994,6 +4994,14 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
 	appendPQExpBuffer(delq, "DROP SUBSCRIPTION %s;\n",
 					  qsubname);
 
+	if (dopt->binary_upgrade)
+	{
+		appendPQExpBufferStr(query, "\n-- For binary upgrade, must preserve pg_subscription.oid\n");
+		appendPQExpBuffer(query,
+						  "SELECT pg_catalog.binary_upgrade_set_next_pg_subscription_oid('%u'::pg_catalog.oid);\n\n",
+						  subinfo->dobj.catId.oid);
+	}
+
 	appendPQExpBuffer(query, "CREATE SUBSCRIPTION %s CONNECTION ",
 					  qsubname);
 	appendStringLiteralAH(query, subinfo->subconninfo, fout);
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 10c94a6c1f..186157e997 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -32,6 +32,9 @@
  *	We control all assignments of pg_authid.oid for historical reasons (the
  *	oids used to be stored in pg_largeobject_metadata, which is now copied via
  *	SQL commands), that might change at some point in the future.
+ *
+ *  We control assignment of pg_subscription.oid because we want the oid to
+ *  match between the old and new cluster.
  */
 
 
diff --git a/src/bin/pg_upgrade/t/004_subscription.pl b/src/bin/pg_upgrade/t/004_subscription.pl
index df5d6dffbc..f065b622c0 100644
--- a/src/bin/pg_upgrade/t/004_subscription.pl
+++ b/src/bin/pg_upgrade/t/004_subscription.pl
@@ -239,6 +239,9 @@ my $tab_upgraded1_oid = $old_sub->safe_psql('postgres',
 my $tab_upgraded2_oid = $old_sub->safe_psql('postgres',
 	"SELECT oid FROM pg_class WHERE relname = 'tab_upgraded2'");
 
+$sub_oid = $old_sub->safe_psql('postgres',
+	"SELECT oid FROM pg_subscription ORDER BY subname");
+
 $old_sub->stop;
 
 # Change configuration so that initial table sync sync does not get started
@@ -278,6 +281,10 @@ $publisher->safe_psql(
 
 $new_sub->start;
 
+# The subscription oid should be preserved
+$result = $new_sub->safe_psql('postgres', "SELECT oid FROM pg_subscription ORDER BY subname");
+is($result, qq($sub_oid), "subscription oid should have been preserved");
+
 # The subscription's running status and failover option should be preserved
 # in the upgraded instance. So regress_sub4 should still have subenabled and
 # subfailover set to true, while regress_sub5 should have both set to false.
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
index 1d5826195e..d53ab7d439 100644
--- a/src/include/catalog/binary_upgrade.h
+++ b/src/include/catalog/binary_upgrade.h
@@ -32,6 +32,7 @@ extern PGDLLIMPORT RelFileNumber binary_upgrade_next_toast_pg_class_relfilenumbe
 
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_enum_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_authid_oid;
+extern PGDLLIMPORT Oid binary_upgrade_next_pg_subscription_oid;
 
 extern PGDLLIMPORT bool binary_upgrade_record_init_privs;
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9c120fc2b7..7607fe67fd 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -11436,6 +11436,10 @@
   provolatile => 'v', proparallel => 'u', prorettype => 'void',
   proargtypes => 'text pg_lsn',
   prosrc => 'binary_upgrade_replorigin_advance' },
+{ oid => '8406', descr => 'for use by pg_upgrade',
+  proname => 'binary_upgrade_set_next_pg_subscription_oid', provolatile => 'v',
+  proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
+  prosrc => 'binary_upgrade_set_next_pg_subscription_oid' },
 
 # conversion functions
 { oid => '4302',
diff --git a/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out b/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out
index 1ee65ede24..39d43368c4 100644
--- a/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out
+++ b/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out
@@ -59,11 +59,12 @@ select * from t
  binary_upgrade_set_next_multirange_pg_type_oid       |  1 | binary_upgrade_set_next_multirange_pg_type_oid
  binary_upgrade_set_next_pg_authid_oid                |    | binary_upgrade_set_next_pg_authid_oid
  binary_upgrade_set_next_pg_enum_oid                  |    | binary_upgrade_set_next_pg_enum_oid
+ binary_upgrade_set_next_pg_subscription_oid          |    | binary_upgrade_set_next_pg_subscription_oid
  binary_upgrade_set_next_pg_tablespace_oid            |    | binary_upgrade_set_next_pg_tablespace_oid
  binary_upgrade_set_next_pg_type_oid                  |    | binary_upgrade_set_next_pg_type_oid
  binary_upgrade_set_next_toast_pg_class_oid           |  1 | binary_upgrade_set_next_toast_pg_class_oid
  binary_upgrade_set_next_toast_relfilenode            |    | binary_upgrade_set_next_toast_relfilenode
-(13 rows)
+(14 rows)
 
 -- Verify clean failure when INCLUDE'd columns result in overlength tuple
 -- The error message details are platform-dependent, so show only SQLSTATE
@@ -108,11 +109,12 @@ select * from t
  binary_upgrade_set_next_multirange_pg_type_oid       |  1 | binary_upgrade_set_next_multirange_pg_type_oid
  binary_upgrade_set_next_pg_authid_oid                |    | binary_upgrade_set_next_pg_authid_oid
  binary_upgrade_set_next_pg_enum_oid                  |    | binary_upgrade_set_next_pg_enum_oid
+ binary_upgrade_set_next_pg_subscription_oid          |    | binary_upgrade_set_next_pg_subscription_oid
  binary_upgrade_set_next_pg_tablespace_oid            |    | binary_upgrade_set_next_pg_tablespace_oid
  binary_upgrade_set_next_pg_type_oid                  |    | binary_upgrade_set_next_pg_type_oid
  binary_upgrade_set_next_toast_pg_class_oid           |  1 | binary_upgrade_set_next_toast_pg_class_oid
  binary_upgrade_set_next_toast_relfilenode            |    | binary_upgrade_set_next_toast_relfilenode
-(13 rows)
+(14 rows)
 
 \set VERBOSITY sqlstate
 insert into t values(repeat('xyzzy', 12), 42, repeat('xyzzy', 4000));
-- 
2.34.1

Reply via email to