From 3efd2dce4d541c5b5406bb78ef644366b3ba70d4 Mon Sep 17 00:00:00 2001 From: Nisha Moond Date: Wed, 30 Sep 2026 15:33:03 +0530 Subject: [PATCH v6 1/2] Use the relation map's index when searching deleted tuples sequentially When the index can't be used to find a recently deleted row, RelationFindDeletedTupleInfoSeq() scans the table. It looked up the relation's replica identity or primary key again to decide which columns to compare, which could disagree with the index chosen when the relation was opened. A concurrent DROP INDEX CONCURRENTLY could remove the replica identity mid-change, so the whole row was compared, and update_deleted was reported as update_missing. Removing the relcache lookups loses nothing, as the relation map entry already records the replica identity index, or failing that the primary key. The only difference is that a deferrable primary key, which cannot serve as a replica identity, is no longer used; comparing only its columns could report a deleted row with a different value as update_deleted. Pass the index from the relation map and compare its key columns only when it is the replica identity or primary key. Otherwise compare the whole row, as the publisher then uses REPLICA IDENTITY FULL. Author: Hayato Kuroda Author: Nisha Moond Reviewed-by: Vignesh C Reviewed-by: Zhijie Hou Discussion: https://postgr.es/m/CABdArM5ydwdRrpaZyK1q2p3-vY_+pnBtTmkvg_pcM=gHwmH7Kg@mail.gmail.com Discussion: https://postgr.es/m/CAA4eK1KHDofZjMRxJLHzP1Pbnkfekf7RLgR=TUJsxqmrYEKP4w@mail.gmail.com Backpatch-through: 19 --- src/backend/executor/execReplication.c | 49 ++++++++++++++++-------- src/backend/replication/logical/worker.c | 17 ++++++-- src/include/executor/executor.h | 2 +- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index dd42acc13e2..782d8ac8ab8 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -536,6 +536,10 @@ update_most_recent_deletion_info(TupleTableSlot *scanslot, * returns the transaction ID, origin, and commit timestamp of the transaction * that deleted this tuple. * + * If 'identidxoid' is valid, it must be the replica identity or primary key + * index, and only its key columns are compared. Otherwise, all columns are + * compared. + * * 'oldestxmin' acts as a cutoff transaction ID. Tuples deleted by transactions * with IDs >= 'oldestxmin' are considered recently dead and are eligible for * conflict detection. @@ -563,7 +567,8 @@ update_most_recent_deletion_info(TupleTableSlot *scanslot, * tuple was deleted most recently. */ bool -RelationFindDeletedTupleInfoSeq(Relation rel, TupleTableSlot *searchslot, +RelationFindDeletedTupleInfoSeq(Relation rel, Oid identidxoid, + 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)); @@ -582,21 +587,35 @@ RelationFindDeletedTupleInfoSeq(Relation rel, TupleTableSlot *searchslot, *delete_time = 0; /* - * If the relation has a replica identity key or a primary key that is - * unusable for locating deleted tuples (see - * IsIndexUsableForFindingDeletedTuple), a full table scan becomes - * necessary. In such cases, comparing the entire tuple is not required, - * since the remote tuple might not include all column values. Instead, - * the indexed columns alone are sufficient to identify the target tuple - * (see logicalrep_rel_mark_updatable). + * We get here when the caller's index, if any, cannot be used for + * locating deleted tuples (see IsIndexUsableForFindingDeletedTuple). If + * that index is the replica identity or primary key, the remote tuple + * might not include all column values, but the index's key columns alone + * are sufficient to identify the target tuple. Otherwise, the remote + * relation has REPLICA IDENTITY FULL, so compare the entire tuple. */ - indexbitmap = RelationGetIndexAttrBitmap(rel, - INDEX_ATTR_BITMAP_IDENTITY_KEY); + if (OidIsValid(identidxoid)) + { + /* The index must have been locked already */ + Relation idxrel = index_open(identidxoid, NoLock); - /* fallback to PK if no replica identity */ - if (!indexbitmap) - indexbitmap = RelationGetIndexAttrBitmap(rel, - INDEX_ATTR_BITMAP_PRIMARY_KEY); + /* + * The index may no longer be the replica identity if DROP INDEX + * CONCURRENTLY or REINDEX CONCURRENTLY ran meanwhile, but it stays + * unique and non-partial, which is all we rely on. See + * FindReplTupleInLocalRel(). + */ + 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++) + indexbitmap = bms_add_member(indexbitmap, + idxrel->rd_index->indkey.values[i] - + FirstLowInvalidHeapAttributeNumber); + + index_close(idxrel, NoLock); + } eq = palloc0_array(TypeCacheEntry *, searchslot->tts_tupleDescriptor->natts); diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index ab7c4ced66d..5a5e3844df1 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -3409,9 +3409,20 @@ FindDeletedTupleInLocalRel(Relation localrel, delete_xid, delete_origin, delete_time); else - return RelationFindDeletedTupleInfoSeq(localrel, remoteslot, - oldestxmin, delete_xid, - delete_origin, delete_time); + { + /* + * Pass the index only if it is the replica identity or primary key, + * so that its key columns are compared. Use the relation map's choice + * rather than looking it up again, since concurrent DDL may have + * changed the relation's replica identity. + */ + return RelationFindDeletedTupleInfoSeq(localrel, + relmapentry->idxisreplident ? + localidxoid : InvalidOid, + remoteslot, oldestxmin, + delete_xid, delete_origin, + delete_time); + } } /* diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 23a09a70aa2..152a1dfa568 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 identidxoid, TupleTableSlot *searchslot, TransactionId oldestxmin, TransactionId *delete_xid, -- 2.54.0 (Apple Git-157)