diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index eafe492e9b2..c32e531b340 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 398a2dbc7f8..256f03f8c2c 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.