On Wed, Dec 17, 2025 at 10:11:58AM +0530, vignesh C wrote: > The attached v3 version patch has the changes for the same.
The "tag" variable needed a change to compensate for the subrinfo->dobj.name change. I plan to push the attached version.
commit d15c9e5 (HEAD, zzy_test-commit-master) Author: Noah Misch <[email protected]> AuthorDate: Wed Dec 17 11:18:00 2025 -0800 Commit: Noah Misch <[email protected]> CommitDate: Wed Dec 17 11:18:00 2025 -0800 Sort DO_SUBSCRIPTION_REL dump objects independent of OIDs. Commit 0decd5e89db9f5edb9b27351082f0d74aae7a9b6 missed DO_SUBSCRIPTION_REL, leading to assertion failures. In the unlikely use case of diffing "pg_dump --binary-upgrade" output, spurious diffs were possible. As part of fixing that, align the DumpableObject naming and sort order with DO_PUBLICATION_REL. The overall effect of this commit is to change sort order from (subname, srsubid) to (rel, subname). Since DO_SUBSCRIPTION_REL is only for --binary-upgrade, accept that larger-than-usual dump order change. Back-patch to v17, where commit 9a17be1e244a45a77de25ed2ada246fd34e4557d introduced DO_SUBSCRIPTION_REL. Reported-by: vignesh C <[email protected]> Author: vignesh C <[email protected]> Discussion: https://postgr.es/m/CALDaNm2x3rd7C0_HjUpJFbxpAqXgm=qtokfkewdva8h+jfp...@mail.gmail.com Backpatch-through: 17 --- src/bin/pg_dump/pg_dump.c | 10 +++++----- src/bin/pg_dump/pg_dump_sort.c | 11 +++++++++++ src/bin/pg_upgrade/t/004_subscription.pl | 26 ++++++++++++++++---------- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 24ad201..27f6be3 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); } @@ -5423,7 +5423,7 @@ dumpSubscriptionTable(Archive *fout, const SubRelInfo *subrinfo) Assert(fout->dopt->binary_upgrade && fout->remoteVersion >= 170000); - tag = psprintf("%s %s", subinfo->dobj.name, subrinfo->dobj.name); + tag = psprintf("%s %s", subinfo->dobj.name, subrinfo->tblinfo->dobj.name); query = createPQExpBuffer(); @@ -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 164c76e..e2a4df4 100644 --- a/src/bin/pg_dump/pg_dump_sort.c +++ b/src/bin/pg_dump/pg_dump_sort.c @@ -454,6 +454,17 @@ 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 77387be..fdb9806 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
