From 0d90b44de0bccd710c40f7dbcca93361ebcafafd Mon Sep 17 00:00:00 2001 From: Zhijie Hou Date: Mon, 12 Oct 2026 18:04:38 +0800 Subject: [PATCH v4] Fix logical replication with multirange WITHOUT OVERLAPS identity keys A GiST index on a multirange is lossy: it stores only the bounding range, so two multiranges like {[1,5)} and {[1,2),[3,5)} look the same to the index. RelationFindReplTupleByIndex() and RelationFindDeletedTupleInfoByIndex() assume that any match through the primary key or replica identity index is exact and skip the equality check, so an update or delete can be applied to the wrong row, and conflict detection can report update_deleted, blaming the transaction that deleted an unrelated row, instead of update_missing. Fix by rechecking the candidate whenever the scan sets xs_recheck. Only the index key columns are compared, since those are all the search slot carries. The comparison uses the scan key's equality operators and collations, i.e. the index's own notion of key equality, rather than the type's default equality operator and the column's attcollation. Reported-by: Andres Freund Author: Paul A. Jungwirth Author: Zhijie Hou Reviewed-by: Nisha Moond Discussion: https://postgr.es/m/kcyaok346iwocfdourf2oojgtz7ggxmh2ugis7qqhggs4qfzc2@cu6pkox4rfwp Discussion: https://postgr.es/m/e5nb5jbus2oa3pffmlo7pdvdckmchd54tqld4k3n6huyg5xxqn@7rounyvufjx6 Discussion: https://postgr.es/m/CA+renyWzTBC4GetPzJM1KqrejrmOWcGnn2NaA7tpee17HFxRPw@mail.gmail.com Backpatch-through: 18 --- src/backend/executor/execReplication.c | 71 +++++++++++++++++-- src/test/subscription/t/034_temporal.pl | 71 +++++++++++++++++++ src/test/subscription/t/035_conflicts.pl | 86 ++++++++++++++++++++++++ 3 files changed, 224 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 18da4fe08f6..8fb28f491db 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -40,6 +40,8 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2, TypeCacheEntry **eq, Bitmapset *columns); +static bool repl_index_key_equal(Relation idxrel, TupleTableSlot *slot1, + TupleTableSlot *slot2, ScanKey skey); /* * Setup a ScanKey for a search in the relation 'rel' for a tuple 'key' that @@ -179,7 +181,9 @@ should_refetch_tuple(TM_Result res, TM_FailureData *tmfd) * contents, and return true. Return false otherwise. * * 'skipduplicates' specifies whether the first matching tuple can be used - * without comparing it against 'searchslot'. If false, all matching tuples are + * without comparing it against 'searchslot', or at worst by comparing just + * the index key columns (when the index scan requires a recheck, as with a + * lossy GiST index for WITHOUT OVERLAPS). If false, all matching tuples are * compared against 'searchslot', which must contain a complete row. */ bool @@ -220,7 +224,11 @@ retry: { /* * Avoid expensive equality check if the index is primary key or - * replica identity index. + * replica identity index, unless the index scan indicates that a + * recheck is needed: a lossy index (e.g. a GiST index on a multirange + * column of a WITHOUT OVERLAPS key) cannot prove an exact match, so + * compare the key columns using the index's own equality semantics + * (see repl_index_key_equal). */ if (!skipduplicates) { @@ -230,6 +238,9 @@ retry: if (!tuples_equal(outslot, searchslot, eq, NULL)) continue; } + else if (scan->xs_recheck && + !repl_index_key_equal(idxrel, outslot, searchslot, skey)) + continue; ExecMaterializeSlot(outslot); @@ -358,6 +369,51 @@ tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2, return true; } +/* + * Compare the key columns of index 'idxrel' between the two slots, using the + * equality operators and collations of the scan key 'skey' built by + * build_replindex_scan_key(). + */ +static bool +repl_index_key_equal(Relation idxrel, TupleTableSlot *slot1, + TupleTableSlot *slot2, ScanKey skey) +{ + Assert(slot1->tts_tupleDescriptor->natts == + slot2->tts_tupleDescriptor->natts); + + for (int i = 0; i < idxrel->rd_index->indnkeyatts; i++) + { + AttrNumber keycol = idxrel->rd_index->indkey.values[i]; + Datum value1; + Datum value2; + bool isnull1; + bool isnull2; + + value1 = slot_getattr(slot1, keycol, &isnull1); + value2 = slot_getattr(slot2, keycol, &isnull2); + + /* + * If one value is NULL and other is not, then they are certainly not + * equal + */ + if (isnull1 != isnull2) + return false; + + /* + * If both are NULL, they can be considered equal. + */ + if (isnull1 || isnull2) + continue; + + if (!DatumGetBool(FunctionCall2Coll(&skey[i].sk_func, + skey[i].sk_collation, + value1, value2))) + return false; + } + + return true; +} + /* * Search the relation 'rel' for tuple using the sequential scan. * @@ -699,8 +755,12 @@ RelationFindDeletedTupleInfoByIndex(Relation rel, Oid idxoid, while (table_index_getnext_slot(scan, ForwardScanDirection, scanslot)) { /* - * Avoid expensive equality check if the index is primary key or - * replica identity index. + * Avoid expensive equality check if the index is a primary key or + * replica identity index, unless the index scan indicates that a + * recheck is needed: a lossy index (e.g. a GiST index on a multirange + * column of a WITHOUT OVERLAPS key) cannot prove an exact match, so + * compare the key columns using the index's own equality semantics + * (see repl_index_key_equal). */ if (!skipduplicates) { @@ -710,6 +770,9 @@ RelationFindDeletedTupleInfoByIndex(Relation rel, Oid idxoid, if (!tuples_equal(scanslot, searchslot, eq, NULL)) continue; } + else if (scan->xs_recheck && + !repl_index_key_equal(idxrel, scanslot, searchslot, skey)) + continue; update_most_recent_deletion_info(scanslot, oldestxmin, delete_xid, delete_time, delete_origin); diff --git a/src/test/subscription/t/034_temporal.pl b/src/test/subscription/t/034_temporal.pl index 66955e1b799..f8e662ef367 100644 --- a/src/test/subscription/t/034_temporal.pl +++ b/src/test/subscription/t/034_temporal.pl @@ -628,4 +628,75 @@ is( $result, qq{[1,2)|[2000-01-01,2010-01-01)|a drop_everything(); + +# ################################# +# Test with a lossy REPLICA IDENTITY index (multirange key) +# +# A temporal key over a multirange column is backed by a lossy GiST index: two +# distinct multiranges that share a bounding range are indistinguishable in the +# index, so an index probe can return candidate tuples that are not exact +# matches. The apply worker must recheck each candidate against the actual row +# values, otherwise it could apply the change to the wrong row. +# ################################# + +$node_publisher->safe_psql('postgres', + "CREATE TABLE temporal_mltrng (id int4multirange, valid_at daterange, a text, PRIMARY KEY (id, valid_at WITHOUT OVERLAPS))" +); +$node_subscriber->safe_psql('postgres', + "CREATE TABLE temporal_mltrng (id int4multirange, valid_at daterange, a text, PRIMARY KEY (id, valid_at WITHOUT OVERLAPS))" +); + +# Two different multiranges that share the bounding range [1,5), with the same +# valid_at. They do not conflict (their ids are not equal), but the GiST index +# cannot tell them apart without a recheck. +$node_publisher->safe_psql( + 'postgres', + "INSERT INTO temporal_mltrng (id, valid_at, a) + VALUES ('{[1,5)}', '[2000-01-01,2010-01-01)', 'a'), + ('{[1,2),[3,5)}', '[2000-01-01,2010-01-01)', 'b')"); + +$node_publisher->safe_psql('postgres', + "CREATE PUBLICATION pub1 FOR ALL TABLES"); +$node_subscriber->safe_psql('postgres', + "CREATE SUBSCRIPTION sub1 CONNECTION '$publisher_connstr' PUBLICATION pub1" +); +$node_subscriber->wait_for_subscription_sync; + +$result = $node_subscriber->safe_psql('postgres', + "SELECT id, a FROM temporal_mltrng ORDER BY a"); +is( $result, qq{{[1,5)}|a +{[1,2),[3,5)}|b}, 'synced temporal_mltrng lossy identity'); + +# Update each row. Each apply probes the identity index and gets *both* rows as +# candidates; without a recheck the wrong row would be updated. +$node_publisher->safe_psql('postgres', + "UPDATE temporal_mltrng SET a = 'a2' WHERE id = '{[1,5)}'"); +$node_publisher->safe_psql('postgres', + "UPDATE temporal_mltrng SET a = 'b2' WHERE id = '{[1,2),[3,5)}'"); + +$node_publisher->wait_for_catchup('sub1'); + +$result = $node_subscriber->safe_psql('postgres', + "SELECT id, a FROM temporal_mltrng ORDER BY a"); +is( $result, qq{{[1,5)}|a2 +{[1,2),[3,5)}|b2}, 'replicated temporal_mltrng UPDATE to correct rows'); + +# Same for DELETE: remove only one of the two bounding-range twins. +$node_publisher->safe_psql('postgres', + "DELETE FROM temporal_mltrng WHERE id = '{[1,2),[3,5)}'"); + +$node_publisher->wait_for_catchup('sub1'); + +$result = $node_subscriber->safe_psql('postgres', + "SELECT id, a FROM temporal_mltrng ORDER BY a"); +is($result, qq{{[1,5)}|a2}, + 'replicated temporal_mltrng DELETE of correct row'); + +# cleanup + +$node_publisher->safe_psql('postgres', "DROP TABLE temporal_mltrng"); +$node_subscriber->safe_psql('postgres', "DROP TABLE temporal_mltrng"); +$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub1"); +$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub1"); + done_testing(); diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl index 80177b214c0..03380ce2467 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -439,6 +439,92 @@ $node_A->safe_psql('postgres', $node_A->safe_psql('postgres', "DROP TABLE tab_defer"); $node_B->safe_psql('postgres', "DROP TABLE tab_defer"); +############################################################################### +# Check that the search for a deleted tuple rechecks a candidate found through +# a lossy replica identity index (a GiST index on a multirange, which stores +# only the bounding range), instead of blaming the transaction that deleted an +# unrelated row. +# +# The publisher updates a row that never existed on the subscriber, while the +# subscriber locally deleted a different row whose multirange key shares the +# same bounding range. Without a recheck, the dead "decoy" row would be +# accepted as the update target and the conflict would be misreported as +# update_deleted instead of update_missing. +############################################################################### + +# Publisher table holding the row that will be updated. +$node_B->safe_psql( + 'postgres', " + CREATE TABLE tab_mr (id int4multirange, valid_at daterange, v int); + ALTER TABLE tab_mr REPLICA IDENTITY FULL; + INSERT INTO tab_mr VALUES ('{[1,5)}', '[2000-01-01,2010-01-01)', 1); + ALTER PUBLICATION tap_pub_B ADD TABLE tab_mr; +"); + +# Subscriber table with the lossy replica identity index. The index is +# created before the conflict detection slot's xmin is pinned below, so that +# it can be used to find deleted tuples. +$node_A->safe_psql('postgres', + "CREATE TABLE tab_mr (id int4multirange, valid_at daterange, v int, PRIMARY KEY (id, valid_at WITHOUT OVERLAPS))" +); + +$node_A->safe_psql('postgres', + "ALTER SUBSCRIPTION $subname_AB REFRESH PUBLICATION WITH (copy_data = false)" +); +$node_A->wait_for_subscription_sync($node_B, $subname_AB); + +# Pin the conflict detection slot's xmin: hold an open transaction with an +# assigned transaction ID so that oldest_nonremovable_xid cannot advance past +# it. This keeps the decoy row deleted below retained and recently dead, +# making the test independent of timing. +my $holder = $node_A->background_psql('postgres'); +$holder->query_safe('BEGIN'); +my $pinned_xid = $holder->query_safe('SELECT txid_current()'); + +ok( $node_A->poll_query_until( + 'postgres', + "SELECT xmin::text = '$pinned_xid' FROM pg_replication_slots WHERE slot_name = 'pg_conflict_detection'" + ), + "conflict detection slot xmin pinned at $pinned_xid"); + +# Insert and locally delete the decoy row: a different multirange with the +# same bounding range as the publisher's key. +$node_A->safe_psql( + 'postgres', " + INSERT INTO tab_mr VALUES ('{[1,2),[3,5)}', '[2000-01-01,2010-01-01)', 9); + DELETE FROM tab_mr WHERE id = '{[1,2),[3,5)}'; +"); + +$log_location = -s $node_A->logfile; + +# The publisher updates its row; the old key sent is '{[1,5)}'. +$node_B->safe_psql('postgres', + "UPDATE tab_mr SET v = 2 WHERE id = '{[1,5)}'"); + +$node_B->wait_for_catchup($subname_AB); + +$logfile = slurp_file($node_A->logfile(), $log_location); + +# The decoy row must not be mistaken for the update target. +like( + $logfile, + qr/conflict detected on relation "public.tab_mr": conflict=update_missing.* +.*DETAIL:.* Could not find the row to be updated: remote row \(\Q{[1,5)}\E, \Q[2000-01-01,2010-01-01)\E, 2\), replica identity \(id, valid_at\)=\(\Q{[1,5)}\E, \Q[2000-01-01,2010-01-01)\E\)/, + 'update target reported as missing'); + +ok( $logfile !~ /conflict detected on relation "public.tab_mr": conflict=update_deleted/, + 'decoy row sharing the bounding range is not reported as update_deleted'); + +# Clean up +$holder->query_safe('COMMIT'); +ok($holder->quit, 'release the pinned transaction'); +$node_B->safe_psql('postgres', + "ALTER PUBLICATION tap_pub_B DROP TABLE tab_mr"); +$node_A->safe_psql('postgres', + "ALTER SUBSCRIPTION $subname_AB REFRESH PUBLICATION"); +$node_A->safe_psql('postgres', "DROP TABLE tab_mr"); +$node_B->safe_psql('postgres', "DROP TABLE tab_mr"); + ############################################################################### # Check that the xmin value of the conflict detection slot can be advanced when # the subscription has no tables. -- 2.47.3