database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-12-19 17:02:53 +03:00
Code Activity

Sort DO_SUBSCRIPTION_REL dump objects independent of OIDs.

Browse Source 
Commit 0decd5e89d 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
9a17be1e24 introduced DO_SUBSCRIPTION_REL.

Reported-by: vignesh C <vignesh21@gmail.com>
Author: vignesh C <vignesh21@gmail.com>
Discussion: https://postgr.es/m/CALDaNm2x3rd7C0_HjUpJFbxpAqXgm=QtoKfkEWDVA8h+JFpa_w@mail.gmail.com
Backpatch-through: 17
This commit is contained in:
Noah Misch
2025-12-18 10:23:47 -08:00
parent d77a5f9817
commit 573e679a26
3 changed files with 33 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/bin/pg_dump/pg_dump.c
View File

@@ -5232,7 +5232,9 @@ getSubscriptionTables(Archive *fout)
subrinfo[i].dobj.catId.tableoid = relid; subrinfo[i].dobj.catId.tableoid = relid;
subrinfo[i].dobj.catId.oid = cur_srsubid; subrinfo[i].dobj.catId.oid = cur_srsubid;
AssignDumpId(&subrinfo[i].dobj); 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].tblinfo = tblinfo;
subrinfo[i].srsubstate = PQgetvalue(res, i, i_srsubstate)[0]; subrinfo[i].srsubstate = PQgetvalue(res, i, i_srsubstate)[0];
if (PQgetisnull(res, i, i_srsublsn)) if (PQgetisnull(res, i, i_srsublsn))
@@ -5240,8 +5242,6 @@ getSubscriptionTables(Archive *fout)
else else
subrinfo[i].srsublsn = pg_strdup(PQgetvalue(res, i, i_srsublsn)); subrinfo[i].srsublsn = pg_strdup(PQgetvalue(res, i, i_srsublsn));
subrinfo[i].subinfo = subinfo;
/* Decide whether we want to dump it */ /* Decide whether we want to dump it */
selectDumpableObject(&(subrinfo[i].dobj), fout); selectDumpableObject(&(subrinfo[i].dobj), fout);
} }
@@ -5269,7 +5269,7 @@ dumpSubscriptionTable(Archive *fout, const SubRelInfo *subrinfo)
Assert(fout->dopt->binary_upgrade && fout->remoteVersion >= 170000); 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(); query = createPQExpBuffer();
@@ -5284,7 +5284,7 @@ dumpSubscriptionTable(Archive *fout, const SubRelInfo *subrinfo)
"\n-- For binary upgrade, must preserve the subscriber table.\n"); "\n-- For binary upgrade, must preserve the subscriber table.\n");
appendPQExpBufferStr(query, appendPQExpBufferStr(query,
"SELECT pg_catalog.binary_upgrade_add_sub_rel_state("); "SELECT pg_catalog.binary_upgrade_add_sub_rel_state(");
appendStringLiteralAH(query, subrinfo->dobj.name, fout); appendStringLiteralAH(query, subinfo->dobj.name, fout);
appendPQExpBuffer(query, appendPQExpBuffer(query,
", %u, '%c'", ", %u, '%c'",
subrinfo->tblinfo->dobj.catId.oid, subrinfo->tblinfo->dobj.catId.oid,

11
src/bin/pg_dump/pg_dump_sort.c
View File

@@ -454,6 +454,17 @@ DOTypeNameCompare(const void *p1, const void *p2)
if (cmpval != 0) if (cmpval != 0)
return cmpval; 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 * Shouldn't get here except after catalog corruption, but if we do, sort

27
src/bin/pg_upgrade/t/004_subscription.pl
View File

@@ -200,23 +200,27 @@ $old_sub->safe_psql(
rmtree($new_sub->data_dir . "/pg_upgrade_output.d"); rmtree($new_sub->data_dir . "/pg_upgrade_output.d");
# Verify that the upgrade should be successful with tables in 'ready'/'init' # Verify that the upgrade should be successful with tables in 'ready'/'init'
# state along with retaining the replication origin's remote lsn, subscription's # state along with retaining the replication origin's remote lsn,
# running status, and failover option. # subscription's running status, and failover option. Use multiple tables to
# verify deterministic pg_dump ordering of subscription relations during
# --binary-upgrade.
$publisher->safe_psql( $publisher->safe_psql(
'postgres', qq[ 'postgres', qq[
CREATE TABLE tab_upgraded(id int);
CREATE TABLE tab_upgraded1(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( $old_sub->safe_psql(
'postgres', qq[ 'postgres', qq[
CREATE TABLE tab_upgraded(id int);
CREATE TABLE tab_upgraded1(id int); CREATE TABLE tab_upgraded1(id int);
CREATE SUBSCRIPTION regress_sub4 CONNECTION '$connstr' PUBLICATION regress_pub4 WITH (failover = true); CREATE SUBSCRIPTION regress_sub4 CONNECTION '$connstr' PUBLICATION regress_pub4 WITH (failover = 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 = 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) $old_sub->poll_query_until('postgres', $synced_query)
or die "Timed out while waiting for the table to reach ready state"; or die "Timed out while waiting for the table to reach ready state";
@@ -254,6 +258,8 @@ my $remote_lsn = $old_sub->safe_psql('postgres',
# Have the subscription in disabled state before upgrade # Have the subscription in disabled state before upgrade
$old_sub->safe_psql('postgres', "ALTER SUBSCRIPTION regress_sub5 DISABLE"); $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', my $tab_upgraded1_oid = $old_sub->safe_psql('postgres',
"SELECT oid FROM pg_class WHERE relname = 'tab_upgraded1'"); "SELECT oid FROM pg_class WHERE relname = 'tab_upgraded1'");
my $tab_upgraded2_oid = $old_sub->safe_psql('postgres', my $tab_upgraded2_oid = $old_sub->safe_psql('postgres',
@@ -268,9 +274,9 @@ $new_sub->append_conf('postgresql.conf',
# ------------------------------------------------------ # ------------------------------------------------------
# Check that pg_upgrade is successful when all tables are in ready or in # 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 # init state (tab_upgraded and tab_upgraded1 tables are in ready state and
# in init state) along with retaining the replication origin's remote lsn, # tab_upgraded2 table is in init state) along with retaining the replication
# subscription's running status, and failover option. # origin's remote lsn, subscription's running status, and failover option.
# ------------------------------------------------------ # ------------------------------------------------------
command_ok( command_ok(
[ [
@@ -317,9 +323,10 @@ regress_sub5|f|f),
# Subscription relations should be preserved # Subscription relations should be preserved
$result = $new_sub->safe_psql('postgres', $result = $new_sub->safe_psql('postgres',
"SELECT srrelid, srsubstate FROM pg_subscription_rel ORDER BY srrelid"); "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), $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 # The replication origin's remote_lsn should be preserved