From b666a0677259d467c09997ec0f714db2b9062674 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Wed, 23 Sep 2026 00:16:57 -0700 Subject: [PATCH v2] Fix wrong replication for multirange WITHOUT OVERLAPS indexes Unlike a btree replica identity, a WITHOUT OVERLAPS constraint index may require rechecks, since the index tuples may use lossy compression. We were skipping the recheck, which could cause us to update/delete the wrong row. If the lookup signals that a recheck is needed, compare the tuples for equality. But unless REPLICA IDENTITY FULL, we only need to compare the index keys. Conflict detection in retain_dead_tuples had the same bug, causing us to report a cause of update_deleted when we should have reported update_missing. The fix is the same: honor rechecks. 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 | 44 +++++++++--- src/test/subscription/t/034_temporal.pl | 71 +++++++++++++++++++ src/test/subscription/t/035_conflicts.pl | 86 ++++++++++++++++++++++++ 3 files changed, 192 insertions(+), 9 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 18da4fe08f..55252fb2bf 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -179,7 +179,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 attributes (when the index 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 @@ -197,6 +199,7 @@ RelationFindReplTupleByIndex(Relation rel, Oid idxoid, Relation idxrel; bool found; TypeCacheEntry **eq = NULL; + Bitmapset *indexbitmap = NULL; /* Open the index. */ idxrel = index_open(idxoid, RowExclusiveLock); @@ -219,15 +222,26 @@ retry: while (table_index_getnext_slot(scan, ForwardScanDirection, outslot)) { /* - * 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. But a WITHOUT OVERLAPS key might require a + * recheck (e.g. GiST multirange). */ - if (!skipduplicates) + if (!skipduplicates || scan->xs_recheck) { if (eq == NULL) eq = palloc0_array(TypeCacheEntry *, outslot->tts_tupleDescriptor->natts); - if (!tuples_equal(outslot, searchslot, eq, NULL)) + /* Look up the key columns if we have a PK/RI index. */ + if (skipduplicates && indexbitmap == NULL) + { + indexbitmap = RelationGetIndexAttrBitmap(rel, + INDEX_ATTR_BITMAP_IDENTITY_KEY); + if (!indexbitmap) + indexbitmap = RelationGetIndexAttrBitmap(rel, + INDEX_ATTR_BITMAP_PRIMARY_KEY); + } + + if (!tuples_equal(outslot, searchslot, eq, indexbitmap)) continue; } @@ -668,6 +682,7 @@ RelationFindDeletedTupleInfoByIndex(Relation rel, Oid idxoid, IndexScanDesc scan; TupleTableSlot *scanslot; TypeCacheEntry **eq = NULL; + Bitmapset *indexbitmap = NULL; TupleDesc desc PG_USED_FOR_ASSERTS_ONLY = RelationGetDescr(rel); Assert(equalTupleDescs(desc, searchslot->tts_tupleDescriptor)); @@ -699,15 +714,26 @@ 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. But a WITHOUT OVERLAPS key might require a + * recheck (e.g. GiST multirange). */ - if (!skipduplicates) + if (!skipduplicates || scan->xs_recheck) { if (eq == NULL) eq = palloc0_array(TypeCacheEntry *, scanslot->tts_tupleDescriptor->natts); - if (!tuples_equal(scanslot, searchslot, eq, NULL)) + /* Look up the key columns if we have a PK/RI index. */ + if (skipduplicates && indexbitmap == NULL) + { + indexbitmap = RelationGetIndexAttrBitmap(rel, + INDEX_ATTR_BITMAP_IDENTITY_KEY); + if (!indexbitmap) + indexbitmap = RelationGetIndexAttrBitmap(rel, + INDEX_ATTR_BITMAP_PRIMARY_KEY); + } + + if (!tuples_equal(scanslot, searchslot, eq, indexbitmap)) continue; } diff --git a/src/test/subscription/t/034_temporal.pl b/src/test/subscription/t/034_temporal.pl index 66955e1b79..f8e662ef36 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 80177b214c..e204ea6603 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -865,4 +865,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.45.0