From 5b3c16301be655f8205f3f5308696aa785375493 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Mon, 15 Dec 2025 23:27:40 +0530
Subject: [PATCH v3] Fix pg_dump crash for DO_SUBSCRIPTION_REL sorting
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

pg_dump did not fully order DO_SUBSCRIPTION_REL objects. When multiple
subscription–relation entries belonged to the same subscription, the comparison
fell through to the assertion path and crashed.

Fix this by extending the comparison to order such entries by the referenced
table's schema name and table name.
---
 src/bin/pg_dump/pg_dump.c                |  8 ++++----
 src/bin/pg_dump/pg_dump_sort.c           | 10 +++++++++
 src/bin/pg_upgrade/t/004_subscription.pl | 26 +++++++++++++++---------
 3 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 24ad201af2f..0f3f41c10b4 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5386,7 +5386,9 @@ getSubscriptionRelations(Archive *fout)
 		subrinfo[i].dobj.catId.tableoid = relid;
 		subrinfo[i].dobj.catId.oid = cur_srsubid;
 		AssignDumpId(&subrinfo[i].dobj);
-		subrinfo[i].dobj.name = pg_strdup(subinfo->dobj.name);
+		subrinfo[i].dobj.namespace = tblinfo->dobj.namespace;
+		subrinfo[i].dobj.name = tblinfo->dobj.name;
+		subrinfo[i].subinfo = subinfo;
 		subrinfo[i].tblinfo = tblinfo;
 		subrinfo[i].srsubstate = PQgetvalue(res, i, i_srsubstate)[0];
 		if (PQgetisnull(res, i, i_srsublsn))
@@ -5394,8 +5396,6 @@ getSubscriptionRelations(Archive *fout)
 		else
 			subrinfo[i].srsublsn = pg_strdup(PQgetvalue(res, i, i_srsublsn));
 
-		subrinfo[i].subinfo = subinfo;
-
 		/* Decide whether we want to dump it */
 		selectDumpableObject(&(subrinfo[i].dobj), fout);
 	}
@@ -5438,7 +5438,7 @@ dumpSubscriptionTable(Archive *fout, const SubRelInfo *subrinfo)
 							 "\n-- For binary upgrade, must preserve the subscriber table.\n");
 		appendPQExpBufferStr(query,
 							 "SELECT pg_catalog.binary_upgrade_add_sub_rel_state(");
-		appendStringLiteralAH(query, subrinfo->dobj.name, fout);
+		appendStringLiteralAH(query, subinfo->dobj.name, fout);
 		appendPQExpBuffer(query,
 						  ", %u, '%c'",
 						  subrinfo->tblinfo->dobj.catId.oid,
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 164c76e0864..661d933e8aa 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -454,6 +454,16 @@ DOTypeNameCompare(const void *p1, const void *p2)
 		if (cmpval != 0)
 			return cmpval;
 	}
+	else if (obj1->objType == DO_SUBSCRIPTION_REL)
+	{
+		SubRelInfo *srobj1 = *(SubRelInfo *const *) p1;
+		SubRelInfo *srobj2 = *(SubRelInfo *const *) p2;
+
+		/* Sort by subscription name, since (namespace, name) match the rel */
+		cmpval = strcmp(srobj1->subinfo->dobj.name, srobj2->subinfo->dobj.name);
+		if (cmpval != 0)
+			return cmpval;
+	}
 
 	/*
 	 * Shouldn't get here except after catalog corruption, but if we do, sort
diff --git a/src/bin/pg_upgrade/t/004_subscription.pl b/src/bin/pg_upgrade/t/004_subscription.pl
index 77387be0f9d..fdb9806713e 100644
--- a/src/bin/pg_upgrade/t/004_subscription.pl
+++ b/src/bin/pg_upgrade/t/004_subscription.pl
@@ -250,22 +250,25 @@ rmtree($new_sub->data_dir . "/pg_upgrade_output.d");
 # Verify that the upgrade should be successful with tables in 'ready'/'init'
 # state along with retaining the replication origin's remote lsn,
 # subscription's running status, failover option, and retain_dead_tuples
-# option.
+# option. Use multiple tables to verify deterministic pg_dump ordering
+# of subscription relations during --binary-upgrade.
 $publisher->safe_psql(
 	'postgres', qq[
+		CREATE TABLE tab_upgraded(id int);
 		CREATE TABLE tab_upgraded1(id int);
-		CREATE PUBLICATION regress_pub4 FOR TABLE tab_upgraded1;
+		CREATE PUBLICATION regress_pub4 FOR TABLE tab_upgraded, tab_upgraded1;
 ]);
 
 $old_sub->safe_psql(
 	'postgres', qq[
+		CREATE TABLE tab_upgraded(id int);
 		CREATE TABLE tab_upgraded1(id int);
 		CREATE SUBSCRIPTION regress_sub4 CONNECTION '$connstr' PUBLICATION regress_pub4 WITH (failover = true, retain_dead_tuples = true);
 ]);
 
-# Wait till the table tab_upgraded1 reaches 'ready' state
+# Wait till the tables tab_upgraded and tab_upgraded1 reach 'ready' state
 my $synced_query =
-  "SELECT count(1) = 1 FROM pg_subscription_rel WHERE srsubstate = 'r'";
+  "SELECT count(1) = 2 FROM pg_subscription_rel WHERE srsubstate = 'r'";
 $old_sub->poll_query_until('postgres', $synced_query)
   or die "Timed out while waiting for the table to reach ready state";
 
@@ -303,6 +306,8 @@ my $remote_lsn = $old_sub->safe_psql('postgres',
 # Have the subscription in disabled state before upgrade
 $old_sub->safe_psql('postgres', "ALTER SUBSCRIPTION regress_sub5 DISABLE");
 
+my $tab_upgraded_oid = $old_sub->safe_psql('postgres',
+	"SELECT oid FROM pg_class WHERE relname = 'tab_upgraded'");
 my $tab_upgraded1_oid = $old_sub->safe_psql('postgres',
 	"SELECT oid FROM pg_class WHERE relname = 'tab_upgraded1'");
 my $tab_upgraded2_oid = $old_sub->safe_psql('postgres',
@@ -317,10 +322,10 @@ $new_sub->append_conf('postgresql.conf',
 
 # ------------------------------------------------------
 # Check that pg_upgrade is successful when all tables are in ready or in
-# init state (tab_upgraded1 table is in ready state and tab_upgraded2 table is
-# in init state) along with retaining the replication origin's remote lsn,
-# subscription's running status, failover option, and retain_dead_tuples
-# option.
+# init state (tab_upgraded and tab_upgraded1 tables are in ready state and
+# tab_upgraded2 table is in init state) along with retaining the replication
+# origin's remote lsn, subscription's running status, failover option, and
+# retain_dead_tuples option.
 # ------------------------------------------------------
 command_ok(
 	[
@@ -369,9 +374,10 @@ regress_sub5|f|f|f),
 # Subscription relations should be preserved
 $result = $new_sub->safe_psql('postgres',
 	"SELECT srrelid, srsubstate FROM pg_subscription_rel ORDER BY srrelid");
-is( $result, qq($tab_upgraded1_oid|r
+is( $result, qq($tab_upgraded_oid|r
+$tab_upgraded1_oid|r
 $tab_upgraded2_oid|i),
-	"there should be 2 rows in pg_subscription_rel(representing tab_upgraded1 and tab_upgraded2)"
+	"there should be 3 rows in pg_subscription_rel(representing tab_upgraded, tab_upgraded1 and tab_upgraded2)"
 );
 
 # The replication origin's remote_lsn should be preserved
-- 
2.43.0

