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 <exclusion@gmail.com>
Discussion: http://postgr.es/m/045dec3f-9b3d-aa44-0c99-85f6992306c7@gmail.com

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 000212f24c42ee5a30be662ac06bbb930aae9a9a..c33a5a5888ad39d8e9b9132725e59be7dcfdbcdd 100644 (file)
--- 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 d9d8408e869579930e1b60a3cc91219cf7c52d1b..483681ef2c6bbeded3d615f0d34d7105f352d597 100644 (file)
--- 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 7837126bd22f285f344537404bbf8541ea23080a..5a5c23fc3b018296fc184160d4d6941cfb1b5751 100644 (file)
--- 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 87d685ae39201d00ca351e959102c3005509194c..7c95f6c2aa7c1a76786703bd0aeb01652c8f17c9 100644 (file)
--- 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 0ef9a61bc16b677e211ad88df033b4ae8aa5f74b..277fb74d2c0566b488e170df9f857ed360b472ea 100644 (file)
--- 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);