Skip to content

Commit 5a07966

Browse files
tvondratvondra
committed
Fix row filters with multiple publications
When publishing changes through a artition root, we should use the row filter for the top-most ancestor. The relation may be added to multiple publications, using different ancestors, and 52e4f0c handled this incorrectly. With c91f71b we find the correct top-most ancestor, but the code tried to fetch the row filter from all publications, including those using a different ancestor etc. No row filter can be found for such publications, which was treated as replicating all rows. Similarly to c91f71b, this seems to be a rare issue in practice. It requires multiple publications including the same partitioned relation, through different ancestors. Fixed by only passing publications containing the top-most ancestor to pgoutput_row_filter_init(), so that treating a missing row filter as replicating all rows is correct. Report and fix by me, test case by Hou zj. Reviews and improvements by Amit Kapila. Author: Tomas Vondra, Hou zj, Amit Kapila Reviewed-by: Amit Kapila, Hou zj Discussion: https://postgr.es/m/d26d24dd-2fab-3c48-0162-2b7f84a9c893%40enterprisedb.com
1 parent a9b7e92 commit 5a07966

File tree

2 files changed

+67
-6
lines changed

2 files changed

+67
-6
lines changed

‎src/backend/replication/pgoutput/pgoutput.c

+21-5
Original file line numberDiff line numberDiff line change
@@ -1890,8 +1890,6 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
18901890
entry->pubactions.pubdelete |= pub->pubactions.pubdelete;
18911891
entry->pubactions.pubtruncate |= pub->pubactions.pubtruncate;
18921892

1893-
rel_publications = lappend(rel_publications, pub);
1894-
18951893
/*
18961894
* We want to publish the changes as the top-most ancestor
18971895
* across all publications. So we need to check if the
@@ -1902,9 +1900,27 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
19021900
if (publish_ancestor_level > ancestor_level)
19031901
continue;
19041902

1905-
/* The new value is an ancestor, so let's keep it. */
1906-
publish_as_relid = pub_relid;
1907-
publish_ancestor_level = ancestor_level;
1903+
/*
1904+
* If we found an ancestor higher up in the tree, discard
1905+
* the list of publications through which we replicate it,
1906+
* and use the new ancestor.
1907+
*/
1908+
if (publish_ancestor_level < ancestor_level)
1909+
{
1910+
publish_as_relid = pub_relid;
1911+
publish_ancestor_level = ancestor_level;
1912+
1913+
/* reset the publication list for this relation */
1914+
rel_publications = NIL;
1915+
}
1916+
else
1917+
{
1918+
/* Same ancestor level, has to be the same OID. */
1919+
Assert(publish_as_relid == pub_relid);
1920+
}
1921+
1922+
/* Track publications for this ancestor. */
1923+
rel_publications = lappend(rel_publications, pub);
19081924
}
19091925
}
19101926

‎src/test/subscription/t/028_row_filter.pl

+46-1
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,11 @@
237237
$node_publisher->safe_psql('postgres',
238238
"CREATE TABLE tab_rowfilter_child (b text) INHERITS (tab_rowfilter_inherited)"
239239
);
240+
$node_publisher->safe_psql('postgres',
241+
"CREATE TABLE tab_rowfilter_viaroot_part (a int) PARTITION BY RANGE (a)");
242+
$node_publisher->safe_psql('postgres',
243+
"CREATE TABLE tab_rowfilter_viaroot_part_1 PARTITION OF tab_rowfilter_viaroot_part FOR VALUES FROM (1) TO (20)"
244+
);
240245

