diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 9dd7168461..e2b29cd9bd 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -132,7 +132,7 @@ build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel, * contents, and return true. Return false otherwise. */ bool -RelationFindReplTupleByIndex(Relation rel, Oid idxoid, +RelationFindReplTupleByIndex(Relation rel, Relation idxrel, LockTupleMode lockmode, TupleTableSlot *searchslot, TupleTableSlot *outslot) @@ -142,15 +142,12 @@ RelationFindReplTupleByIndex(Relation rel, Oid idxoid, IndexScanDesc scan; SnapshotData snap; TransactionId xwait; - Relation idxrel; bool found; TypeCacheEntry **eq = NULL; bool isIdxSafeToSkipDuplicates; - /* Open the index. */ - idxrel = index_open(idxoid, RowExclusiveLock); - - isIdxSafeToSkipDuplicates = (GetRelationIdentityOrPK(rel) == idxoid); + isIdxSafeToSkipDuplicates = + (GetRelationIdentityOrPK(rel) == RelationGetRelid(idxrel)); InitDirtySnapshot(snap); @@ -175,16 +172,7 @@ retry: if (!isIdxSafeToSkipDuplicates) { if (eq == NULL) - { -#ifdef USE_ASSERT_CHECKING - /* apply assertions only once for the input idxoid */ - IndexInfo *indexInfo = BuildIndexInfo(idxrel); - - Assert(IsIndexUsableForReplicaIdentityFull(indexInfo)); -#endif - eq = palloc0(sizeof(*eq) * outslot->tts_tupleDescriptor->natts); - } if (!tuples_equal(outslot, searchslot, eq)) continue; @@ -260,9 +248,6 @@ retry: index_endscan(scan); - /* Don't release lock until commit. */ - index_close(idxrel, NoLock); - return found; } diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index 57ad22b48a..57a5de44b1 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -731,52 +731,6 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root, return entry; } -/* - * Returns true if the given index consists only of expressions such as: - * CREATE INDEX idx ON table(foo(col)); - * - * Returns false even if there is one column reference: - * CREATE INDEX idx ON table(foo(col), col_2); - */ -static bool -IsIndexOnlyOnExpression(IndexInfo *indexInfo) -{ - for (int i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++) - { - AttrNumber attnum = indexInfo->ii_IndexAttrNumbers[i]; - - if (AttributeNumberIsValid(attnum)) - return false; - } - - return true; -} - -/* - * Returns true if the attrmap contains the leftmost column of the index. - * Otherwise returns false. - * - * attrmap is a map of local attributes to remote ones. We can consult this - * map to check whether the local index attribute has a corresponding remote - * attribute. - */ -static bool -RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap) -{ - AttrNumber keycol; - - Assert(indexInfo->ii_NumIndexAttrs >= 1); - - keycol = indexInfo->ii_IndexAttrNumbers[0]; - if (!AttributeNumberIsValid(keycol)) - return false; - - if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol)) - return false; - - return attrmap->attnums[AttrNumberGetAttrOffset(keycol)] >= 0; -} - /* * Returns the oid of an index that can be used by the apply worker to scan * the relation. The index must be btree, non-partial, and have at least @@ -815,19 +769,16 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap) { Oid idxoid = lfirst_oid(lc); bool isUsableIdx; - bool containsLeftMostCol; Relation idxRel; IndexInfo *idxInfo; idxRel = index_open(idxoid, AccessShareLock); idxInfo = BuildIndexInfo(idxRel); - isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo); - containsLeftMostCol = - RemoteRelContainsLeftMostColumnOnIdx(idxInfo, attrmap); + isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap); index_close(idxRel, AccessShareLock); /* Return the first eligible index found */ - if (isUsableIdx && containsLeftMostCol) + if (isUsableidx) return idxoid; } @@ -837,15 +788,35 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap) /* * Returns true if the index is usable for replica identity full. For details, * see FindUsableIndexForReplicaIdentityFull. + * + * attrmap is a map of local attributes to remote ones. We can consult this + * map to check whether the local index attribute has a corresponding remote + * attribute. */ bool -IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo) +IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap) { bool is_btree = (indexInfo->ii_Am == BTREE_AM_OID); bool is_partial = (indexInfo->ii_Predicate != NIL); - bool is_only_on_expression = IsIndexOnlyOnExpression(indexInfo); + AttrNumber keycol; - return is_btree && !is_partial && !is_only_on_expression; + /* Must be btree and non-partial index */ + if (!is_btree || is_partial) + return false; + + Assert(indexInfo->ii_NumIndexAttrs >= 1); + + /* The leftmost index field must not be an expression */ + keycol = indexInfo->ii_IndexAttrNumbers[0]; + if (!AttributeNumberIsValid(keycol)) + return false; + + /* And must reference the remote relation column */ + if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol) || + attrmap->attnums[AttrNumberGetAttrOffset(keycol)] < 0) + return false; + + return true; } /* diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 0ee764d68f..6c09334ffc 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -78,7 +78,7 @@ * * To avoid this, and similar prepare confusions the subscription's two_phase * commit is enabled only after the initial sync is over. The two_phase option - * has been implemented as a tri-state with values DISABLED, PENDING, and +n * has been implemented as a tri-state with values DISABLED, PENDING, and * ENABLED. * * Even if the user specifies they want a subscription with two_phase = on, @@ -140,6 +140,7 @@ #include #include +#include "access/genam.h" #include "access/table.h" #include "access/tableam.h" #include "access/twophase.h" @@ -410,7 +411,7 @@ static void apply_handle_delete_internal(ApplyExecutionData *edata, ResultRelInfo *relinfo, TupleTableSlot *remoteslot, Oid localindexoid); -static bool FindReplTupleInLocalRel(EState *estate, Relation localrel, +static bool FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel, LogicalRepRelation *remoterel, Oid localidxoid, TupleTableSlot *remoteslot, @@ -2677,7 +2678,7 @@ apply_handle_update_internal(ApplyExecutionData *edata, EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL); ExecOpenIndices(relinfo, false); - found = FindReplTupleInLocalRel(estate, localrel, + found = FindReplTupleInLocalRel(edata, localrel, &relmapentry->remoterel, localindexoid, remoteslot, &localslot); @@ -2830,7 +2831,7 @@ apply_handle_delete_internal(ApplyExecutionData *edata, EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL); ExecOpenIndices(relinfo, false); - found = FindReplTupleInLocalRel(estate, localrel, remoterel, localindexoid, + found = FindReplTupleInLocalRel(edata, localrel, remoterel, localindexoid, remoteslot, &localslot); /* If found delete it. */ @@ -2869,12 +2870,13 @@ apply_handle_delete_internal(ApplyExecutionData *edata, * Local tuple, if found, is returned in '*localslot'. */ static bool -FindReplTupleInLocalRel(EState *estate, Relation localrel, +FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel, LogicalRepRelation *remoterel, Oid localidxoid, TupleTableSlot *remoteslot, TupleTableSlot **localslot) { + EState *estate = edata->estate; bool found; /* @@ -2889,9 +2891,24 @@ FindReplTupleInLocalRel(EState *estate, Relation localrel, (remoterel->replident == REPLICA_IDENTITY_FULL)); if (OidIsValid(localidxoid)) - found = RelationFindReplTupleByIndex(localrel, localidxoid, + { + Relation idxrel; + + /* Open the index. */ + idxrel = index_open(localidxoid, RowExclusiveLock); + + /* Index must be PK, RI, or usable for REPLICA IDENTITY FULL tables */ + Assert(GetRelationIdentityOrPK(idxrel) == localidxoid || + IsIndexUsableForReplicaIdentityFull(BuildIndexInfo(idxrel), + edata->targetRel->attrmap)); + + found = RelationFindReplTupleByIndex(localrel, idxrel, LockTupleExclusive, remoteslot, *localslot); + + /* Don't release lock until commit. */ + index_close(idxrel, NoLock); + } else found = RelationFindReplTupleSeq(localrel, LockTupleExclusive, remoteslot, *localslot); @@ -3009,7 +3026,7 @@ apply_handle_tuple_routing(ApplyExecutionData *edata, bool found; /* Get the matching local tuple from the partition. */ - found = FindReplTupleInLocalRel(estate, partrel, + found = FindReplTupleInLocalRel(edata, partrel, &part_entry->remoterel, part_entry->localindexoid, remoteslot_part, &localslot); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index ac02247947..92153da1df 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -646,7 +646,7 @@ extern void check_exclusion_constraint(Relation heap, Relation index, /* * prototypes from functions in execReplication.c */ -extern bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid, +extern bool RelationFindReplTupleByIndex(Relation rel, Relation idxrel, LockTupleMode lockmode, TupleTableSlot *searchslot, TupleTableSlot *outslot); diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h index 921b9974db..3f4d906d74 100644 --- a/src/include/replication/logicalrelation.h +++ b/src/include/replication/logicalrelation.h @@ -48,7 +48,7 @@ extern LogicalRepRelMapEntry *logicalrep_partition_open(LogicalRepRelMapEntry *r Relation partrel, AttrMap *map); extern void logicalrep_rel_close(LogicalRepRelMapEntry *rel, LOCKMODE lockmode); -extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo); +extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap); extern Oid GetRelationIdentityOrPK(Relation rel); #endif /* LOGICALRELATION_H */