From bcc7adc8e4cf9a2558a5a522cbbd3cc58e1314eb Mon Sep 17 00:00:00 2001 From: Zhijie Hou Date: Mon, 12 Oct 2026 18:04:38 +0800 Subject: [PATCH v5_PG18] 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 Reviewed-by: kedar anavardekar 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 | 60 +++++++++++++++++++-- src/test/subscription/t/034_temporal.pl | 71 +++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 3 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index c21c8e720ad..08ac54a3aed 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -37,6 +37,8 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2, TypeCacheEntry **eq); +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 @@ -176,8 +178,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', 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) using the index's own equality + * semantics. If false, all matching tuples are compared against 'searchslot', + * which must contain a complete row. */ bool RelationFindReplTupleByIndexExt(Relation rel, Oid idxoid, @@ -216,7 +221,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) { @@ -226,6 +232,9 @@ retry: if (!tuples_equal(outslot, searchslot, eq)) continue; } + else if (scan->xs_recheck && + !repl_index_key_equal(idxrel, outslot, searchslot, skey)) + continue; ExecMaterializeSlot(outslot); @@ -362,6 +371,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. * diff --git a/src/test/subscription/t/034_temporal.pl b/src/test/subscription/t/034_temporal.pl index 6bbf6567279..6dcb879d53b 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(); -- 2.34.1