From c3709100be73ad5af7ff536476d4d713bca41b1a Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 15 Apr 2024 12:20:56 +0200 Subject: [PATCH] Fix propagating attnotnull in multiple inheritance In one of the many strange corner cases of multiple inheritance being used, commit b0e96f311985 missed a CommandCounterIncrement() call after updating the attnotnull flag during ALTER TABLE ADD COLUMN, which caused a catalog tuple to be update attempted twice in the same command, giving rise to a "tuple already updated by self" error. Add the missing call to solve that, and a test case that reproduces the scenario. As a (perhaps surprising) secondary effect, this CCI addition triggers another behavior change: when a primary key is added to a parent partitioned table and the column in an existing partition does not have a not-null constraint, we no longer error out. This will probably be a welcome change by some users, and I think it's unlikely that anybody will miss the old behavior. Reported-by: Alexander Lakhin Discussion: http://postgr.es/m/045dec3f-9b3d-aa44-0c99-85f6992306c7@gmail.com --- src/backend/commands/tablecmds.c | 4 ++++ src/test/regress/expected/constraints.out | 10 +++++++++- src/test/regress/expected/inherit.out | 8 ++++++++ src/test/regress/sql/constraints.sql | 7 +++++++ src/test/regress/sql/inherit.sql | 7 +++++++ 5 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 000212f24c..c33a5a5888 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7757,6 +7757,10 @@ set_attnotnull(List **wqueue, Relation rel, AttrNumber attnum, bool recurse, List *children; ListCell *lc; + /* Make above update visible, for multiple inheritance cases */ + if (retval) + CommandCounterIncrement(); + children = find_inheritance_children(RelationGetRelid(rel), lockmode); foreach(lc, children) { diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index d9d8408e86..483681ef2c 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -1010,13 +1010,21 @@ ERROR: constraint "cnn_parent_pkey" of relation "cnn_parent" does not exist create table cnn2_parted(a int primary key) partition by list (a); create table cnn2_part1(a int); alter table cnn2_parted attach partition cnn2_part1 for values in (1); -ERROR: primary key column "a" is not marked NOT NULL +insert into cnn2_part1 values (null); +ERROR: null value in column "a" of relation "cnn2_part1" violates not-null constraint +DETAIL: Failing row contains (null). drop table cnn2_parted, cnn2_part1; create table cnn2_parted(a int not null) partition by list (a); create table cnn2_part1(a int primary key); alter table cnn2_parted attach partition cnn2_part1 for values in (1); ERROR: column "a" in child table must be marked NOT NULL drop table cnn2_parted, cnn2_part1; +create table cnn2_parted(a int) partition by list (a); +create table cnn_part1 partition of cnn2_parted for values in (1, null); +insert into cnn_part1 values (null); +alter table cnn2_parted add primary key (a); +ERROR: column "a" of relation "cnn_part1" contains null values +drop table cnn2_parted; -- columns in regular and LIKE inheritance should be marked not-nullable -- for primary keys, even if those are deferred CREATE TABLE notnull_tbl4 (a INTEGER PRIMARY KEY INITIALLY DEFERRED); diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 7837126bd2..5a5c23fc3b 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -2177,6 +2177,14 @@ Child tables: cc1, alter table pp1 add primary key (f1); -- Leave these tables around, for pg_upgrade testing +-- Test a not-null addition that must walk down the hierarchy +CREATE TABLE inh_parent (); +CREATE TABLE inh_child (i int) INHERITS (inh_parent); +CREATE TABLE inh_grandchild () INHERITS (inh_parent, inh_child); +ALTER TABLE inh_parent ADD COLUMN i int NOT NULL; +NOTICE: merging definition of column "i" for child "inh_child" +NOTICE: merging definition of column "i" for child "inh_grandchild" +drop table inh_parent, inh_child, inh_grandchild; -- Test the same constraint name for different columns in different parents create table inh_parent1(a int constraint nn not null); create table inh_parent2(b int constraint nn not null); diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql index 87d685ae39..7c95f6c2aa 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -661,6 +661,7 @@ ALTER TABLE cnn_parent DROP CONSTRAINT cnn_parent_pkey; create table cnn2_parted(a int primary key) partition by list (a); create table cnn2_part1(a int); alter table cnn2_parted attach partition cnn2_part1 for values in (1); +insert into cnn2_part1 values (null); drop table cnn2_parted, cnn2_part1; create table cnn2_parted(a int not null) partition by list (a); @@ -668,6 +669,12 @@ create table cnn2_part1(a int primary key); alter table cnn2_parted attach partition cnn2_part1 for values in (1); drop table cnn2_parted, cnn2_part1; +create table cnn2_parted(a int) partition by list (a); +create table cnn_part1 partition of cnn2_parted for values in (1, null); +insert into cnn_part1 values (null); +alter table cnn2_parted add primary key (a); +drop table cnn2_parted; + -- columns in regular and LIKE inheritance should be marked not-nullable -- for primary keys, even if those are deferred CREATE TABLE notnull_tbl4 (a INTEGER PRIMARY KEY INITIALLY DEFERRED); diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 0ef9a61bc1..277fb74d2c 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -804,6 +804,13 @@ alter table pp1 alter column f1 drop not null; alter table pp1 add primary key (f1); -- Leave these tables around, for pg_upgrade testing +-- Test a not-null addition that must walk down the hierarchy +CREATE TABLE inh_parent (); +CREATE TABLE inh_child (i int) INHERITS (inh_parent); +CREATE TABLE inh_grandchild () INHERITS (inh_parent, inh_child); +ALTER TABLE inh_parent ADD COLUMN i int NOT NULL; +drop table inh_parent, inh_child, inh_grandchild; + -- Test the same constraint name for different columns in different parents create table inh_parent1(a int constraint nn not null); create table inh_parent2(b int constraint nn not null); -- 2.11.4.GIT