postgres.git / summary / log / commit / refs
commit 9b581c53418666205938311ef86047aa3c6b741f
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue Aug 29 17:19:24 2023 +0000
First released in: 17.0
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
src/backend/catalog/heap.c | 13 ++++++++++++-
src/backend/catalog/pg_constraint.c | 22 ++++++++++++++++++----
src/include/catalog/pg_constraint.h | 3 ++-
src/test/regress/expected/inherit.out | 9 +++++++++
src/test/regress/sql/inherit.sql | 6 ++++++
5 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index b534da7d806..b42711f5744 100644
--- 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 2a725f62809..e9d4d6006ef 100644
--- 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 f6f5796fe08..f366f8cd148 100644
--- 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 6daca12340c..dae61b9a0b1 100644
--- 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 d8fae92a533..9ceaec1d78e 100644
--- 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;
[parent: 952db4979f9f]