diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b2f60c3d632..74b56ecd070 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -555,9 +555,8 @@ static ObjectAddress ATExecDropColumn(List **wqueue, Relation rel, const char *c bool recurse, bool recursing, bool missing_ok, LOCKMODE lockmode, ObjectAddresses *addrs); -static void ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd, - bool recurse, LOCKMODE lockmode, - AlterTableUtilityContext *context); +static void ATPrepAddPrimaryKey(AlteredTableInfo *tab, Relation rel, + AlterTableCmd *cmd, LOCKMODE lockmode); static void verifyNotNullPKCompatible(HeapTuple tuple, const char *colname); static ObjectAddress ATExecAddIndex(AlteredTableInfo *tab, Relation rel, IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode); @@ -5120,7 +5119,6 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, case AT_AddConstraint: /* ADD CONSTRAINT */ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE); - ATPrepAddPrimaryKey(wqueue, rel, cmd, recurse, lockmode, context); if (recurse) { /* recurses at exec time; lock descendants and set flag */ @@ -5368,6 +5366,19 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, lfirst_node(AlterTableCmd, lcmd), lockmode, pass, context); + /* + * For ADD PRIMARY KEY without USING INDEX, queue any required NOT + * NULL constraints after transforming all explicit constraints. + * This lets a named NOT NULL constraint take precedence + * regardless of subcommand order. USING INDEX is handled during + * transformation. + */ + if (pass == AT_PASS_ADD_CONSTR) + { + foreach_node(AlterTableCmd, cmd, subcmds) + ATPrepAddPrimaryKey(tab, tab->rel, cmd, lockmode); + } + /* * After the ALTER TYPE or SET EXPRESSION pass, do cleanup work * (this is not done in ATExecAlterColumnType since it should be @@ -9510,7 +9521,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, * hierarchy (failing to ensure that leads to funny corner cases). For the * normal case where we're asked to recurse, this routine checks if the * not-null constraints exist already, and if not queues a requirement for - * them to be created by phase 2. + * them to be created in the AT_PASS_COL_ATTRS pass. * * For the case where we're asked not to recurse, we verify that a not-null * constraint exists on each column of each (direct) child table, throwing an @@ -9525,16 +9536,17 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, * deadlocks during parallel pg_restore of PKs on partitioned tables. */ static void -ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd, - bool recurse, LOCKMODE lockmode, - AlterTableUtilityContext *context) +ATPrepAddPrimaryKey(AlteredTableInfo *tab, Relation rel, AlterTableCmd *cmd, + LOCKMODE lockmode) { Constraint *pkconstr; List *children = NIL; bool got_children = false; pkconstr = castNode(Constraint, cmd->def); - if (pkconstr->contype != CONSTR_PRIMARY) + + /* USING INDEX queues its own NOT NULL constraints during transformation */ + if (pkconstr->contype != CONSTR_PRIMARY || pkconstr->indexname != NULL) return; /* Verify that columns are not-null, or request that they be made so */ @@ -9558,7 +9570,7 @@ ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd, heap_freetuple(tuple); continue; } - else if (!recurse) + else if (!cmd->recurse) { /* * No constraint on this column. Asked not to recurse, we won't @@ -9595,7 +9607,8 @@ ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd, newcmd->recurse = true; newcmd->def = (Node *) nnconstr; - ATPrepCmd(wqueue, rel, newcmd, true, false, lockmode, context); + tab->subcmds[AT_PASS_COL_ATTRS] = + lappend(tab->subcmds[AT_PASS_COL_ATTRS], newcmd); } } diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index b891d68d4a7..96aa9b8e5fb 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1046,6 +1046,31 @@ ERROR: column "test2" of relation "atacc1" contains null values -- now add a primary key column with a default (succeeds). alter table atacc1 add column test2 int default 0 primary key; drop table atacc1; +-- Replacing a NOT NULL column must create a new constraint for the primary key. +create table atacc1 (id int not null); +alter table atacc1 drop column id, add column id int, add primary key (id); +drop table atacc1; +-- A NOT NULL constraint dropped in the same ALTER TABLE must be replaced. +create table atacc1 (id int not null); +alter table atacc1 alter column id drop not null, add primary key (id); +drop table atacc1; +create table atacc1 (id int constraint atacc1_id_nn not null); +alter table atacc1 drop constraint atacc1_id_nn, add primary key (id); +drop table atacc1; +-- ADD PRIMARY KEY must prefer an explicitly named replacement NOT NULL. +create table atacc1 (id int not null); +alter table atacc1 alter column id drop not null, + add primary key (id), add constraint atacc1_id_nn not null id; +drop table atacc1; +create table atacc1 (id int not null); +alter table atacc1 alter column id drop not null, + add constraint atacc1_id_nn not null id, add primary key (id); +drop table atacc1; +-- The replacement NOT NULL constraint must propagate to the partitions too. +create table atacc1 (id int not null) partition by range (id); +create table atacc1_1 partition of atacc1 for values from (0) to (10); +alter table atacc1 alter column id drop not null, add primary key (id); +drop table atacc1; -- this combination used to have order-of-execution problems (bug #15580) create table atacc1 (a int); insert into atacc1 values(1); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 9f6c2a4bb08..18f3fdac57a 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -782,6 +782,32 @@ alter table atacc1 add column test2 int primary key; alter table atacc1 add column test2 int default 0 primary key; drop table atacc1; +-- Replacing a NOT NULL column must create a new constraint for the primary key. +create table atacc1 (id int not null); +alter table atacc1 drop column id, add column id int, add primary key (id); +drop table atacc1; +-- A NOT NULL constraint dropped in the same ALTER TABLE must be replaced. +create table atacc1 (id int not null); +alter table atacc1 alter column id drop not null, add primary key (id); +drop table atacc1; +create table atacc1 (id int constraint atacc1_id_nn not null); +alter table atacc1 drop constraint atacc1_id_nn, add primary key (id); +drop table atacc1; +-- ADD PRIMARY KEY must prefer an explicitly named replacement NOT NULL. +create table atacc1 (id int not null); +alter table atacc1 alter column id drop not null, + add primary key (id), add constraint atacc1_id_nn not null id; +drop table atacc1; +create table atacc1 (id int not null); +alter table atacc1 alter column id drop not null, + add constraint atacc1_id_nn not null id, add primary key (id); +drop table atacc1; +-- The replacement NOT NULL constraint must propagate to the partitions too. +create table atacc1 (id int not null) partition by range (id); +create table atacc1_1 partition of atacc1 for values from (0) to (10); +alter table atacc1 alter column id drop not null, add primary key (id); +drop table atacc1; + -- this combination used to have order-of-execution problems (bug #15580) create table atacc1 (a int); insert into atacc1 values(1);