Skip to content

Commit d49936f

Browse files
nmischnmisch
committed
Sort DO_SUBSCRIPTION_REL dump objects independent of OIDs.
Commit 0decd5e 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 9a17be1 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
1 parent 951b60f commit d49936f

File tree

3 files changed

+32
-15
lines changed

3 files changed

+32
-15
lines changed

‎src/bin/pg_dump/pg_dump.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5386,16 +5386,16 @@ getSubscriptionRelations(Archive *fout)
53865386
subrinfo[i].dobj.catId.tableoid = relid;
53875387
subrinfo[i].dobj.catId.oid = cur_srsubid;
53885388
AssignDumpId(&subrinfo[i].dobj);
5389-
subrinfo[i].dobj.name = pg_strdup(subinfo->dobj.name);
5389+
subrinfo[i].dobj.namespace = tblinfo->dobj.namespace;
5390+
subrinfo[i].dobj.name = tblinfo->dobj.name;
5391+
subrinfo[i].subinfo = subinfo;
53905392
subrinfo[i].tblinfo = tblinfo;
53915393
subrinfo[i].srsubstate = PQgetvalue(res, i, i_srsubstate)[0];
53925394
if (PQgetisnull(res, i, i_srsublsn))
53935395
subrinfo[i].srsublsn = NULL;
53945396
else
53955397
subrinfo[i].srsublsn = pg_strdup(PQgetvalue(res, i, i_srsublsn));
53965398

5397-
subrinfo[i].subinfo = subinfo;
5398-
53995399
/* Decide whether we want to dump it */
54005400
selectDumpableObject(&(subrinfo[i].dobj), fout);
54015401
}
@@ -5423,7 +5423,7 @@ dumpSubscriptionTable(Archive *fout, const SubRelInfo *subrinfo)
54235423

54245424
Assert(fout->dopt->binary_upgrade && fout->remoteVersion >= 170000);
54255425

5426-
tag = psprintf("%s %s", subinfo->dobj.name, subrinfo->dobj.name);
5426+
tag = psprintf("%s %s", subinfo->dobj.name, subrinfo->tblinfo->dobj.name);
54275427

54285428
query = createPQExpBuffer();
54295429

@@ -5438,7 +5438,7 @@ dumpSubscriptionTable(Archive *fout, const SubRelInfo *subrinfo)
54385438
"\n-- For binary upgrade, must preserve the subscriber table.\n");
54395439
appendPQExpBufferStr(query,
54405440
"SELECT pg_catalog.binary_upgrade_add_sub_rel_state(");
5441-
appendStringLiteralAH(query, subrinfo->dobj.name, fout);
5441+
appendStringLiteralAH(query, subinfo->dobj.name, fout);
54425442
appendPQExpBuffer(query,
54435443
", %u, '%c'",
54445444
subrinfo->tblinfo->dobj.catId.oid,

‎src/bin/pg_dump/pg_dump_sort.c‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,17 @@ DOTypeNameCompare(const void *p1, const void *p2)
454454
if (cmpval != 0)
455455
return cmpval;
456456
}
457+
else if (obj1->objType == DO_SUBSCRIPTION_REL)
458+
{
459+
SubRelInfo *srobj1 = *(SubRelInfo *const *) p1;
460+
SubRelInfo *srobj2 = *(SubRelInfo *const *) p2;
461+
462+
/* Sort by subscription name, since (namespace, name) match the rel */
463+
cmpval = strcmp(srobj1->subinfo->dobj.name,
464+
srobj2->subinfo->dobj.name);
465+
if (cmpval != 0)
466+
return cmpval;
467+
}
457468

458469
/*
459470
* Shouldn't get here except after catalog corruption, but if we do, sort

‎src/bin/pg_upgrade/t/004_subscription.pl‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -250,22 +250,25 @@
250250
# Verify that the upgrade should be successful with tables in 'ready'/'init'
251251
# state along with retaining the replication origin's remote lsn,
252252
# subscription's running status, failover option, and retain_dead_tuples
253-
# option.
253+
# option. Use multiple tables to verify deterministic pg_dump ordering
254+
# of subscription relations during --binary-upgrade.
254255
$publisher->safe_psql(
255256
'postgres', qq[
257+
CREATE TABLE tab_upgraded(id int);
256258
CREATE TABLE tab_upgraded1(id int);
257-
CREATE PUBLICATION regress_pub4 FOR TABLE tab_upgraded1;
259+
CREATE PUBLICATION regress_pub4 FOR TABLE tab_upgraded, tab_upgraded1;
258260
]);
259261

260262
$old_sub->safe_psql(
261263
'postgres', qq[
264+
CREATE TABLE tab_upgraded(id int);
262265
CREATE TABLE tab_upgraded1(id int);
263266
CREATE SUBSCRIPTION regress_sub4 CONNECTION '$connstr' PUBLICATION regress_pub4 WITH (failover = true, retain_dead_tuples = true);
264267
]);
265268

266-
# Wait till the table tab_upgraded1 reaches 'ready' state
269+
# Wait till the tables tab_upgraded and tab_upgraded1 reach 'ready' state
267270
my $synced_query =
268-
"SELECT count(1) = 1 FROM pg_subscription_rel WHERE srsubstate = 'r'";
271+
"SELECT count(1) = 2 FROM pg_subscription_rel WHERE srsubstate = 'r'";
269272
$old_sub->poll_query_until('postgres', $synced_query)
270273
or die "Timed out while waiting for the table to reach ready state";
271274

@@ -303,6 +306,8 @@
303306
# Have the subscription in disabled state before upgrade
304307
$old_sub->safe_psql('postgres', "ALTER SUBSCRIPTION regress_sub5 DISABLE");
305308

309+
my $tab_upgraded_oid = $old_sub->safe_psql('postgres',
310+
"SELECT oid FROM pg_class WHERE relname = 'tab_upgraded'");
306311
my $tab_upgraded1_oid = $old_sub->safe_psql('postgres',
307312
"SELECT oid FROM pg_class WHERE relname = 'tab_upgraded1'");
308313
my $tab_upgraded2_oid = $old_sub->safe_psql('postgres',
@@ -317,10 +322,10 @@
317322

318323
# ------------------------------------------------------
319324
# Check that pg_upgrade is successful when all tables are in ready or in
320-
# init state (tab_upgraded1 table is in ready state and tab_upgraded2 table is
321-
# in init state) along with retaining the replication origin's remote lsn,
322-
# subscription's running status, failover option, and retain_dead_tuples
323-
# option.
325+
# init state (tab_upgraded and tab_upgraded1 tables are in ready state and
326+
# tab_upgraded2 table is in init state) along with retaining the replication
327+
# origin's remote lsn, subscription's running status, failover option, and
328+
# retain_dead_tuples option.
324329
# ------------------------------------------------------
325330
command_ok(
326331
[
@@ -369,9 +374,10 @@
369374
# Subscription relations should be preserved
370375
$result = $new_sub->safe_psql('postgres',
371376
"SELECT srrelid, srsubstate FROM pg_subscription_rel ORDER BY srrelid");
372-
is( $result, qq($tab_upgraded1_oid|r
377+
is( $result, qq($tab_upgraded_oid|r
378+
$tab_upgraded1_oid|r
373379
$tab_upgraded2_oid|i),
374-
"there should be 2 rows in pg_subscription_rel(representing tab_upgraded1 and tab_upgraded2)"
380+
"there should be 3 rows in pg_subscription_rel(representing tab_upgraded, tab_upgraded1 and tab_upgraded2)"
375381
);
376382

377383
# The replication origin's remote_lsn should be preserved

0 commit comments

Comments
 (0)