Disallow changing NO INHERIT status of a not-null constraint

It makes no sense to add a NO INHERIT not-null constraint to a child
table that already has one in that column inherited from its parent.
Disallow that, and add tests for the relevant cases.

Per complaint from Kyotaro Horiguchi.  I also used part of his proposed
patch.

Co-authored-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20230828.161658.1184657435220765047.horikyota.ntt@gmail.com

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index b534da7d8062fcb8c73384ac54bbce2a790cca94..b42711f574467a1516812aeca8cf14f53339b25c 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2533,7 +2533,7 @@ AddRelationNewConstraints(Relation rel,
                         * update its catalog status and we're done.
                         */
                        if (AdjustNotNullInheritance1(RelationGetRelid(rel), colnum,
-                                                                                 cdef->inhcount))
+                                                                                 cdef->inhcount, cdef->is_no_inherit))
                                continue;
 
                        /*
@@ -2830,6 +2830,17 @@ AddRelationNotNullConstraints(Relation rel, List *constraints,
 
                        if (old->attnum == attnum)
                        {
+                               /*
+                                * If we get a constraint from the parent, having a local NO
+                                * INHERIT one doesn't work.
+                                */
+                               if (constr->is_no_inherit)
+                                       ereport(ERROR,
+                                                       (errcode(ERRCODE_DATATYPE_MISMATCH),
+                                                        errmsg("cannot define not-null constraint on column \"%s\" with NO INHERIT",
+                                                                       strVal(linitial(constr->keys))),
+                                                        errdetail("The column has an inherited not-null constraint.")));
+
                                inhcount++;
                                old_notnulls = foreach_delete_current(old_notnulls, lc2);
                        }

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 2a725f628094642032f982494b092907f4f4261b..e9d4d6006ef146b4e8716fec564fd54429074fac 100644 (file)
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -669,7 +669,8 @@ extractNotNullColumn(HeapTuple constrTup)
  * If no not-null constraint is found for the column, return false.
  */
 bool
-AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count)
+AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count,
+                                                 bool is_no_inherit)
 {
        HeapTuple       tup;
 
@@ -681,6 +682,19 @@ AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count)
 
                pg_constraint = table_open(ConstraintRelationId, RowExclusiveLock);
                conform = (Form_pg_constraint) GETSTRUCT(tup);
+
+               /*
+                * Don't let the NO INHERIT status change (but don't complain
+                * unnecessarily.)  In the future it might be useful to let an
+                * inheritable constraint replace a non-inheritable one, but we'd need
+                * to recurse to children to get it added there.
+                */
+               if (is_no_inherit != conform->connoinherit)
+                       ereport(ERROR,
+                                       errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                                       errmsg("cannot change NO INHERIT status of inherited NOT NULL constraint \"%s\" on relation \"%s\"",
+                                                  NameStr(conform->conname), get_rel_name(relid)));
+
                if (count > 0)
                        conform->coninhcount += count;
 
@@ -691,9 +705,9 @@ AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count)
                                 get_rel_name(relid));
 
                /*
-                * If the constraints are no longer inherited, mark them local.  It's
-                * arguable that we should drop them instead, but it's hard to see
-                * that being better.  The user can drop it manually later.
+                * If the constraint is no longer inherited, mark it local.  It's
+                * arguable that we should drop it instead, but it's hard to see that
+                * being better.  The user can drop it manually later.
                 */
                if (conform->coninhcount == 0)
                        conform->conislocal = true;

diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index f6f5796fe08b698515ddca605f179931a6e9812b..f366f8cd148f4b5d7738aaf5d50afbab3a38aa50 100644 (file)
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -248,7 +248,8 @@ extern char *ChooseConstraintName(const char *name1, const char *name2,
 extern HeapTuple findNotNullConstraintAttnum(Oid relid, AttrNumber attnum);
 extern HeapTuple findNotNullConstraint(Oid relid, const char *colname);
 extern AttrNumber extractNotNullColumn(HeapTuple constrTup);
-extern bool AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count);
+extern bool AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count,
+                                                                         bool is_no_inherit);
 extern void AdjustNotNullInheritance(Oid relid, Bitmapset *columns, int count);
 extern List *RelationGetNotNullConstraints(Oid relid, bool cooked);
 

diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 6daca12340cbf4676ce08ce18ee2d4eacd5be606..dae61b9a0b17dbe78be5ad4baf34ec650030bbce 100644 (file)
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -2051,6 +2051,15 @@ Not-null constraints:
 Inherits: pp1,
           cc1
 
+-- cannot create table with inconsistent NO INHERIT constraint
+create table cc3 (a2 int not null no inherit) inherits (cc1);
+NOTICE:  moving and merging column "a2" with inherited definition
+DETAIL:  User-specified column moved to the position of the inherited column.
+ERROR:  cannot define not-null constraint on column "a2" with NO INHERIT
+DETAIL:  The column has an inherited not-null constraint.
+-- change NO INHERIT status of inherited constraint: no dice, it's inherited
+alter table cc2 add not null a2 no inherit;
+ERROR:  cannot change NO INHERIT status of inherited NOT NULL constraint "nn" on relation "cc2"
 -- remove constraint from cc2: no dice, it's inherited
 alter table cc2 alter column a2 drop not null;
 ERROR:  cannot drop inherited constraint "nn" of relation "cc2"

diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index d8fae92a533e8ac152623a1bb602baac3a6fd55f..9ceaec1d78eb01d4ddf3eb11159a72bf54392eb3 100644 (file)
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -736,6 +736,12 @@ alter table pp1 alter column f1 set not null;
 \d+ cc1
 \d+ cc2
 
+-- cannot create table with inconsistent NO INHERIT constraint
+create table cc3 (a2 int not null no inherit) inherits (cc1);
+
+-- change NO INHERIT status of inherited constraint: no dice, it's inherited
+alter table cc2 add not null a2 no inherit;
+
 -- remove constraint from cc2: no dice, it's inherited
 alter table cc2 alter column a2 drop not null;