From 06dfb424266230cc04f1cbb5c891817e59ee2153 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Tue, 20 Jan 2026 09:57:24 +0800 Subject: [PATCH v2 1/2] tablecmds: clarify recurse/recursing semantics in ALTER TABLE code Add clearer documentation of the recurse and recursing flags used by ALTER TABLE processing. Centralize the explanation in ATPrepCmd(), add a brief description in ATController(), and replace repeated local comments with references to the central explanation. No functional changes; documentation and comments only. Author: Chao Li --- src/backend/commands/tablecmds.c | 60 +++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f976c0e5c7e..5194ffc84bb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -4518,11 +4518,10 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode) * * The caller must lock the relation, with an appropriate lock level * for the subcommands requested, using AlterTableGetLockLevel(stmt->cmds) - * or higher. We pass the lock level down - * so that we can apply it recursively to inherited tables. Note that the - * lock level we want as we recurse might well be higher than required for - * that specific subcommand. So we pass down the overall lock requirement, - * rather than reassess it at lower levels. + * or higher. We pass the lock level down so that we can apply it recursively + * to inherited tables. Note that the lock level we want as we recurse might + * well be higher than required for that specific subcommand. So we pass down + * the overall lock requirement, rather than reassess it at lower levels. * * The caller also provides a "context" which is to be passed back to * utility.c when we need to execute a subcommand such as CREATE INDEX. @@ -4868,6 +4867,11 @@ AlterTableGetLockLevel(List *cmds) * * parsetree is passed in to allow it to be passed to event triggers * when requested. + * + * recurse indicates whether the ALTER TABLE command should recurse to + * child tables. At the top level this usually corresponds to the absence + * of ONLY in the command, as determined in AlterTable(). The value is + * passed down to lower-level helpers such as ATPrepCmd(). */ static void ATController(AlterTableStmt *parsetree, @@ -4903,6 +4907,20 @@ ATController(AlterTableStmt *parsetree, * * Caller must have acquired appropriate lock type on relation already. * This lock should be held until commit. + * + * The recurse argument tells whether we should recurse to child tables + * if the command type supports that. The recursing argument tells whether + * we are being called as part of such a recursion. At a top-level call, + * recurse generally reflects that the user did not specify ONLY in the + * command, subject to the command's recursion semantics. + * + * recurse=false, recursing=false: top-level call, no recursion + * recurse=true, recursing=false: top-level call, recurse if supported + * recurse=true, recursing=true: recursive call, continue recursion + * recurse=false, recursing=true: recursive call, stop recursion + * + * "Children" or "child tables" mean inheritance children, partitions, or both, + * depending on the command type. */ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, @@ -7217,6 +7235,8 @@ check_of_type(HeapTuple typetuple) * actually add a column or merely merge with an existing column. (We can't * check this in a static pre-pass because it won't handle multiple inheritance * situations correctly.) + * + * recurse / recursing semantics: see ATPrepCmd() */ static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing, @@ -7241,6 +7261,8 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing, * * cmd is pass-by-ref so that we can replace it with the parse-transformed * copy (but that happens only after we check for IF NOT EXISTS). + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, @@ -7937,6 +7959,8 @@ set_attnotnull(List **wqueue, Relation rel, AttrNumber attnum, * * We must recurse to child tables during execution, rather than using * ALTER TABLE's normal prep-time recursion. + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName, @@ -8264,6 +8288,8 @@ ATExecCookedColumnDefault(Relation rel, AttrNumber attnum, * ALTER TABLE ALTER COLUMN ADD IDENTITY * * Return the address of the affected column. + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName, @@ -8395,6 +8421,8 @@ ATExecAddIdentity(Relation rel, const char *colName, * ALTER TABLE ALTER COLUMN SET { GENERATED or sequence options } * * Return the address of the affected column. + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress ATExecSetIdentity(Relation rel, const char *colName, Node *def, @@ -8512,6 +8540,8 @@ ATExecSetIdentity(Relation rel, const char *colName, Node *def, * ALTER TABLE ALTER COLUMN DROP IDENTITY * * Return the address of the affected column. + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode, @@ -8780,6 +8810,8 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName, /* * ALTER TABLE ALTER COLUMN DROP EXPRESSION + * + * recurse / recursing semantics: see ATPrepCmd() */ static void ATPrepDropExpression(Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, LOCKMODE lockmode) @@ -9280,6 +9312,8 @@ ATExecSetStorage(Relation rel, const char *colName, Node *newValue, LOCKMODE loc * on whether attinhcount goes to zero or not. (We can't check this in a * static pre-pass because it won't handle multiple inheritance situations * correctly.) + * + * recurse / recursing semantics: see ATPrepCmd() */ static void ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing, @@ -9308,6 +9342,8 @@ ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing, * is added to 'addrs', which must be non-NULL in such invocations. All * columns are dropped at the same time after all the children have been * checked recursively. + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress ATExecDropColumn(List **wqueue, Relation rel, const char *colName, @@ -9937,6 +9973,8 @@ ChooseForeignKeyConstraintNameAddition(List *colnames) * AddRelationNewConstraints would normally assign different names to the * child constraints. To fix that, we must capture the name assigned at * the parent table and pass that down. + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, @@ -10092,6 +10130,8 @@ ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * thereof). We also need action triggers on each leaf partition on the * referenced side, and check triggers on each leaf partition on the * referencing side. + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, @@ -12933,6 +12973,8 @@ AlterConstrUpdateConstraintEntry(ATAlterConstraint *cmdcon, Relation conrel, * * Return value is the address of the validated constraint. If the constraint * was already validated, InvalidObjectAddress is returned. + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName, @@ -13142,6 +13184,8 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation fkrel, * Add an entry to the wqueue to validate the given check constraint in Phase 3 * and update the convalidated field in the pg_constraint catalog for the * specified relation and all its inheriting children. + * + * recurse / recursing semantics: see ATPrepCmd() */ static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel, @@ -13245,6 +13289,8 @@ QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel, * Add an entry to the wqueue to validate the given not-null constraint in * Phase 3 and update the convalidated field in the pg_constraint catalog for * the specified relation and all its inheriting children. + * + * recurse / recursing semantics: see ATPrepCmd() */ static void QueueNNConstraintValidation(List **wqueue, Relation conrel, Relation rel, @@ -14102,6 +14148,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, * DROP NOT NULL. * * Returns the address of the constraint being removed. + * + * recurse / recursing semantics: see ATPrepCmd() */ static ObjectAddress dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior behavior, @@ -14398,6 +14446,8 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha * the first two execution steps in phase 2; they must not see the effects * of any other subcommand types, since the USING expressions are parsed * against the unmodified table's state. + * + * recurse / recursing semantics: see ATPrepCmd() */ static void ATPrepAlterColumnType(List **wqueue, -- 2.39.5 (Apple Git-154)