From 1b89e7f7947d219f3a338f03b3e62b1a20a1d047 Mon Sep 17 00:00:00 2001 From: Mikhail Nikalayeu Date: Thu, 27 Aug 2026 20:29:30 +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 | 30 ++++++++++++++-------- src/include/executor/executor.h | 1 + src/include/replication/logicalrelation.h | 5 ++++ 5 files changed, 49 insertions(+), 17 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index bb78d89265d..52277e5781b 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -174,9 +174,17 @@ should_refetch_tuple(TM_Result res, TM_FailureData *tmfd) * * 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) @@ -189,13 +197,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 0915b4d8b15..ad21732a7f3 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. @@ -484,7 +484,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; } @@ -756,7 +757,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; @@ -917,13 +919,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. @@ -936,7 +944,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 5b53b5fb64e..75100b1230d 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -380,14 +380,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, @@ -2657,7 +2660,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); @@ -2682,7 +2686,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; @@ -2694,11 +2698,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); /* @@ -2840,7 +2845,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); } @@ -2866,7 +2872,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; @@ -2884,6 +2890,7 @@ apply_handle_delete_internal(ApplyExecutionData *edata, RelationGetIndexList(localrel) == NIL); found = FindReplTupleInLocalRel(edata, localrel, remoterel, localindexoid, + isidentity, remoteslot, &localslot); /* If found delete it. */ @@ -2934,6 +2941,7 @@ static bool FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel, LogicalRepRelation *remoterel, Oid localidxoid, + bool isidentity, TupleTableSlot *remoteslot, TupleTableSlot **localslot) { @@ -2957,14 +2965,14 @@ 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(localrel) == localidxoid || + Assert(isidentity || (remoterel->replident == REPLICA_IDENTITY_FULL && IsIndexUsableForReplicaIdentityFull(idxrel, edata->targetRel->attrmap))); index_close(idxrel, AccessShareLock); #endif - found = RelationFindReplTupleByIndex(localrel, localidxoid, + found = RelationFindReplTupleByIndex(localrel, localidxoid, isidentity, LockTupleExclusive, remoteslot, *localslot); } @@ -3067,7 +3075,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: @@ -3090,6 +3099,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) { diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index b6697c0fc75..d9f4b663979 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -757,6 +757,7 @@ extern void check_exclusion_constraint(Relation heap, Relation index, * prototypes from functions in execReplication.c */ 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 7a561a8e8d8..e3657296e9b 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