From e2a30d7e43f568f0e9eb80b9f20ea63043e463fe Mon Sep 17 00:00:00 2001 From: Mikhail Nikalayeu Date: Thu, 27 Aug 2026 20:31:33 +0200 Subject: [PATCH v1] Don't re-derive in the scan what the apply worker knew about its index FindLogicalRepLocalIndex() picks the index to search the local relation by, either as the relation's replica identity or primary key, or as one usable for a REPLICA IDENTITY FULL remote relation. Which of the two it is decides whether the first index match is the tuple or every match has to be compared against the search slot. Only the OID is kept, though, so RelationFindReplTupleByIndex() works that out a second time on its own, by testing the index it is handed against GetRelationIdentityOrPK(). The second answer need not match the first. Apply holds only RowExclusiveLock, which conflicts with neither REINDEX CONCURRENTLY swapping the index for its rebuilt copy nor DROP INDEX CONCURRENTLY marking it invalid, so either can commit in between ExecOpenIndices() being where the invalidation lands. The scan then compares whole rows against a search slot that carries one only under REPLICA IDENTITY FULL, nothing matches, and the change is silently dropped as an update_missing conflict. Assertion builds trip over the identity check just above instead. Record which of the two it was in the relation map entry, next to the OID, and pass it down, settling the question once. Oversight in 89e46da5e51. Author: Mikhail Nikalayeu Reviewed-by: Amit Kapila Discussion: ................ Backpatch-through: 16, where it was introduced --- src/backend/executor/execReplication.c | 11 ++++++-- src/backend/replication/logical/relation.c | 19 ++++++++++--- src/backend/replication/logical/worker.c | 33 ++++++++++++++-------- src/include/executor/executor.h | 1 + src/include/replication/logicalrelation.h | 5 ++++ 5 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index cb1202e4506..fa8640da50c 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -171,9 +171,17 @@ build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel, * * If a matching tuple is found, lock it with lockmode, fill the slot with its * contents, and return true. Return false otherwise. + * + * 'isIdxSafeToSkipDuplicates' says how the scan's matches are to be treated: + * if true, the index determines the tuple all by itself and the first match + * is taken; if false, every match is compared against 'searchslot', which must + * then carry a complete row -- the remote relation has REPLICA IDENTITY FULL. + * The caller decides; deriving it here would let concurrent DDL flip it after + * the index was chosen. */ bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid, + bool isIdxSafeToSkipDuplicates, LockTupleMode lockmode, TupleTableSlot *searchslot, TupleTableSlot *outslot) @@ -186,13 +194,10 @@ RelationFindReplTupleByIndex(Relation rel, Oid idxoid, Relation idxrel; bool found; TypeCacheEntry **eq = NULL; - bool isIdxSafeToSkipDuplicates; /* Open the index. */ idxrel = index_open(idxoid, RowExclusiveLock); - isIdxSafeToSkipDuplicates = (GetRelationIdentityOrPK(rel) == idxoid); - InitDirtySnapshot(snap); /* Build scan key. */ diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index 8794574f42a..9409ba7d800 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -55,7 +55,7 @@ typedef struct LogicalRepPartMapEntry } LogicalRepPartMapEntry; static Oid FindLogicalRepLocalIndex(Relation localrel, LogicalRepRelation *remoterel, - AttrMap *attrMap); + AttrMap *attrMap, bool *isidentity); /* * Relcache invalidation callback for our relation map cache. @@ -453,7 +453,8 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) * on the relation). */ entry->localindexoid = FindLogicalRepLocalIndex(entry->localrel, remoterel, - entry->attrmap); + entry->attrmap, + &entry->isidentity); entry->localrelvalid = true; } @@ -725,7 +726,8 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root, * anything in the LogicalRepPartMapContext (hence CacheMemoryContext). */ entry->localindexoid = FindLogicalRepLocalIndex(partrel, remoterel, - entry->attrmap); + entry->attrmap, + &entry->isidentity); entry->localrelvalid = true; @@ -877,13 +879,19 @@ GetRelationIdentityOrPK(Relation rel) /* * Returns the index oid if we can use an index for subscriber. Otherwise, * returns InvalidOid. + * + * '*isidentity' is set to whether the returned index was chosen as the + * relation's replica identity or primary key, rather than as one usable + * for a REPLICA IDENTITY FULL remote relation. */ static Oid FindLogicalRepLocalIndex(Relation localrel, LogicalRepRelation *remoterel, - AttrMap *attrMap) + AttrMap *attrMap, bool *isidentity) { Oid idxoid; + *isidentity = false; + /* * We never need index oid for partitioned tables, always rely on leaf * partition's index. @@ -896,7 +904,10 @@ FindLogicalRepLocalIndex(Relation localrel, LogicalRepRelation *remoterel, */ idxoid = GetRelationIdentityOrPK(localrel); if (OidIsValid(idxoid)) + { + *isidentity = true; return idxoid; + } if (remoterel->replident == REPLICA_IDENTITY_FULL) { diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 10c5dbed25f..24ee7f5d334 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -386,14 +386,17 @@ static void apply_handle_update_internal(ApplyExecutionData *edata, ResultRelInfo *relinfo, TupleTableSlot *remoteslot, LogicalRepTupleData *newtup, - Oid localindexoid); + Oid localindexoid, + bool isidentity); static void apply_handle_delete_internal(ApplyExecutionData *edata, ResultRelInfo *relinfo, TupleTableSlot *remoteslot, - Oid localindexoid); + Oid localindexoid, + bool isidentity); static bool FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel, LogicalRepRelation *remoterel, Oid localidxoid, + bool isidentity, TupleTableSlot *remoteslot, TupleTableSlot **localslot); static void apply_handle_tuple_routing(ApplyExecutionData *edata, @@ -2638,7 +2641,8 @@ apply_handle_update(StringInfo s) remoteslot, &newtup, CMD_UPDATE); else apply_handle_update_internal(edata, edata->targetRelInfo, - remoteslot, &newtup, rel->localindexoid); + remoteslot, &newtup, rel->localindexoid, + rel->isidentity); finish_edata(edata); @@ -2663,7 +2667,7 @@ apply_handle_update_internal(ApplyExecutionData *edata, ResultRelInfo *relinfo, TupleTableSlot *remoteslot, LogicalRepTupleData *newtup, - Oid localindexoid) + Oid localindexoid, bool isidentity) { EState *estate = edata->estate; LogicalRepRelMapEntry *relmapentry = edata->targetRel; @@ -2674,11 +2678,12 @@ apply_handle_update_internal(ApplyExecutionData *edata, MemoryContext oldctx; EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL); + ExecOpenIndices(relinfo, false); found = FindReplTupleInLocalRel(edata, localrel, &relmapentry->remoterel, - localindexoid, + localindexoid, isidentity, remoteslot, &localslot); ExecClearTuple(remoteslot); @@ -2797,7 +2802,8 @@ apply_handle_delete(StringInfo s) ExecOpenIndices(relinfo, false); apply_handle_delete_internal(edata, relinfo, - remoteslot, rel->localindexoid); + remoteslot, rel->localindexoid, + rel->isidentity); ExecCloseIndices(relinfo); } @@ -2823,7 +2829,7 @@ static void apply_handle_delete_internal(ApplyExecutionData *edata, ResultRelInfo *relinfo, TupleTableSlot *remoteslot, - Oid localindexoid) + Oid localindexoid, bool isidentity) { EState *estate = edata->estate; Relation localrel = relinfo->ri_RelationDesc; @@ -2840,6 +2846,7 @@ apply_handle_delete_internal(ApplyExecutionData *edata, RelationGetIndexList(localrel) == NIL); found = FindReplTupleInLocalRel(edata, localrel, remoterel, localindexoid, + isidentity, remoteslot, &localslot); /* If found delete it. */ @@ -2880,6 +2887,7 @@ static bool FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel, LogicalRepRelation *remoterel, Oid localidxoid, + bool isidentity, TupleTableSlot *remoteslot, TupleTableSlot **localslot) { @@ -2903,13 +2911,13 @@ FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel, Relation idxrel = index_open(localidxoid, AccessShareLock); /* Index must be PK, RI, or usable for REPLICA IDENTITY FULL tables */ - Assert(GetRelationIdentityOrPK(idxrel) == localidxoid || + Assert(isidentity || IsIndexUsableForReplicaIdentityFull(BuildIndexInfo(idxrel), edata->targetRel->attrmap)); index_close(idxrel, AccessShareLock); #endif - found = RelationFindReplTupleByIndex(localrel, localidxoid, + found = RelationFindReplTupleByIndex(localrel, localidxoid, isidentity, LockTupleExclusive, remoteslot, *localslot); } @@ -3012,7 +3020,8 @@ apply_handle_tuple_routing(ApplyExecutionData *edata, case CMD_DELETE: apply_handle_delete_internal(edata, partrelinfo, remoteslot_part, - part_entry->localindexoid); + part_entry->localindexoid, + part_entry->isidentity); break; case CMD_UPDATE: @@ -3033,6 +3042,7 @@ apply_handle_tuple_routing(ApplyExecutionData *edata, found = FindReplTupleInLocalRel(edata, partrel, &part_entry->remoterel, part_entry->localindexoid, + part_entry->isidentity, remoteslot_part, &localslot); if (!found) { @@ -3128,7 +3138,8 @@ apply_handle_tuple_routing(ApplyExecutionData *edata, /* DELETE old tuple found in the old partition. */ apply_handle_delete_internal(edata, partrelinfo, localslot, - part_entry->localindexoid); + part_entry->localindexoid, + part_entry->isidentity); /* INSERT new tuple into the new partition. */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 7b8b07bd034..9008a602342 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -653,6 +653,7 @@ extern void check_exclusion_constraint(Relation heap, Relation index, */ extern StrategyNumber get_equal_strategy_number_for_am(Oid am); extern bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid, + bool isIdxSafeToSkipDuplicates, LockTupleMode lockmode, TupleTableSlot *searchslot, TupleTableSlot *outslot); diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h index e687b40a566..06a6800797f 100644 --- a/src/include/replication/logicalrelation.h +++ b/src/include/replication/logicalrelation.h @@ -33,6 +33,11 @@ typedef struct LogicalRepRelMapEntry AttrMap *attrmap; /* map of local attributes to remote ones */ bool updatable; /* Can apply updates/deletes? */ Oid localindexoid; /* which index to use, or InvalidOid if none */ + bool isidentity; /* localindexoid was chosen as the + * relation's replica identity or + * primary key, rather than as one + * usable for a REPLICA IDENTITY + * FULL remote relation */ /* Sync state. */ char state; -- 2.43.0