From 35b485b72d0675d631cde9f95e65d9c0db9254b8 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 22 Apr 2024 11:32:04 +0200 Subject: [PATCH] Acquire locks on children before recursing ALTER TABLE ADD CONSTRAINT was missing this, as evidenced by assertion failures. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/5b4cd32f-1d5b-c080-c688-31736bbcd739@gmail.com --- src/backend/commands/tablecmds.c | 33 ++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3556240c8e..8941181912 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -427,6 +427,7 @@ static void ATSimplePermissions(AlterTableType cmdtype, Relation rel, int allowe static void ATSimpleRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode, AlterTableUtilityContext *context); +static void ATLockAllDescendants(Oid relid, LOCKMODE lockmode); static void ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode); static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode, @@ -1621,9 +1622,7 @@ RemoveRelations(DropStmt *drop) * will lock those objects in the other order. */ if (state.actual_relkind == RELKIND_PARTITIONED_INDEX) - (void) find_all_inheritors(state.heapOid, - state.heap_lockmode, - NULL); + ATLockAllDescendants(state.heapOid, state.heap_lockmode); /* OK, we're ready to delete this one */ obj.classId = RelationRelationId; @@ -4979,10 +4978,15 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, break; case AT_AddConstraint: /* ADD CONSTRAINT */ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE); - /* Recursion occurs during execution phase */ - /* No command-specific prep needed except saving recurse flag */ if (recurse) + { + /* + * Make note for execution phase about need for recursion; + * also acquire lock on all descendants. + */ + ATLockAllDescendants(RelationGetRelid(rel), lockmode); cmd->recurse = true; + } pass = AT_PASS_ADD_CONSTR; break; case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */ @@ -6721,6 +6725,17 @@ ATSimpleRecursion(List **wqueue, Relation rel, } } +/* + * ATLockAllDescendants + * + * Acquire lock on all descendants of the given relation. + */ +static void +ATLockAllDescendants(Oid relid, LOCKMODE lockmode) +{ + (void) find_all_inheritors(relid, lockmode, NULL); +} + /* * Obtain list of partitions of the given table, locking them all at the given * lockmode and ensuring that they all pass CheckTableNotInUse. @@ -9370,10 +9385,9 @@ ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd, /* * Acquire locks all the way down the hierarchy. The recursion to lower - * levels occurs at execution time as necessary, so we don't need to do it - * here, and we don't need the returned list either. + * levels occurs at execution time as necessary. */ - (void) find_all_inheritors(RelationGetRelid(rel), lockmode, NULL); + ATLockAllDescendants(RelationGetRelid(rel), lockmode); /* * Construct the list of constraints that we need to add to each child @@ -11258,8 +11272,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) */ pkrel = table_open(constrForm->confrelid, ShareRowExclusiveLock); if (pkrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - (void) find_all_inheritors(RelationGetRelid(pkrel), - ShareRowExclusiveLock, NULL); + ATLockAllDescendants(RelationGetRelid(pkrel), ShareRowExclusiveLock); DeconstructFkConstraintRow(tuple, &numfks, conkey, confkey, conpfeqop, conppeqop, conffeqop, -- 2.39.2