From 07841e481d4cda749a648960f93d3887ab4ebc95 Mon Sep 17 00:00:00 2001 From: Hayato Kuroda Date: Wed, 30 Sep 2026 16:17:02 +0900 Subject: [PATCH vatop-v3] Don't use RelationGetIndexAttrBitmap --- src/backend/executor/execReplication.c | 37 ++++++--- src/backend/replication/logical/worker.c | 11 ++- src/include/executor/executor.h | 2 +- src/test/subscription/t/035_conflicts.pl | 101 +++++++++++++++++++++++ 4 files changed, 138 insertions(+), 13 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 168d8a68c13..3a036e3a57e 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -561,9 +561,14 @@ update_most_recent_deletion_info(TupleTableSlot *scanslot, * * The commit timestamp of the deleting transaction is used to determine which * tuple was deleted most recently. + * + * If 'idxoid' is valid, its key columns are used for comparison. The index + * must be an identity or primary key index. Otherwise, all columns are used + * for comparison. */ bool -RelationFindDeletedTupleInfoSeq(Relation rel, TupleTableSlot *searchslot, +RelationFindDeletedTupleInfoSeq(Relation rel, Oid idxoid, + TupleTableSlot *searchslot, TransactionId oldestxmin, TransactionId *delete_xid, ReplOriginId *delete_origin, @@ -572,7 +577,7 @@ RelationFindDeletedTupleInfoSeq(Relation rel, TupleTableSlot *searchslot, TupleTableSlot *scanslot; TableScanDesc scan; TypeCacheEntry **eq; - Bitmapset *indexbitmap; + Bitmapset *indexbitmap = NULL; TupleDesc desc PG_USED_FOR_ASSERTS_ONLY = RelationGetDescr(rel); Assert(equalTupleDescs(desc, searchslot->tts_tupleDescriptor)); @@ -590,16 +595,25 @@ RelationFindDeletedTupleInfoSeq(Relation rel, TupleTableSlot *searchslot, * the indexed columns alone are sufficient to identify the target tuple * (see logicalrep_rel_mark_updatable). */ - indexbitmap = RelationGetIndexAttrBitmap(rel, - INDEX_ATTR_BITMAP_IDENTITY_KEY); + if (OidIsValid(idxoid)) + { + Relation idxrel = index_open(idxoid, AccessShareLock); - /* - * fallback to PK if no replica identity, but only if the PK is not - * deferrable. - */ - if (!indexbitmap && OidIsValid(RelationGetPrimaryKeyIndex(rel, false))) - indexbitmap = RelationGetIndexAttrBitmap(rel, - INDEX_ATTR_BITMAP_PRIMARY_KEY); + Assert(idxrel->rd_index->indisunique); + Assert(heap_attisnull(idxrel->rd_indextuple, Anum_pg_index_indpred, + NULL)); + + for (int i = 0; i < idxrel->rd_index->indnkeyatts; i++) + { + AttrNumber attnum = idxrel->rd_index->indkey.values[i]; + + Assert(AttributeNumberIsValid(attnum)); + indexbitmap = bms_add_member(indexbitmap, + attnum - FirstLowInvalidHeapAttributeNumber); + } + + index_close(idxrel, AccessShareLock); + } eq = palloc0_array(TypeCacheEntry *, searchslot->tts_tupleDescriptor->natts); @@ -627,6 +641,7 @@ RelationFindDeletedTupleInfoSeq(Relation rel, TupleTableSlot *searchslot, table_endscan(scan); ExecDropSingleTupleTableSlot(scanslot); + bms_free(indexbitmap); return *delete_time != 0; } diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index ab7c4ced66d..3f4f298f069 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -3409,9 +3409,18 @@ FindDeletedTupleInLocalRel(Relation localrel, delete_xid, delete_origin, delete_time); else - return RelationFindDeletedTupleInfoSeq(localrel, remoteslot, + { + /* + * Tell the identity or primary index if exists to refer its key + * columns. We should not search such an index because concurrent DDL + * might have changed them. + */ + Oid idxoid = relmapentry->idxisreplident ? localidxoid : InvalidOid; + + return RelationFindDeletedTupleInfoSeq(localrel, idxoid, remoteslot, oldestxmin, delete_xid, delete_origin, delete_time); + } } /* diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 23a09a70aa2..1e4c51ab7b2 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -781,7 +781,7 @@ extern bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid, TupleTableSlot *outslot); extern bool RelationFindReplTupleSeq(Relation rel, LockTupleMode lockmode, TupleTableSlot *searchslot, TupleTableSlot *outslot); -extern bool RelationFindDeletedTupleInfoSeq(Relation rel, +extern bool RelationFindDeletedTupleInfoSeq(Relation rel, Oid idxoid, TupleTableSlot *searchslot, TransactionId oldestxmin, TransactionId *delete_xid, diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl index e6dc72e8fb9..ec76b1b1ce2 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -439,6 +439,107 @@ $node_A->safe_psql('postgres', $node_A->safe_psql('postgres', "DROP TABLE tab_defer"); $node_B->safe_psql('postgres', "DROP TABLE tab_defer"); +############################################################################### +# Ensure that a sequential scan for finding deleted tuples keeps using the +# replica identity selected by the relation map when concurrent DROP INDEX +# drops that index. +############################################################################### + +SKIP: +{ + skip 'Injection points not supported by this build', 1 + unless $ENV{enable_injection_points} eq 'yes'; + skip 'Extension injection_points not installed', 1 + unless $node_A->check_extension('injection_points'); + + $node_A->safe_psql('postgres', 'CREATE EXTENSION injection_points'); + + # Start a transaction before creating an index to make the index not + # suitable for the index search. See IsIndexUsableForFindingDeletedTuple(). + my $old_xact = $node_A->background_psql('postgres'); + $old_xact->query_safe(q[ + BEGIN; + SELECT txid_current(); + ]); + + $node_B->safe_psql( + 'postgres', q[ + CREATE TABLE tab_drop_deleted (a int PRIMARY KEY, b int); + INSERT INTO tab_drop_deleted VALUES (1, 1); + CREATE PUBLICATION pub_drop_deleted FOR TABLE tab_drop_deleted; + ]); + $node_A->safe_psql( + 'postgres', q[ + CREATE TABLE tab_drop_deleted (a int NOT NULL, b int); + CREATE UNIQUE INDEX tab_drop_deleted_ri ON tab_drop_deleted (a); + ALTER TABLE tab_drop_deleted REPLICA IDENTITY + USING INDEX tab_drop_deleted_ri; + ]); + $node_A->safe_psql( + 'postgres', + "CREATE SUBSCRIPTION sub_drop_deleted + CONNECTION '$node_B_connstr application_name=drop_deleted' + PUBLICATION pub_drop_deleted + WITH (retain_dead_tuples = true)" + ); + $node_A->wait_for_subscription_sync($node_B, 'drop_deleted'); + + # Make the deleted row differ outside the replica identity. A sequential + # scan can find it only if it compares the cached index's key columns. + $node_A->safe_psql( + 'postgres', q[ + UPDATE tab_drop_deleted SET b = 10 WHERE a = 1; + DELETE FROM tab_drop_deleted WHERE a = 1; + ]); + $node_A->safe_psql('postgres', + "SELECT injection_points_attach('apply-update-before-open-indices', 'wait')" + ); + $node_B->safe_psql('postgres', + 'UPDATE tab_drop_deleted SET b = 2 WHERE a = 1'); + $node_A->wait_for_event('logical replication apply worker', + 'apply-update-before-open-indices'); + + $log_location = -s $node_A->logfile; + + # Drop the replica identity index concurrently + my $drop = $node_A->background_psql('postgres'); + $drop->query_until( + qr/starting_drop/, q[ + \echo starting_drop + DROP INDEX CONCURRENTLY tab_drop_deleted_ri; + ]); + $node_A->poll_query_until('postgres', + "SELECT count(*) = 0 FROM pg_index" + . " WHERE indrelid = 'tab_drop_deleted'::regclass" + . " AND indisreplident") + or die "timed out waiting for the identity index to be invalidated"; + + $node_A->safe_psql( + 'postgres', + "SELECT injection_points_detach('apply-update-before-open-indices'); + SELECT injection_points_wakeup('apply-update-before-open-indices');" + ); + + # Ensure update_deleted is detected. Without the cached index key columns, + # the sequential scan would compare the whole row and report + # update_missing. + $node_B->wait_for_catchup('drop_deleted'); + $logfile = slurp_file($node_A->logfile(), $log_location); + like( + $logfile, + qr/conflict detected on relation "public.tab_drop_deleted": conflict=update_deleted/, + 'sequential scan uses cached identity columns after concurrent drop'); + + # Clean up + $drop->quit; + $old_xact->quit; + $node_A->safe_psql('postgres', 'DROP SUBSCRIPTION sub_drop_deleted'); + $node_A->safe_psql('postgres', 'DROP TABLE tab_drop_deleted'); + $node_A->safe_psql('postgres', 'DROP EXTENSION injection_points'); + $node_B->safe_psql('postgres', 'DROP PUBLICATION pub_drop_deleted'); + $node_B->safe_psql('postgres', 'DROP TABLE tab_drop_deleted'); +} + ############################################################################### # Check that the xmin value of the conflict detection slot can be advanced when # the subscription has no tables. -- 2.52.0