From 3cde90ecc5f85e7c6cb9fba29d473b2f6dd40ee1 Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Wed, 21 Oct 2026 08:50:02 +0530 Subject: [PATCH] Fix logical replication with lossy identity indexes. A GiST index on a multirange stores only its bounding range, so different multiranges can produce the same index key. However, RelationFindReplTupleByIndex() and RelationFindDeletedTupleInfoByIndex() assume that matches through a primary key or replica identity index are exact and skip the equality check. This can cause updates or deletes to target the wrong row, and conflict detection to report update_deleted instead of update_missing. Fix this by rechecking candidates when the index scan sets xs_recheck. Compare only the index key columns, using the scan keys' equality operators and collations to match the index's notion of key equality. Backpatch to 18, where WITHOUT OVERLAPS was introduced. The change to RelationFindDeletedTupleInfoByIndex() applies only to 19 and later, where update_deleted conflict detection was added. Reported-by: Andres Freund Author: Paul A. Jungwirth Author: Zhijie Hou Reviewed-by: Nisha Moond Reviewed-by: kedar anavardekar Reviewed-by: Amit Kapila 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 | 66 ++++++++++++++++-- src/test/subscription/t/034_temporal.pl | 71 +++++++++++++++++++ src/test/subscription/t/035_conflicts.pl | 86 ++++++++++++++++++++++++ 3 files changed, 219 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 18da4fe08f6..13644328164 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,8 +181,11 @@ 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 - * compared against 'searchslot', which must contain a complete row. + * without comparing it against 'searchslot', unless the index scan sets + * xs_recheck (as with a lossy GiST index for WITHOUT OVERLAPS), in which case + * only the index key columns are compared, using the index's own equality + * semantics. If false, all matching tuples are compared against 'searchslot', + * which must contain a complete row. */ bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid, @@ -220,7 +225,8 @@ 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. */ if (!skipduplicates) { @@ -230,6 +236,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 +367,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. * @@ -700,7 +754,8 @@ RelationFindDeletedTupleInfoByIndex(Relation rel, Oid idxoid, { /* * 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. */ if (!skipduplicates) { @@ -710,6 +765,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..d3a6d6b237f 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..48ce95d9d13 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 deleted 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 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 = $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 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/, + 'deleted 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.