241246
# setup structure on subscriber
242247
$node_subscriber->safe_psql('postgres',
@@ -283,6 +288,11 @@
283288
$node_subscriber->safe_psql('postgres',
284289
"CREATE TABLE tab_rowfilter_child (b text) INHERITS (tab_rowfilter_inherited)"
285290
);
291+
$node_subscriber->safe_psql('postgres',
292+
"CREATE TABLE tab_rowfilter_viaroot_part (a int)");
293+
$node_subscriber->safe_psql('postgres',
294+
"CREATE TABLE tab_rowfilter_viaroot_part_1 (a int)"
295+
);
286296

287297
# setup logical replication
288298
$node_publisher->safe_psql('postgres',
@@ -330,6 +340,15 @@
330340
"CREATE PUBLICATION tap_pub_inherits FOR TABLE tab_rowfilter_inherited WHERE (a > 15)"
331341
);
332342

343+
# two publications, each publishing the partition through a different ancestor, with
344+
# different row filters
345+
$node_publisher->safe_psql('postgres',
346+
"CREATE PUBLICATION tap_pub_viaroot_1 FOR TABLE tab_rowfilter_viaroot_part WHERE (a > 15) WITH (publish_via_partition_root)"
347+
);
348+
$node_publisher->safe_psql('postgres',
349+
"CREATE PUBLICATION tap_pub_viaroot_2 FOR TABLE tab_rowfilter_viaroot_part_1 WHERE (a < 15) WITH (publish_via_partition_root)"
350+
);
351+
333352
#
334353
# The following INSERTs are executed before the CREATE SUBSCRIPTION, so these
335354
# SQL commands are for testing the initial data copy using logical replication.
@@ -376,7 +395,7 @@
376395
);
377396

378397
$node_subscriber->safe_psql('postgres',
379-
"CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_1, tap_pub_2, tap_pub_3, tap_pub_4a, tap_pub_4b, tap_pub_5a, tap_pub_5b, tap_pub_toast, tap_pub_inherits"
398+
"CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_1, tap_pub_2, tap_pub_3, tap_pub_4a, tap_pub_4b, tap_pub_5a, tap_pub_5b, tap_pub_toast, tap_pub_inherits, tap_pub_viaroot_2, tap_pub_viaroot_1"
380399
);
381400

382401
$node_publisher->wait_for_catchup($appname);
@@ -534,6 +553,8 @@
534553
"INSERT INTO tab_rowfilter_inherited (a) VALUES (14), (16)");
535554
$node_publisher->safe_psql('postgres',
536555
"INSERT INTO tab_rowfilter_child (a, b) VALUES (13, '13'), (17, '17')");
556+
$node_publisher->safe_psql('postgres',
557+
"INSERT INTO tab_rowfilter_viaroot_part (a) VALUES (14), (15), (16)");
537558

538559
$node_publisher->wait_for_catchup($appname);
539560

@@ -688,6 +709,30 @@
688709
"SELECT a = repeat('1234567890', 200), b FROM tab_rowfilter_toast");
689710
is($result, qq(t|1), 'check replicated rows to tab_rowfilter_toast');
690711

712+
# Check expected replicated rows for tab_rowfilter_viaroot_part and
713+
# tab_rowfilter_viaroot_part_1. We should replicate only rows matching
714+
# the row filter for the top-level ancestor:
715+
#
716+
# tab_rowfilter_viaroot_part filter is: (a > 15)
717+
# - INSERT (14) NO, 14 < 15
718+
# - INSERT (15) NO, 15 = 15
719+
# - INSERT (16) YES, 16 > 15
720+
$result =
721+
$node_subscriber->safe_psql('postgres',
722+
"SELECT a FROM tab_rowfilter_viaroot_part");
723+
is( $result, qq(16),
724+
'check replicated rows to tab_rowfilter_viaroot_part'
725+
);
726+
727+
# Check there is no data in tab_rowfilter_viaroot_part_1 because rows are
728+
# replicated via the top most parent table tab_rowfilter_viaroot_part
729+
$result =
730+
$node_subscriber->safe_psql('postgres',
731+
"SELECT a FROM tab_rowfilter_viaroot_part_1");
732+
is( $result, qq(),
733+
'check replicated rows to tab_rowfilter_viaroot_part_1'
734+
);
735+
691736
# Testcase end: FOR TABLE with row filter publications
692737
# ======================================================
693738

0 commit comments

Comments
 (0)