mirror of
https://github.com/postgres/postgres.git
synced 2026-10-23 13:29:09 +03:00
Use the relation map's index when searching deleted tuples sequentially.
When the index cannot 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 determine which columns to compare, which could differ from the index selected when the relation was opened. A concurrent DROP INDEX CONCURRENTLY could remove the replica identity in between, so the search compared the whole row and reported the conflict as update_deleted instead of update_missing. Use the index recorded in the relation map instead. The relation map already records the replica identity index, or the primary key if there is no replica identity. It also avoids using a deferrable primary key, which cannot serve as a replica identity; using its columns could incorrectly report a deleted row with different values as update_deleted instead of update_missing. Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Author: Nisha Moond <nisha.moond412@gmail.com> Reviewed-by: Vignesh C <vignesh21@gmail.com> Reviewed-by: Zhijie Hou <houzhijie22@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/CABdArM5ydwdRrpaZyK1q2p3-vY_+pnBtTmkvg_pcM=gHwmH7Kg@mail.gmail.com Discussion: https://postgr.es/m/OS7PR01MB1831779ED93F5470CA1D57605F58C2@OS7PR01MB18317.jpnprd01.prod.outlook.com Backpatch-through: 19
This commit is contained in:
@@ -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);
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
/*
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user