On 2024-Feb-05, Alvaro Herrera wrote: > Hmm, let me have a look, I can probably get this one fixed today before > embarking on a larger fix elsewhere in the same feature.
You know what -- this missing CCI has a much more visible impact, which is that the attnotnull marker that a primary key imposes on a partition is propagated early. So this regression test no longer fails: 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); Here, in the existing code the ALTER TABLE ATTACH fails with the error message that ERROR: primary key column "a" is not marked NOT NULL but with the patch, this no longer occurs. I'm not sure that this behavior change is desirable ... I have vague memories of people complaining that this sort of error was not very welcome ... but on the other hand it seems now pretty clear that if it *is* desirable, then its implementation is no good, because a single added CCI breaks it. I'm leaning towards accepting the behavior change, but I'd like to investigate a little bit more first, but what do others think? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
>From 486f6bfa8faa6162257c5f12b52180b5a3d89704 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Mon, 5 Feb 2024 10:18:43 +0100 Subject: [PATCH v1] Fix failure to merge NOT NULL constraints in inheritance set_attnotnull() was not careful to CommandCounterIncrement() in cases of multiple recursion. Omission in b0e96f311985. Reported-by: Alexander Lakhin <exclus...@gmail.com> Discussion: https://postgr.es/m/045dec3f-9b3d-aa44-0c99-85f699230...@gmail.com --- src/backend/commands/tablecmds.c | 4 ++++ src/test/regress/expected/constraints.out | 1 - src/test/regress/expected/inherit.out | 8 ++++++++ src/test/regress/sql/inherit.sql | 8 ++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 9f51696740..02724d5f04 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7703,6 +7703,10 @@ set_attnotnull(List **wqueue, Relation rel, AttrNumber attnum, bool recurse, List *children; ListCell *lc; + /* Make above catalog changes visible */ + 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 5b068477bf..bef3d6577c 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -1010,7 +1010,6 @@ 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 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); diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 130a924228..fe33bc4c2f 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -2496,6 +2496,14 @@ drop table inh_p1, inh_p2, inh_p3, inh_p4 cascade; NOTICE: drop cascades to 2 other objects DETAIL: drop cascades to table inh_multiparent drop cascades to table inh_multiparent2 +-- recursively add column with constraint while merging existing column +create table inh_p1 (); +create table inh_p2 (f1 int) inherits (inh_p1); +create table inh_p3 () inherits (inh_p1, inh_p2); +alter table inh_p1 add column f1 int not null; +NOTICE: merging definition of column "f1" for child "inh_p2" +NOTICE: merging definition of column "f1" for child "inh_p3" +drop table inh_p1, inh_p2, inh_p3; -- -- Mixed ownership inheritance tree -- diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 0ef9a61bc1..41cdde1d90 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -961,6 +961,14 @@ select conrelid::regclass, contype, conname, drop table inh_p1, inh_p2, inh_p3, inh_p4 cascade; +-- recursively add column with constraint while merging existing column +create table inh_p1 (); +create table inh_p2 (f1 int) inherits (inh_p1); +create table inh_p3 () inherits (inh_p1, inh_p2); +alter table inh_p1 add column f1 int not null; + +drop table inh_p1, inh_p2, inh_p3; + -- -- Mixed ownership inheritance tree -- -- 2.39.2
