From 2773755db4d6df08c14c0854dcf7f3d811fd8ebb Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 24 Apr 2024 16:54:39 +0200 Subject: [PATCH v2 2/2] Disallow changing NO INHERIT property of a constraint ... unless it happens during binary upgrade, or to make an existing constraint into an inherited one of a constraint in a parent relation. --- src/backend/catalog/heap.c | 20 +++++++++++++++----- src/backend/catalog/pg_constraint.c | 15 +++++++++++---- src/include/catalog/pg_constraint.h | 2 +- src/test/regress/expected/inherit.out | 2 +- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index f0278b9c01..136cc42a92 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2549,13 +2549,23 @@ AddRelationNewConstraints(Relation rel, /* * If the column already has an inheritable not-null constraint, - * we need only adjust its inheritance status and we're done. If - * the constraint is there but marked NO INHERIT, then it is - * updated in the same way, but we also recurse to the children - * (if any) to add the constraint there as well. + * we need only adjust its coninhcount and we're done. In certain + * cases (see below), if the constraint is there but marked NO + * INHERIT, then we mark it as no longer such and coninhcount + * updated, plus we must also recurse to the children (if any) to + * add the constraint there. + * + * We only allow the inheritability status to change during binary + * upgrade (where it's used to add the not-null constraints for + * children of tables with primary keys), or when we're recursing + * processing a table down an inheritance hierarchy; directly + * allowing a constraint to change from NO INHERIT to INHERIT + * during ALTER TABLE ADD CONSTRAINT would be far too surprising + * behavior. */ existing = AdjustNotNullInheritance1(RelationGetRelid(rel), colnum, - cdef->inhcount, cdef->is_no_inherit); + cdef->inhcount, cdef->is_no_inherit, + IsBinaryUpgrade || allow_merge); if (existing == 1) continue; /* all done! */ else if (existing == -1) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index aaf3537d3f..6b8496e085 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -721,7 +721,7 @@ extractNotNullColumn(HeapTuple constrTup) */ int AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count, - bool is_no_inherit) + bool is_no_inherit, bool allow_noinherit_change) { HeapTuple tup; @@ -744,16 +744,23 @@ AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count, 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\"", + errmsg("cannot change NO INHERIT status of NOT NULL constraint \"%s\" on relation \"%s\"", NameStr(conform->conname), get_rel_name(relid))); /* * If the constraint already exists in this relation but it's marked - * NO INHERIT, we can just remove that flag, and instruct caller to - * recurse to add the constraint to children. + * NO INHERIT, we can just remove that flag (provided caller allows + * such a change), and instruct caller to recurse to add the + * constraint to children. */ if (!is_no_inherit && conform->connoinherit) { + if (!allow_noinherit_change) + ereport(ERROR, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot change NO INHERIT status of NOT NULL constraint \"%s\" in relation \"%s\"", + NameStr(conform->conname), get_rel_name(relid))); + conform->connoinherit = false; retval = -1; /* caller must add constraint on child rels */ } diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h index 5c52d71e09..68bf55fdf7 100644 --- a/src/include/catalog/pg_constraint.h +++ b/src/include/catalog/pg_constraint.h @@ -262,7 +262,7 @@ extern HeapTuple findNotNullConstraint(Oid relid, const char *colname); extern HeapTuple findDomainNotNullConstraint(Oid typid); extern AttrNumber extractNotNullColumn(HeapTuple constrTup); extern int AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count, - bool is_no_inherit); + bool is_no_inherit, bool allow_noinherit_change); 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 5a5c23fc3b..203ac6c52e 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -2126,7 +2126,7 @@ 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" +ERROR: cannot change NO INHERIT status of 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" -- 2.39.2