database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-03 20:02:46 +03:00
Code Activity

Fix some more bugs in foreign keys connecting partitioned tables

Browse Source 
* In DetachPartitionFinalize() we were applying a tuple conversion map
  to tuples that didn't need one, which can lead to erratic behavior if
  a partitioned table has a partition with a different column order, as
  reported by Alexander Lakhin. This was introduced by 53af9491a0.
  Don't do that.  Also, modify a recently added test case to exercise
  this.

* The same function as well as CloneFkReferenced() were acquiring
  AccessShareLock on a partition, only to have CreateTrigger() later
  acquire ShareRowExclusiveLock on it.  This can lead to deadlock by
  lock escalation, unnecessarily.  Avoid that by acquiring the stronger
  lock to begin with.  This probably dates back to branch 12, but I have
  never seen a report of this being a problem in the field.

* Innocuous but wasteful: also introduced by 53af9491a0, we were
  reading a pg_constraint tuple from syscache that we don't need, as
  reported by Tender Wang.  Don't.

Backpatch to 15.

Discussion: https://postgr.es/m/461e9c26-2076-8224-e119-84998b6a784e@gmail.com
This commit is contained in:
Álvaro Herrera
2024-10-30 10:54:03 +01:00
parent 2845cd1ca0
commit 2d5fe51405
3 changed files with 27 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
src/backend/commands/tablecmds.c
View File

@ -10372,6 +10372,9 @@ addFkRecurseReferenced(Constraint *fkconstraint, Relation rel,
Oid deleteTriggerOid,
updateTriggerOid;
Assert(CheckRelationLockedByMe(pkrel, ShareRowExclusiveLock, true));
Assert(CheckRelationLockedByMe(rel, ShareRowExclusiveLock, true));
/*
* Create the action triggers that enforce the constraint.
*/
@ -10398,6 +10401,7 @@ addFkRecurseReferenced(Constraint *fkconstraint, Relation rel,
Oid partIndexId;
ObjectAddress address;
/* XXX would it be better to acquire these locks beforehand? */
partRel = table_open(pd->oids[i], ShareRowExclusiveLock);
/*
@ -10503,6 +10507,8 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
updateTriggerOid;
Assert(OidIsValid(parentConstr));
Assert(CheckRelationLockedByMe(rel, ShareRowExclusiveLock, true));
Assert(CheckRelationLockedByMe(pkrel, ShareRowExclusiveLock, true));
if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
ereport(ERROR,
@ -10796,13 +10802,8 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
continue;
}
/*
* Because we're only expanding the key space at the referenced side,
* we don't need to prevent any operation in the referencing table, so
* AccessShareLock suffices (assumes that dropping the constraint
* acquires AccessExclusiveLock).
*/
fkRel = table_open(constrForm->conrelid, AccessShareLock);
/* We need the same lock level that CreateTrigger will acquire */
fkRel = table_open(constrForm->conrelid, ShareRowExclusiveLock);
indexOid = constrForm->conindid;
DeconstructFkConstraintRow(tuple,
@ -19436,8 +19437,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
foreach(cell, fks)
{
ForeignKeyCacheInfo *fk = lfirst(cell);
HeapTuple contup,
parentConTup;
HeapTuple contup;
Form_pg_constraint conform;
Oid insertTriggerOid,
updateTriggerOid;
@ -19455,13 +19455,6 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
continue;
}
Assert(OidIsValid(conform->conparentid));
parentConTup = SearchSysCache1(CONSTROID,
ObjectIdGetDatum(conform->conparentid));
if (!HeapTupleIsValid(parentConTup))
elog(ERROR, "cache lookup failed for constraint %u",
conform->conparentid);
/*
* The constraint on this table must be marked no longer a child of
* the parent's constraint, as do its check triggers.
@ -19502,7 +19495,6 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
Oid conffeqop[INDEX_MAX_KEYS];
int numfkdelsetcols;
AttrNumber confdelsetcols[INDEX_MAX_KEYS];
AttrMap *attmap;
Relation refdRel;
DeconstructFkConstraintRow(contup,
@ -19535,20 +19527,19 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
fkconstraint->old_pktable_oid = InvalidOid;
fkconstraint->location = -1;
attmap = build_attrmap_by_name(RelationGetDescr(partRel),
RelationGetDescr(rel),
false);
/* set up colnames, used to generate the constraint name */
for (int i = 0; i < numfks; i++)
{
Form_pg_attribute att;
att = TupleDescAttr(RelationGetDescr(partRel),
attmap->attnums[conkey[i] - 1] - 1);
conkey[i] - 1);
fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs,
makeString(NameStr(att->attname)));
}
refdRel = table_open(fk->confrelid, AccessShareLock);
refdRel = table_open(fk->confrelid, ShareRowExclusiveLock);
addFkRecurseReferenced(fkconstraint, partRel,
refdRel,
@ -19565,11 +19556,10 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
true,
InvalidOid, InvalidOid,
conform->conperiod);
table_close(refdRel, AccessShareLock);
table_close(refdRel, NoLock); /* keep lock till end of xact */
}
ReleaseSysCache(contup);
ReleaseSysCache(parentConTup);
}
list_free_deep(fks);
if (trigrel)