From 7e0f1ce72dbcc39e772c632cccd6380b49c4bdcd Mon Sep 17 00:00:00 2001 From: Nisha Moond Date: Fri, 13 Nov 2026 16:50:46 +0530 Subject: [PATCH v1] Fix lookup of rows with NULL keys through a hash index in logical replication. With REPLICA IDENTITY FULL, the apply worker can use a hash index on the subscriber to find the old row of an UPDATE or DELETE. Hash indexes don't store NULLs, so when the indexed column is NULL in the old row, the index scan finds nothing. The change was then skipped and reported as update_missing or delete_missing, leaving the subscriber out of sync. Fall back to a sequential scan in that case. Rows with non-NULL keys still use the index. Do the same when searching for a deleted row to detect update_deleted conflicts with retain_dead_tuples. --- src/backend/executor/execReplication.c | 44 ++++++++++ .../subscription/t/032_subscribe_use_index.pl | 80 +++++++++++++++++++ src/test/subscription/t/035_conflicts.pl | 67 ++++++++++++++++ 3 files changed, 191 insertions(+) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 66a788e9e3d..f1b11e215b5 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -175,6 +175,29 @@ should_refetch_tuple(TM_Result res, TM_FailureData *tmfd) return refetch; } +/* + * Return true if 'searchslot' has a NULL in a key column of 'idxrel' and the + * index can't search for NULLs (e.g. hash). Such a scan finds nothing. + */ +static bool +replindex_key_unsearchable(Relation idxrel, TupleTableSlot *searchslot) +{ + int2vector *indkey = &idxrel->rd_index->indkey; + + if (idxrel->rd_indam->amsearchnulls) + return false; + + for (int i = 0; i < IndexRelationGetNumberOfKeyAttributes(idxrel); i++) + { + AttrNumber attno = indkey->values[i]; + + if (AttributeNumberIsValid(attno) && searchslot->tts_isnull[attno - 1]) + return true; + } + + return false; +} + /* * Search the relation 'rel' for tuple using the index. * @@ -207,6 +230,17 @@ RelationFindReplTupleByIndex(Relation rel, Oid idxoid, /* Open the index. */ idxrel = index_open(idxoid, RowExclusiveLock); + /* + * The index can't find this row, so use a sequential scan. + * !skipduplicates means the index was chosen for REPLICA IDENTITY FULL, + * so 'searchslot' has the whole row. + */ + if (!skipduplicates && replindex_key_unsearchable(idxrel, searchslot)) + { + index_close(idxrel, NoLock); + return RelationFindReplTupleSeq(rel, lockmode, searchslot, outslot); + } + InitDirtySnapshot(snap); /* Build scan key. */ @@ -736,6 +770,16 @@ RelationFindDeletedTupleInfoByIndex(Relation rel, Oid idxoid, idxrel = index_open(idxoid, RowExclusiveLock); + /* The index can't find this row, so use a sequential scan. */ + if (!skipduplicates && replindex_key_unsearchable(idxrel, searchslot)) + { + index_close(idxrel, NoLock); + ExecDropSingleTupleTableSlot(scanslot); + return RelationFindDeletedTupleInfoSeq(rel, InvalidOid, searchslot, + oldestxmin, delete_xid, + delete_origin, delete_time); + } + /* Build scan key. */ skey_attoff = build_replindex_scan_key(skey, rel, idxrel, searchslot); diff --git a/src/test/subscription/t/032_subscribe_use_index.pl b/src/test/subscription/t/032_subscribe_use_index.pl index bd80465f727..a5becaab3ff 100644 --- a/src/test/subscription/t/032_subscribe_use_index.pl +++ b/src/test/subscription/t/032_subscribe_use_index.pl @@ -606,6 +606,86 @@ $node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full"); # Testcase end: Subscription can use hash index # ============================================================================= +# ============================================================================= +# Testcase start: Subscription finds rows with NULL keys when using hash index +# +# Hash indexes do not store NULLs, so an index scan for a NULL key value finds +# nothing. UPDATE/DELETE of a row whose hash-indexed column is NULL must still +# be applied, rather than being reported as update_missing/delete_missing. + +# create table, initial data (including rows with NULL in the indexed column) +# and publication on pub +$node_publisher->safe_psql( + 'postgres', q[ + CREATE TABLE test_hash_null (x int, y text); + ALTER TABLE test_hash_null REPLICA IDENTITY FULL; + INSERT INTO test_hash_null VALUES + (1, 'one'), (2, 'two'), + (NULL, 'null-1'), (NULL, 'null-2'), (NULL, 'null-3'); + CREATE PUBLICATION tap_pub_hash_null FOR TABLE test_hash_null; +]); + +# create table, hash index and subscription on sub +$node_subscriber->safe_psql( + 'postgres', qq[ + CREATE TABLE test_hash_null (x int, y text); + CREATE INDEX test_hash_null_idx ON test_hash_null USING HASH (x); + CREATE SUBSCRIPTION tap_sub_hash_null CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_hash_null; +]); + +# wait for initial table synchronization to finish +$node_subscriber->wait_for_subscription_sync($node_publisher, $appname); + +# update a row with a non-NULL key, and make sure the hash index is used +$node_publisher->safe_psql('postgres', + "UPDATE test_hash_null SET y = 'two updated' WHERE x = 2"); +$node_publisher->wait_for_catchup($appname); +$node_subscriber->poll_query_until('postgres', + q{select (idx_scan = 1) from pg_stat_all_indexes where indexrelname = 'test_hash_null_idx';} + ) + or die + "Timed out while waiting for check subscriber tap_sub_hash_null updates one row via index"; + +# update and delete rows with a NULL key, and set the key of a row with a +# NULL key to a non-NULL value +$node_publisher->safe_psql( + 'postgres', q[ + UPDATE test_hash_null SET y = 'null-1 updated' WHERE y = 'null-1'; + DELETE FROM test_hash_null WHERE y = 'null-2'; + UPDATE test_hash_null SET x = 3 WHERE y = 'null-3'; +]); +$node_publisher->wait_for_catchup($appname); + +# make sure that the subscriber has the same data as the publisher +my $pub_result = $node_publisher->safe_psql('postgres', + "SELECT x, y FROM test_hash_null ORDER BY y"); +$result = $node_subscriber->safe_psql('postgres', + "SELECT x, y FROM test_hash_null ORDER BY y"); +is($result, $pub_result, + 'ensure subscriber applies UPDATE/DELETE of rows with NULL in hash-indexed column' +); +is( $result, qq(|null-1 updated +3|null-3 +1|one +2|two updated), + 'ensure subscriber has the correct data at the end of the test'); + +# cleanup pub +$node_publisher->safe_psql( + 'postgres', q[ + DROP PUBLICATION tap_pub_hash_null; + DROP TABLE test_hash_null; +]); +# cleanup sub +$node_subscriber->safe_psql( + 'postgres', q[ + DROP SUBSCRIPTION tap_sub_hash_null; + DROP TABLE test_hash_null; +]); + +# Testcase end: Subscription finds rows with NULL keys when using hash index +# ============================================================================= + # ============================================================================= # Testcase start: Subscription keeps using an index that concurrent DDL has # demoted from replica identity diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl index 48ce95d9d13..c133a08d530 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -525,6 +525,73 @@ $node_A->safe_psql('postgres', $node_A->safe_psql('postgres', "DROP TABLE tab_mr"); $node_B->safe_psql('postgres', "DROP TABLE tab_mr"); +############################################################################### +# Check that the search for a deleted tuple finds a row whose key is NULL in a +# hash index chosen for REPLICA IDENTITY FULL. Hash indexes do not store NULLs, +# so the row must be found without the index scan, and the conflict reported +# as update_deleted instead of update_missing. +############################################################################### + +# Publisher table holding the row that will be updated. +$node_B->safe_psql( + 'postgres', " + CREATE TABLE tab_hash_null (a int, b text); + ALTER TABLE tab_hash_null REPLICA IDENTITY FULL; + INSERT INTO tab_hash_null VALUES (NULL, 'null-1'); + ALTER PUBLICATION tap_pub_B ADD TABLE tab_hash_null; +"); + +# Subscriber table with the hash 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_hash_null (a int, b text); + CREATE INDEX tab_hash_null_idx ON tab_hash_null USING HASH (a); + ALTER SUBSCRIPTION $subname_AB REFRESH PUBLICATION; +"); +$node_A->wait_for_subscription_sync($node_B, $subname_AB); + +# Pin the conflict detection slot's xmin, as above. +$holder = $node_A->background_psql('postgres'); +$holder->query_safe('BEGIN'); +$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"); + +# Delete the row locally, then update it on the publisher. +$node_A->safe_psql('postgres', + "DELETE FROM tab_hash_null WHERE b = 'null-1'"); + +$log_location = -s $node_A->logfile; + +$node_B->safe_psql('postgres', + "UPDATE tab_hash_null SET b = 'null-1 updated' WHERE b = 'null-1'"); + +$node_B->wait_for_catchup($subname_AB); + +$logfile = slurp_file($node_A->logfile(), $log_location); +like( + $logfile, + qr/conflict detected on relation "public.tab_hash_null": conflict=update_deleted.* +.*DETAIL:.* Could not find the row to be updated: remote row \(null, null-1 updated\), replica identity full \(null, null-1\). +.*The row to be updated was deleted locally in transaction [0-9]+ at .*/, + 'update target row with NULL key was deleted in tab_hash_null'); + +# 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_hash_null"); +$node_A->safe_psql('postgres', + "ALTER SUBSCRIPTION $subname_AB REFRESH PUBLICATION"); +$node_A->safe_psql('postgres', "DROP TABLE tab_hash_null"); +$node_B->safe_psql('postgres', "DROP TABLE tab_hash_null"); + ############################################################################### # Check that the xmin value of the conflict detection slot can be advanced when # the subscription has no tables. -- 2.54.0 (Apple Git-157)