From 34a00bfa7f7da45018fd6d5a787f3908b732c8a6 Mon Sep 17 00:00:00 2001 From: Hou Zhijie Date: Sun, 11 Oct 2026 22:15:53 +0800 Subject: [PATCH v3 2/3] Fix wrong replication for multirange WITHOUT OVERLAPS identity keys Unlike a btree replica identity, a WITHOUT OVERLAPS constraint index may require rechecks, since the index tuples may use lossy compression: a GiST index on a multirange 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() skip the equality check whenever the scan uses the primary key or replica identity index, assuming any match is exact. The former can update or delete the wrong row; the latter 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 Reviewed-by: Nisha Moond Discussion: https://postgr.es/m/kcyaok346iwocfdourf2oojgtz7ggxmh2ugis7qqhggs4qfzc2@cu6pkox4rfwp Discussion: https://postgr.es/m/e5nb5jbus2oa3pffmlo7pdvdckmchd54tqld4k3n6huyg5xxqn@7rounyvufjx6 Backpatch-through: 18 --- src/backend/executor/execReplication.c | 22 +++++- src/test/subscription/t/034_temporal.pl | 71 +++++++++++++++++++ src/test/subscription/t/035_conflicts.pl | 86 ++++++++++++++++++++++++ 3 files changed, 176 insertions(+), 3 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 57090993c6b..76f99ea70c3 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -181,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 @@ -222,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) { @@ -232,6 +238,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); @@ -751,7 +760,11 @@ 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: 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) { @@ -761,6 +774,9 @@ RelationFindDeletedTupleInfoByIndex(Relation rel, Oid idxoid, if (!tuples_equal(scanslot, searchslot, eq)) 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 246991224c7..16756ce7c42 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -960,4 +960,90 @@ $node_subscriber->safe_psql('clt_ts_test', "DROP SUBSCRIPTION sub_ts_test"); $node_subscriber->safe_psql('postgres', "DROP DATABASE clt_ts_test"); $node_subscriber->safe_psql('postgres', "DROP TABLESPACE backup_space"); +############################################################################### +# Check that update_deleted vs update_missing is classified correctly when the +# replica identity is a lossy index (a WITHOUT OVERLAPS multirange GiST index). +# +# RelationFindDeletedTupleInfoByIndex() probes the identity index for a +# recently-dead tuple to decide whether a missing update target was deleted +# (update_deleted) or never existed (update_missing). A GiST index on a +# multirange is lossy: two multiranges with the same bounding range, e.g. +# {[1,5)} and {[1,2),[3,5)}, are indistinguishable to the index. Without a +# recheck, a dead tuple that merely shares the bounding range is accepted, +# misreporting update_deleted (and blaming the transaction that deleted the +# unrelated row) when the answer should be update_missing. +############################################################################### + +my $node_pub_mr = PostgreSQL::Test::Cluster->new('pub_mr'); +$node_pub_mr->init(allows_streaming => 'logical'); +$node_pub_mr->append_conf('postgresql.conf', 'track_commit_timestamp = on'); +$node_pub_mr->start; + +my $node_sub_mr = PostgreSQL::Test::Cluster->new('sub_mr'); +$node_sub_mr->init(allows_streaming => 'logical'); +$node_sub_mr->append_conf('postgresql.conf', + qq(track_commit_timestamp = on +autovacuum = off)); +$node_sub_mr->start; + +my $mr_ddl = qq( + CREATE TABLE tmr ( + id int4multirange, + valid_at daterange, + b text, + PRIMARY KEY (id, valid_at WITHOUT OVERLAPS))); +$node_pub_mr->safe_psql('postgres', $mr_ddl); +$node_sub_mr->safe_psql('postgres', $mr_ddl); + +# The target row exists on the publisher before the subscription, and the +# subscription uses copy_data = false, so the subscriber never holds a copy of +# it (neither live nor dead). A later UPDATE of it is therefore a missing +# target on the subscriber. +$node_pub_mr->safe_psql('postgres', + "INSERT INTO tmr VALUES ('{[1,5)}', '[2000-01-01,2001-01-01)', 'orig')"); +$node_pub_mr->safe_psql('postgres', "CREATE PUBLICATION pub_mr FOR TABLE tmr"); + +my $connstr_mr = $node_pub_mr->connstr . ' dbname=postgres'; +$node_sub_mr->safe_psql('postgres', + "CREATE SUBSCRIPTION sub_mr CONNECTION '$connstr_mr' PUBLICATION pub_mr WITH (copy_data = false, retain_dead_tuples = true)" +); +$node_sub_mr->wait_for_subscription_sync($node_pub_mr, 'sub_mr'); + +# Wait until dead tuples are being retained for conflict detection. +ok( $node_sub_mr->poll_query_until( + 'postgres', + "SELECT xmin IS NOT NULL FROM pg_replication_slots WHERE slot_name = 'pg_conflict_detection'" + ), + "conflict detection slot xmin is valid on the multirange subscriber"); + +# Create a dead tuple whose multirange differs from the target's but shares the +# same bounding range [1,5). Deleted locally, so its origin differs from the +# apply worker's. +$node_sub_mr->safe_psql('postgres', + "INSERT INTO tmr VALUES ('{[1,2),[3,5)}', '[2000-01-01,2001-01-01)', 'twin')"); +$node_sub_mr->safe_psql('postgres', "DELETE FROM tmr WHERE id = '{[1,2),[3,5)}'"); + +my $mr_log_offset = -s $node_sub_mr->logfile; + +# Update the target on the publisher. On the subscriber there is no matching +# row, only the lossy-bounding-range twin's dead tuple. +$node_pub_mr->safe_psql('postgres', + "UPDATE tmr SET b = 'upd' WHERE id = '{[1,5)}'"); +$node_pub_mr->wait_for_catchup('sub_mr'); + +my $mr_log = slurp_file($node_sub_mr->logfile, $mr_log_offset); +like( + $mr_log, + qr/conflict detected on relation "public.tmr": conflict=update_missing/, + 'missing update target is not misreported as update_deleted (lossy identity index)' +); +unlike( + $mr_log, + qr/conflict detected on relation "public.tmr": conflict=update_deleted/, + 'lossy dead twin does not cause a spurious update_deleted'); + +$node_sub_mr->safe_psql('postgres', "DROP SUBSCRIPTION sub_mr"); +$node_sub_mr->stop; +$node_pub_mr->stop; + done_testing(); -- 2.34.1