diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 790c09c522..d2b15a3387 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -4513,9 +4513,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, lockmode); break; case AT_AddConstraint: /* ADD CONSTRAINT */ - cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode, - cur_pass, context); - /* Might not have gotten AddConstraint back from parse transform */ + /* Transform the command only during initial examination */ + if (cur_pass == AT_PASS_ADD_CONSTR) + cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, + false, lockmode, + cur_pass, context); + /* Depending on constraint type, might be no more work to do now */ if (cmd != NULL) address = ATExecAddConstraint(wqueue, tab, rel, @@ -4523,9 +4526,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, false, false, lockmode); break; case AT_AddConstraintRecurse: /* ADD CONSTRAINT with recursion */ - cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, true, lockmode, - cur_pass, context); - /* Might not have gotten AddConstraint back from parse transform */ + /* Transform the command only during initial examination */ + if (cur_pass == AT_PASS_ADD_CONSTR) + cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, + true, lockmode, + cur_pass, context); + /* Depending on constraint type, might be no more work to do now */ if (cmd != NULL) address = ATExecAddConstraint(wqueue, tab, rel, @@ -4787,75 +4793,88 @@ ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, foreach(lc, atstmt->cmds) { AlterTableCmd *cmd2 = lfirst_node(AlterTableCmd, lc); + int pass; - if (newcmd == NULL && - (cmd->subtype == cmd2->subtype || - (cmd->subtype == AT_AddConstraintRecurse && - cmd2->subtype == AT_AddConstraint))) + /* + * This switch need only cover the subcommand types that can be added + * by parse_utilcmd.c; otherwise, we'll use the default strategy of + * executing the subcommand immediately, as a substitute for the + * original subcommand. (Note, however, that this does cause + * AT_AddConstraint subcommands to be rescheduled into later passes, + * which is important for index and foreign key constraints.) + * + * We assume we needn't do any phase-1 checks for added subcommands. + */ + switch (cmd2->subtype) { - /* Found the transformed version of our subcommand */ - cmd2->subtype = cmd->subtype; /* copy recursion flag */ - newcmd = cmd2; + case AT_SetNotNull: + /* Need command-specific recursion decision */ + ATPrepSetNotNull(wqueue, rel, cmd2, + recurse, false, + lockmode, context); + pass = AT_PASS_COL_ATTRS; + break; + case AT_AddIndex: + /* This command never recurses */ + /* No command-specific prep needed */ + pass = AT_PASS_ADD_INDEX; + break; + case AT_AddIndexConstraint: + /* This command never recurses */ + /* No command-specific prep needed */ + pass = AT_PASS_ADD_INDEXCONSTR; + break; + case AT_AddConstraint: + /* Recursion occurs during execution phase */ + if (recurse) + cmd2->subtype = AT_AddConstraintRecurse; + switch (castNode(Constraint, cmd2->def)->contype) + { + case CONSTR_PRIMARY: + case CONSTR_UNIQUE: + case CONSTR_EXCLUSION: + pass = AT_PASS_ADD_INDEXCONSTR; + break; + default: + pass = AT_PASS_ADD_OTHERCONSTR; + break; + } + break; + case AT_AlterColumnGenericOptions: + /* This command never recurses */ + /* No command-specific prep needed */ + pass = AT_PASS_MISC; + break; + default: + pass = cur_pass; + break; + } + + if (pass < cur_pass) + { + /* Cannot schedule into a pass we already finished */ + elog(ERROR, "ALTER TABLE scheduling failure: too late for pass %d", + pass); + } + else if (pass > cur_pass) + { + /* OK, queue it up for later */ + tab->subcmds[pass] = lappend(tab->subcmds[pass], cmd2); } else { - int pass; - /* - * Schedule added subcommand appropriately. We assume we needn't - * do any phase-1 checks for it. This switch only has to cover - * the subcommand types that can be added by parse_utilcmd.c. + * We should see at most one subcommand for the current pass, + * which is the transformed version of the original subcommand. */ - switch (cmd2->subtype) + if (newcmd == NULL && cmd->subtype == cmd2->subtype) { - case AT_SetNotNull: - /* Need command-specific recursion decision */ - ATPrepSetNotNull(wqueue, rel, cmd2, - recurse, false, - lockmode, context); - pass = AT_PASS_COL_ATTRS; - break; - case AT_AddIndex: - /* This command never recurses */ - /* No command-specific prep needed */ - pass = AT_PASS_ADD_INDEX; - break; - case AT_AddIndexConstraint: - /* This command never recurses */ - /* No command-specific prep needed */ - pass = AT_PASS_ADD_INDEXCONSTR; - break; - case AT_AddConstraint: - /* Recursion occurs during execution phase */ - if (recurse) - cmd2->subtype = AT_AddConstraintRecurse; - switch (castNode(Constraint, cmd2->def)->contype) - { - case CONSTR_PRIMARY: - case CONSTR_UNIQUE: - case CONSTR_EXCLUSION: - pass = AT_PASS_ADD_INDEXCONSTR; - break; - default: - pass = AT_PASS_ADD_OTHERCONSTR; - break; - } - break; - case AT_AlterColumnGenericOptions: - /* This command never recurses */ - /* No command-specific prep needed */ - pass = AT_PASS_MISC; - break; - default: - elog(ERROR, "unexpected AlterTableType: %d", - (int) cmd2->subtype); - pass = AT_PASS_UNSET; - break; + /* Found the transformed version of our subcommand */ + newcmd = cmd2; } - /* Must be for a later pass than we're currently doing */ - if (pass <= cur_pass) - elog(ERROR, "ALTER TABLE scheduling failure"); - tab->subcmds[pass] = lappend(tab->subcmds[pass], cmd2); + else + elog(ERROR, "ALTER TABLE scheduling failure: bogus item for pass %d", + pass); } } diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 6f90eae2f8..f56615393e 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -3678,6 +3678,42 @@ ALTER TABLE ataddindex Indexes: "ataddindex_expr_excl" EXCLUDE USING btree ((f1 ~~ 'a'::text) WITH =) +DROP TABLE ataddindex; +CREATE TABLE ataddindex(id int, ref_id int); +ALTER TABLE ataddindex + ADD PRIMARY KEY (id), + ADD FOREIGN KEY (ref_id) REFERENCES ataddindex; +\d ataddindex + Table "public.ataddindex" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + ref_id | integer | | | +Indexes: + "ataddindex_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id) +Referenced by: + TABLE "ataddindex" CONSTRAINT "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id) + +DROP TABLE ataddindex; +CREATE TABLE ataddindex(id int, ref_id int); +ALTER TABLE ataddindex + ADD UNIQUE (id), + ADD FOREIGN KEY (ref_id) REFERENCES ataddindex (id); +\d ataddindex + Table "public.ataddindex" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | | + ref_id | integer | | | +Indexes: + "ataddindex_id_key" UNIQUE CONSTRAINT, btree (id) +Foreign-key constraints: + "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id) +Referenced by: + TABLE "ataddindex" CONSTRAINT "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id) + DROP TABLE ataddindex; -- unsupported constraint types for partitioned tables CREATE TABLE partitioned ( diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index ce6401d80d..4cc55d8525 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -2252,6 +2252,20 @@ ALTER TABLE ataddindex \d ataddindex DROP TABLE ataddindex; +CREATE TABLE ataddindex(id int, ref_id int); +ALTER TABLE ataddindex + ADD PRIMARY KEY (id), + ADD FOREIGN KEY (ref_id) REFERENCES ataddindex; +\d ataddindex +DROP TABLE ataddindex; + +CREATE TABLE ataddindex(id int, ref_id int); +ALTER TABLE ataddindex + ADD UNIQUE (id), + ADD FOREIGN KEY (ref_id) REFERENCES ataddindex (id); +\d ataddindex +DROP TABLE ataddindex; + -- unsupported constraint types for partitioned tables CREATE TABLE partitioned ( a int,