diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index dd42acc13e2..18da4fe08f6 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 is 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). + * If the caller's replica identity key or primary key 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). */ - 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..44d735cdc55 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -3409,9 +3409,12 @@ FindDeletedTupleInLocalRel(Relation localrel, delete_xid, delete_origin, delete_time); else - return RelationFindDeletedTupleInfoSeq(localrel, remoteslot, - oldestxmin, delete_xid, - delete_origin, delete_time); + 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, diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl index 5804e38ed69..80177b214c0 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -376,6 +376,69 @@ like( .*The row to be updated was deleted locally in transaction [0-9]+ at .*/, 'update target row was deleted in tab'); +############################################################################### +# Ensure that a deferrable primary key is not used to match deleted tuples in +# a sequential table scan. Such a key cannot serve as a replica identity, so +# the whole tuple must be compared, and a deleted row that only shares the key +# value must not be reported as update_deleted. +############################################################################### + +# Create the table and publish it from node B only, so that local changes on +# node A are not sent back. Skip the initial copy, so that node A never has +# the row from node B. +$node_B->safe_psql( + 'postgres', " + CREATE TABLE tab_defer (a int, b int); + ALTER TABLE tab_defer REPLICA IDENTITY FULL; + INSERT INTO tab_defer VALUES (1, 1);"); +$node_A->safe_psql('postgres', "CREATE TABLE tab_defer (a int, b int)"); +$node_B->safe_psql('postgres', + "ALTER PUBLICATION tap_pub_B ADD TABLE tab_defer"); +$node_A->safe_psql('postgres', + "ALTER SUBSCRIPTION $subname_AB REFRESH PUBLICATION WITH (copy_data = false)" +); +$node_A->wait_for_subscription_sync($node_B, $subname_AB); + +# Disable the logical replication from node B to node A +$node_A->safe_psql('postgres', "ALTER SUBSCRIPTION $subname_AB DISABLE"); + +# Wait for the apply worker to stop +$node_A->poll_query_until('postgres', + "SELECT count(*) = 0 FROM pg_stat_activity WHERE backend_type = 'logical replication apply worker'" +); + +# The primary key is created after the conflict detection slot's xmin, so it +# cannot be used to find deleted tuples and a sequential scan is used instead. +# Then delete a local row that has the same key but a different value. +$node_A->safe_psql( + 'postgres', " + ALTER TABLE tab_defer ADD PRIMARY KEY (a) DEFERRABLE; + INSERT INTO tab_defer VALUES (1, 10); + DELETE FROM tab_defer WHERE a = 1;"); + +$node_B->safe_psql('postgres', "UPDATE tab_defer SET b = 2 WHERE a = 1;"); + +$log_location = -s $node_A->logfile; + +$node_A->safe_psql('postgres', "ALTER SUBSCRIPTION $subname_AB ENABLE;"); +$node_B->wait_for_catchup($subname_AB); + +$logfile = slurp_file($node_A->logfile(), $log_location); +like( + $logfile, + qr/conflict detected on relation "public.tab_defer": conflict=update_missing.* +.*DETAIL:.* Could not find the row to be updated: remote row \(1, 2\), replica identity full \(1, 1\)/, + 'deleted row matching only the deferrable primary key is not reported as update_deleted' +); + +# Clean up +$node_B->safe_psql('postgres', + "ALTER PUBLICATION tap_pub_B DROP TABLE tab_defer"); +$node_A->safe_psql('postgres', + "ALTER SUBSCRIPTION $subname_AB REFRESH PUBLICATION"); +$node_A->safe_psql('postgres', "DROP TABLE tab_defer"); +$node_B->safe_psql('postgres', "DROP TABLE tab_defer"); + ############################################################################### # Check that the xmin value of the conflict detection slot can be advanced when # the subscription has no tables.