pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feed42P16 error when dropping and adding column
9+ messages / 4 participants
[nested] [flat]
* 42P16 error when dropping and adding column
@ 2026-09-17 09:51 Ludvig Janiuk <ludvig.janiuk@proton.me>
0 siblings, 1 reply; 9+ messages in thread
From: Ludvig Janiuk @ 2026-09-17 09:51 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
Hello, it seems that between pqsl 17 and 18, the behavior of the
following reproducer has changed:
====
CREATE TABLE reproducer(id int NOT NULL); ALTER TABLE reproducer DROP
COLUMN "id", ADD COLUMN "id" int, ADD PRIMARY KEY("id");
====
Expected output (this is the case on psql 16 and 17):
====
CREATE TABLE
ALTER TABLE
====
Actual output (on psql (PostgreSQL) 18.6 (Ubuntu 18.6-0ubuntu0.26.04.1)):
====
CREATE TABLE
ERROR: 42P16: primary key column "id" is not marked NOT NULL
LOCATION: index_check_primary_key, index.c:266
====
I could not find anything direcly related in the change notes
(https://www.postgresql.org/docs/release/18.0/). There were a few lines
related to NULL, but with my knowledge I could not be sure if they were
relevant.
I hope you can accept my bug report, or let me know the reason why if
this is correct behavior.
Best regards,
--
//Ludvig Janiuk
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: 42P16 error when dropping and adding column
@ 2026-09-17 14:22 Fujii Masao <masao.fujii@gmail.com>
parent: Ludvig Janiuk <ludvig.janiuk@proton.me>
0 siblings, 1 reply; 9+ messages in thread
From: Fujii Masao @ 2026-09-17 14:22 UTC (permalink / raw)
To: Ludvig Janiuk <ludvig.janiuk@proton.me>; +Cc: pgsql-bugs@lists.postgresql.org
On Thu, Sep 17, 2026 at 7:23 PM Ludvig Janiuk <ludvig.janiuk@proton.me> wrote:
>
> Hello, it seems that between pqsl 17 and 18, the behavior of the
> following reproducer has changed:
>
> ====
> CREATE TABLE reproducer(id int NOT NULL); ALTER TABLE reproducer DROP
> COLUMN "id", ADD COLUMN "id" int, ADD PRIMARY KEY("id");
> ====
>
> Expected output (this is the case on psql 16 and 17):
>
> ====
> CREATE TABLE
> ALTER TABLE
> ====
>
> Actual output (on psql (PostgreSQL) 18.6 (Ubuntu 18.6-0ubuntu0.26.04.1)):
>
> ====
> CREATE TABLE
> ERROR: 42P16: primary key column "id" is not marked NOT NULL
> LOCATION: index_check_primary_key, index.c:266
> ====
Thanks for the report!
This issue seems to happen because ALTER TABLE checks for an existing NOT
NULL constraint before dropping the old column. So, finding the NOT NULL
constraint on that old column causes it to skip adding one for the new
column, even though the old constraint will be removed with the old column.
The attached patch fixes this by deferring the check until after column
drops and additions have been executed. This prevents the old column's NOT
NULL constraint from being found and ensures that required NOT NULL
constraints are added to the new columns before creating the primary-key
index.
Regards,
--
Fujii Masao
Attachments:
[application/octet-stream] v1-0001-Fix-ADD-PRIMARY-KEY-after-dropping-and-re-adding-.patch (6.1K, ../../CAHGQGwFmL6L2OPvPMCkqwrRX7QftuCiJ7RVEb8awDc26b2aVjQ@mail.gmail.com/2-v1-0001-Fix-ADD-PRIMARY-KEY-after-dropping-and-re-adding-.patch)
download | inline diff:
From c67215912e973f97f91b3fa19b111a4a0696e7ea Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Thu, 17 Sep 2026 21:35:15 +0900
Subject: [PATCH v1] Fix ADD PRIMARY KEY after dropping and re-adding a column
Previously, dropping a NOT NULL column, adding a new column with the same
name, and making it a primary key in a single ALTER TABLE command could
fail with "primary key column ... is not marked NOT NULL". This happened
because ALTER TABLE checked for an existing NOT NULL constraint before
dropping the old column. Finding the constraint caused it to skip adding
one for the new column, even though the old constraint would be removed
with the old column.
Defer this check until after column drops and additions have been
executed, so that it examines the columns that will actually form the
primary key. Add any required NOT NULL constraints before creating the
primary-key index.
Backpatch to v18, where this issue was introduced by commit a379061a22a.
---
src/backend/commands/tablecmds.c | 21 +++++++++++----------
src/test/regress/expected/alter_table.out | 4 ++++
src/test/regress/sql/alter_table.sql | 5 +++++
3 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2f073ddb84a..99384951864 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);
@@ -5146,7 +5145,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 */
@@ -5518,9 +5516,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
case AT_AddConstraint: /* ADD CONSTRAINT */
/* Transform the command only during initial examination */
if (cur_pass == AT_PASS_ADD_CONSTR)
+ {
+ ATPrepAddPrimaryKey(tab, rel, cmd, lockmode);
cmd = ATParseTransformCmd(wqueue, tab, rel, cmd,
cmd->recurse, lockmode,
cur_pass, context);
+ }
/* Depending on constraint type, might be no more work to do now */
if (cmd != NULL)
address =
@@ -9552,7 +9553,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
@@ -9567,9 +9568,8 @@ 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;
@@ -9600,7 +9600,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
@@ -9637,7 +9637,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 e167a41ce79..5528749b953 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1046,6 +1046,10 @@ 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;
-- 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..6e28b5539eb 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -782,6 +782,11 @@ 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;
+
-- this combination used to have order-of-execution problems (bug #15580)
create table atacc1 (a int);
insert into atacc1 values(1);
--
2.55.0
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: 42P16 error when dropping and adding column
@ 2026-09-17 15:01 Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
parent: Fujii Masao <masao.fujii@gmail.com>
0 siblings, 2 replies; 9+ messages in thread
From: Manuel Reyes Bravo @ 2026-09-17 15:01 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Ludvig Janiuk <ludvig.janiuk@proton.me>; pgsql-bugs@lists.postgresql.org
Hi Fujii-san,
Fujii Masao <masao.fujii@gmail.com> wrote:
> This issue seems to happen because ALTER TABLE checks for an existing NOT
> NULL constraint before dropping the old column. So, finding the NOT NULL
> constraint on that old column causes it to skip adding one for the new
> column, even though the old constraint will be removed with the old column.
The same early check also breaks commands that do not drop the column at
all, only its not-null constraint, and v1 fixes those too. On 18.6 each
of these fails with "primary key column "a" is not marked NOT NULL":
CREATE TABLE t (a int NOT NULL);
ALTER TABLE t ALTER COLUMN a DROP NOT NULL, ADD PRIMARY KEY (a);
CREATE TABLE t (a int CONSTRAINT a_nn NOT NULL);
ALTER TABLE t DROP CONSTRAINT a_nn, ADD PRIMARY KEY (a);
CREATE TABLE p (a int NOT NULL) PARTITION BY RANGE (a);
CREATE TABLE p1 PARTITION OF p FOR VALUES FROM (0) TO (10);
ALTER TABLE p ALTER COLUMN a DROP NOT NULL, ADD PRIMARY KEY (a);
The two DROP NOT NULL forms work on 17.11; the DROP CONSTRAINT form has
no equivalent there.
To see what else v1 changes, I ran 24 ALTER TABLE ... ADD PRIMARY KEY
scenarios on REL_18_STABLE at 459ef520d8f and master at c9c660e6ae0,
with and without v1, and compared the errors and the resulting catalog
state: the pg_constraint rows (with conislocal, coninhcount,
convalidated and connoinherit) and attnotnull. scenarios.sql is
attached.
v1 changes exactly nine of them, all failures that go away:
* dropping and re-adding the column: the reported case, a two-column
key with one column replaced, a partitioned table, an inheritance
parent, and the PRIMARY KEY subcommand written before the DROP
COLUMN;
* the three commands above;
* the reported case with a row in the table, which now fails with
"column "id" of relation "t" contains null values", as on 17.
The other fifteen give identical errors and identical catalog state with
and without v1. Among them are plain ADD PRIMARY KEY on a partitioned
table and on an inheritance parent, ONLY on a parent whose child has or
lacks the not-null, an existing NOT VALID or NO INHERIT not-null, USING
INDEX, SET NOT NULL in the same command, an identity column, and ALTER
COLUMN TYPE in the same command. The results are the same on both
branches.
Building a379061a22a and its parent: before it, all nine behave as they
do with v1; after it, all nine fail with "is not marked NOT NULL". So
the commit named in the patch is the one.
v1's test covers the DROP COLUMN form only. The attached top-up adds
the three commands above to the same place in alter_table.sql. With v1
make check passes on master and REL_18_STABLE; with the tests but
without the tablecmds.c change, alter_table fails on all four.
Regards,
Manu
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 5528749b953..7a0c8d9e033 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1050,6 +1050,17 @@ drop table atacc1;
create table atacc1 (id int not null);
alter table atacc1 drop column id, add column id int, add primary key (id);
drop table atacc1;
+-- Likewise when an earlier subcommand drops the not-null constraint itself.
+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;
+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 6e28b5539eb..0d4ba7dad26 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -786,6 +786,17 @@ drop table atacc1;
create table atacc1 (id int not null);
alter table atacc1 drop column id, add column id int, add primary key (id);
drop table atacc1;
+-- Likewise when an earlier subcommand drops the not-null constraint itself.
+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;
+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);
Attachments:
[text/plain] v1-topup-tests.txt (2.2K, ../../CA+bCEdApL1=WLxpRAOXDqBAF5WUbgvHQDSfP33NFRo+vgfB49g@mail.gmail.com/2-v1-topup-tests.txt)
download | inline diff:
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 5528749b953..7a0c8d9e033 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1050,6 +1050,17 @@ drop table atacc1;
create table atacc1 (id int not null);
alter table atacc1 drop column id, add column id int, add primary key (id);
drop table atacc1;
+-- Likewise when an earlier subcommand drops the not-null constraint itself.
+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;
+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 6e28b5539eb..0d4ba7dad26 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -786,6 +786,17 @@ drop table atacc1;
create table atacc1 (id int not null);
alter table atacc1 drop column id, add column id int, add primary key (id);
drop table atacc1;
+-- Likewise when an earlier subcommand drops the not-null constraint itself.
+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;
+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);
[application/sql] scenarios.sql (6.7K, ../../CA+bCEdApL1=WLxpRAOXDqBAF5WUbgvHQDSfP33NFRo+vgfB49g@mail.gmail.com/3-scenarios.sql)
download
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: 42P16 error when dropping and adding column
@ 2026-09-24 14:09 Ludvig Janiuk <ludvig.janiuk@proton.me>
parent: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
1 sibling, 1 reply; 9+ messages in thread
From: Ludvig Janiuk @ 2026-09-24 14:09 UTC (permalink / raw)
To: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>; Fujii Masao <masao.fujii@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
Thank you both for responding so quickly and reviewing my report.
I'm not familiar with the Postgres bug lifecycle - is there anything
else I need to do? File a formal bug in some tracker? Or has that
already happened?
Best regards,
Ludvig Janiuk
On 2026-09-17 17:01, Manuel Reyes Bravo wrote:
> Hi Fujii-san,
>
> Fujii Masao <masao.fujii@gmail.com> wrote:
>> This issue seems to happen because ALTER TABLE checks for an existing NOT
>> NULL constraint before dropping the old column. So, finding the NOT NULL
>> constraint on that old column causes it to skip adding one for the new
>> column, even though the old constraint will be removed with the old column.
> The same early check also breaks commands that do not drop the column at
> all, only its not-null constraint, and v1 fixes those too. On 18.6 each
> of these fails with "primary key column "a" is not marked NOT NULL":
>
> CREATE TABLE t (a int NOT NULL);
> ALTER TABLE t ALTER COLUMN a DROP NOT NULL, ADD PRIMARY KEY (a);
>
> CREATE TABLE t (a int CONSTRAINT a_nn NOT NULL);
> ALTER TABLE t DROP CONSTRAINT a_nn, ADD PRIMARY KEY (a);
>
> CREATE TABLE p (a int NOT NULL) PARTITION BY RANGE (a);
> CREATE TABLE p1 PARTITION OF p FOR VALUES FROM (0) TO (10);
> ALTER TABLE p ALTER COLUMN a DROP NOT NULL, ADD PRIMARY KEY (a);
>
> The two DROP NOT NULL forms work on 17.11; the DROP CONSTRAINT form has
> no equivalent there.
>
> To see what else v1 changes, I ran 24 ALTER TABLE ... ADD PRIMARY KEY
> scenarios on REL_18_STABLE at 459ef520d8f and master at c9c660e6ae0,
> with and without v1, and compared the errors and the resulting catalog
> state: the pg_constraint rows (with conislocal, coninhcount,
> convalidated and connoinherit) and attnotnull. scenarios.sql is
> attached.
>
> v1 changes exactly nine of them, all failures that go away:
>
> * dropping and re-adding the column: the reported case, a two-column
> key with one column replaced, a partitioned table, an inheritance
> parent, and the PRIMARY KEY subcommand written before the DROP
> COLUMN;
> * the three commands above;
> * the reported case with a row in the table, which now fails with
> "column "id" of relation "t" contains null values", as on 17.
>
> The other fifteen give identical errors and identical catalog state with
> and without v1. Among them are plain ADD PRIMARY KEY on a partitioned
> table and on an inheritance parent, ONLY on a parent whose child has or
> lacks the not-null, an existing NOT VALID or NO INHERIT not-null, USING
> INDEX, SET NOT NULL in the same command, an identity column, and ALTER
> COLUMN TYPE in the same command. The results are the same on both
> branches.
>
> Building a379061a22a and its parent: before it, all nine behave as they
> do with v1; after it, all nine fail with "is not marked NOT NULL". So
> the commit named in the patch is the one.
>
> v1's test covers the DROP COLUMN form only. The attached top-up adds
> the three commands above to the same place in alter_table.sql. With v1
> make check passes on master and REL_18_STABLE; with the tests but
> without the tablecmds.c change, alter_table fails on all four.
>
> Regards,
> Manu
--
//Ludvig Janiuk
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: 42P16 error when dropping and adding column
@ 2026-09-25 00:50 Manu <manuelreyesbravo@gmail.com>
parent: Ludvig Janiuk <ludvig.janiuk@proton.me>
0 siblings, 1 reply; 9+ messages in thread
From: Manu @ 2026-09-25 00:50 UTC (permalink / raw)
To: Ludvig Janiuk <ludvig.janiuk@proton.me>; +Cc: Fujii Masao <masao.fujii@gmail.com>; pgsql-bugs@lists.postgresql.org
Hi Ludvig,
Ludvig Janiuk <ludvig.janiuk@proton.me> wrote:
> I'm not familiar with the Postgres bug lifecycle - is there anything
> else I need to do? File a formal bug in some tracker? Or has that
> already happened?
Nothing else is needed from you. PostgreSQL has no separate bug
tracker: this thread on pgsql-bugs is the bug report, and it is already
archived where the committers look.
Fujii posted a patch (v1) in this thread, and it was tested against
master and REL_18_STABLE. As of today it is not committed yet. Once a
committer pushes it, the commit message usually links back to this
thread, and a fix for an 18 regression like this one normally goes into
the next 18.x minor release, where it shows up in the release notes.
If you want to follow it, watching this thread is enough. Thanks for
the clear report with a reproducer, it made the bisect straightforward.
Regards,
Manu
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: 42P16 error when dropping and adding column
@ 2026-09-25 14:06 Ludvig Janiuk <ludvig.janiuk@proton.me>
parent: Manu <manuelreyesbravo@gmail.com>
0 siblings, 0 replies; 9+ messages in thread
From: Ludvig Janiuk @ 2026-09-25 14:06 UTC (permalink / raw)
To: Manu <manuelreyesbravo@gmail.com>; +Cc: Fujii Masao <masao.fujii@gmail.com>; pgsql-bugs@lists.postgresql.org
Okay, thank you.
Best regards,
Ludvig Janiuk
On 2026-09-25 02:50, Manu wrote:
> Hi Ludvig,
>
> Ludvig Janiuk <ludvig.janiuk@proton.me> wrote:
>> I'm not familiar with the Postgres bug lifecycle - is there anything
>> else I need to do? File a formal bug in some tracker? Or has that
>> already happened?
> Nothing else is needed from you. PostgreSQL has no separate bug
> tracker: this thread on pgsql-bugs is the bug report, and it is already
> archived where the committers look.
>
> Fujii posted a patch (v1) in this thread, and it was tested against
> master and REL_18_STABLE. As of today it is not committed yet. Once a
> committer pushes it, the commit message usually links back to this
> thread, and a fix for an 18 regression like this one normally goes into
> the next 18.x minor release, where it shows up in the release notes.
>
> If you want to follow it, watching this thread is enough. Thanks for
> the clear report with a reproducer, it made the bisect straightforward.
>
> Regards,
> Manu
--
//Ludvig Janiuk
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: 42P16 error when dropping and adding column
@ 2026-09-28 17:00 Fujii Masao <masao.fujii@gmail.com>
parent: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
1 sibling, 1 reply; 9+ messages in thread
From: Fujii Masao @ 2026-09-28 17:00 UTC (permalink / raw)
To: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>; +Cc: Ludvig Janiuk <ludvig.janiuk@proton.me>; pgsql-bugs@lists.postgresql.org
On Fri, Sep 18, 2026 at 12:01 AM Manuel Reyes Bravo
<manuelreyesbravo@gmail.com> wrote:
> v1's test covers the DROP COLUMN form only. The attached top-up adds
> the three commands above to the same place in alter_table.sql.
Thanks! Attached is v2 patch, which includes your tests.
While working with the v1 patch, I found that the following DDL failed even
though it succeeds without the patch. v2 fixes this regression introduced by
v1.
CREATE TABLE t (id int NOT NULL);
ALTER TABLE t
ALTER COLUMN id DROP NOT NULL,
ADD PRIMARY KEY (id),
ADD CONSTRAINT hoge NOT NULL id;
Regards,
--
Fujii Masao
Attachments:
[application/octet-stream] v2-0001-Fix-ADD-PRIMARY-KEY-after-dropping-and-re-adding-.patch (9.1K, ../../CAHGQGwGPJNq_p8w5W9jvA__6GOmFk5A0K7Vxw3ab0ASotvQy3g@mail.gmail.com/2-v2-0001-Fix-ADD-PRIMARY-KEY-after-dropping-and-re-adding-.patch)
download | inline diff:
From 05192d07f4fa1430ee63806de28992f4c9a8c274 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Thu, 17 Sep 2026 21:35:15 +0900
Subject: [PATCH v2] Fix ADD PRIMARY KEY after dropping and re-adding a column
Previously, dropping a NOT NULL column, adding a new column with the same
name, and making it a primary key in a single ALTER TABLE command could
fail with "primary key column ... is not marked NOT NULL". This happened
because ALTER TABLE checked for an existing NOT NULL constraint before
dropping the old column. Finding the constraint caused it to skip adding
one for the new column, even though the old constraint would be removed
with the old column.
Defer this check until after column drops and additions have been
executed, so that it examines the columns that will actually form the
primary key. Add any required NOT NULL constraints before creating the
primary-key index.
Backpatch to v18, where this issue was introduced by commit a379061a22a.
Reported-by: Ludvig Janiuk <ludvig.janiuk@proton.me>
Author: Fujii Masao <masao.fujii@gmail.com>
Co-authored-by: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
Discussion: https://postgr.es/m/0a9b4c44-cdff-4ec8-a8fb-b3a8b59e9ba9@proton.me
Backpatch-through: 18
---
src/backend/commands/tablecmds.c | 35 ++++++++++++++++-------
src/test/regress/expected/alter_table.out | 25 ++++++++++++++++
src/test/regress/sql/alter_table.sql | 26 +++++++++++++++++
3 files changed, 75 insertions(+), 11 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0274d892f2e..6de62cc463d 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);
@@ -5146,7 +5145,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 */
@@ -5394,6 +5392,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
@@ -9552,7 +9563,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
@@ -9567,16 +9578,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 */
@@ -9600,7 +9612,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
@@ -9638,7 +9650,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 e167a41ce79..20707c186b3 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);
--
2.55.0
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: 42P16 error when dropping and adding column
@ 2026-09-28 17:55 Manu <manuelreyesbravo@gmail.com>
parent: Fujii Masao <masao.fujii@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Manu @ 2026-09-28 17:55 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Ludvig Janiuk <ludvig.janiuk@proton.me>; pgsql-bugs@lists.postgresql.org
Hi Fujii-san,
> Thanks! Attached is v2 patch, which includes your tests.
> While working with the v1 patch, I found that the following DDL failed
> [...] v2 fixes this regression introduced by v1.
I ran v2 through a differential of 32 ALTER TABLE cases on master
(a5447a2deac) and REL_18_STABLE, cassert builds, comparing each build
against unpatched master and REL_18: the 24 from the v1 review plus 8
around a separate named NOT NULL combined with ADD PRIMARY KEY.
v2 fixes the reported case and the seven other 42P16 forms in the set,
each with the expected catalog state (the primary key and an
auto-created NOT NULL). The regression you caught,
ALTER TABLE t ALTER COLUMN id DROP NOT NULL,
ADD PRIMARY KEY (id), ADD CONSTRAINT hoge NOT NULL id;
fails on v1 with "cannot create not-null constraint" and succeeds on
v2, as do the drop-column and partitioned variants of it.
No case in the set errors on v2 while succeeding on unpatched master or
REL_18. The scenarios where a primary key still cannot be added (ONLY
on an inheritance parent, a column that already holds a null row, a
redundant named NOT NULL on a column that already has one) error
identically on all three. master and REL_18 give the same result on
every case, and make check passes on both.
Thanks for folding in the tests.
Regards,
Manu
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: 42P16 error when dropping and adding column
@ 2026-10-06 04:22 Fujii Masao <masao.fujii@gmail.com>
parent: Manu <manuelreyesbravo@gmail.com>
0 siblings, 0 replies; 9+ messages in thread
From: Fujii Masao @ 2026-10-06 04:22 UTC (permalink / raw)
To: Manu <manuelreyesbravo@gmail.com>; +Cc: Ludvig Janiuk <ludvig.janiuk@proton.me>; pgsql-bugs@lists.postgresql.org
On Tue, Sep 29, 2026 at 2:55 AM Manu <manuelreyesbravo@gmail.com> wrote:
> No case in the set errors on v2 while succeeding on unpatched master or
> REL_18. The scenarios where a primary key still cannot be added (ONLY
> on an inheritance parent, a column that already holds a null row, a
> redundant named NOT NULL on a column that already has one) error
> identically on all three. master and REL_18 give the same result on
> every case, and make check passes on both.
Thanks for the test! I've pushed the patch.
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2026-10-06 04:22 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-17 09:51 42P16 error when dropping and adding column Ludvig Janiuk <ludvig.janiuk@proton.me>
2026-09-17 14:22 ` Fujii Masao <masao.fujii@gmail.com>
2026-09-17 15:01 ` Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
2026-09-24 14:09 ` Ludvig Janiuk <ludvig.janiuk@proton.me>
2026-09-25 00:50 ` Manu <manuelreyesbravo@gmail.com>
2026-09-25 14:06 ` Ludvig Janiuk <ludvig.janiuk@proton.me>
2026-09-28 17:00 ` Fujii Masao <masao.fujii@gmail.com>
2026-09-28 17:55 ` Manu <manuelreyesbravo@gmail.com>
2026-10-06 04:22 ` Fujii Masao <masao.fujii@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox