diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index ebd8c62038..0bf11f6cb6 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -105,7 +105,7 @@ WITH ( MODULUS numeric_literal, REM and column_constraint is: [ CONSTRAINT constraint_name ] -{ NOT NULL | +{ NOT NULL [ NO INHERIT ] | NULL | CHECK ( expression ) [ NO INHERIT ] | DEFAULT default_expr | 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/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 08c87e6029..c31783373f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -4980,10 +4980,12 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, break; case AT_AddConstraint: /* ADD CONSTRAINT */ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE); - /* Recursion occurs during execution phase */ - /* No command-specific prep needed except saving recurse flag */ if (recurse) + { + /* recurses at exec time; lock descendants and set flag */ + (void) find_all_inheritors(RelationGetRelid(rel), lockmode, NULL); cmd->recurse = true; + } pass = AT_PASS_ADD_CONSTR; break; case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */ @@ -7892,6 +7894,17 @@ ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName, copytup = heap_copytuple(tuple); conForm = (Form_pg_constraint) GETSTRUCT(copytup); + /* + * Don't let a NO INHERIT constraint be changed into inherit. + */ + if (conForm->connoinherit && recurse) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change NO INHERIT status of NOT NULL constraint \"%s\" in relation \"%s\"", + NameStr(conForm->conname), + RelationGetRelationName(rel))); + + /* * If we find an appropriate constraint, we're almost done, but just * need to change some properties on it: if we're recursing, increment 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..eaa65049c7 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" @@ -2277,6 +2277,34 @@ Child tables: inh_nn_child, inh_nn_child2 drop table inh_nn_parent, inh_nn_child, inh_nn_child2; +CREATE TABLE inh_nn_parent (a int, NOT NULL a NO INHERIT); +CREATE TABLE inh_nn_child() INHERITS (inh_nn_parent); +ALTER TABLE inh_nn_parent ADD CONSTRAINT nna NOT NULL a; +ERROR: cannot change NO INHERIT status of NOT NULL constraint "inh_nn_parent_a_not_null" in relation "inh_nn_parent" +ALTER TABLE inh_nn_parent ALTER a SET NOT NULL; +ERROR: cannot change NO INHERIT status of NOT NULL constraint "inh_nn_parent_a_not_null" in relation "inh_nn_parent" +DROP TABLE inh_nn_parent cascade; +NOTICE: drop cascades to table inh_nn_child +-- Adding a PK at the top level of a hierarchy should cause all descendants +-- to be checked for nulls, even past a no-inherit constraint +CREATE TABLE inh_nn_lvl1 (a int); +CREATE TABLE inh_nn_lvl2 () INHERITS (inh_nn_lvl1); +CREATE TABLE inh_nn_lvl3 (CONSTRAINT foo NOT NULL a NO INHERIT) INHERITS (inh_nn_lvl2); +CREATE TABLE inh_nn_lvl4 () INHERITS (inh_nn_lvl3); +CREATE TABLE inh_nn_lvl5 () INHERITS (inh_nn_lvl4); +INSERT INTO inh_nn_lvl2 VALUES (NULL); +ALTER TABLE inh_nn_lvl1 ADD PRIMARY KEY (a); +ERROR: column "a" of relation "inh_nn_lvl2" contains null values +DELETE FROM inh_nn_lvl2; +INSERT INTO inh_nn_lvl5 VALUES (NULL); +ALTER TABLE inh_nn_lvl1 ADD PRIMARY KEY (a); +ERROR: column "a" of relation "inh_nn_lvl5" contains null values +DROP TABLE inh_nn_lvl1 CASCADE; +NOTICE: drop cascades to 4 other objects +DETAIL: drop cascades to table inh_nn_lvl2 +drop cascades to table inh_nn_lvl3 +drop cascades to table inh_nn_lvl4 +drop cascades to table inh_nn_lvl5 -- -- test inherit/deinherit -- diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 277fb74d2c..2205e59aff 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -844,6 +844,26 @@ select conrelid::regclass, conname, contype, conkey, \d+ inh_nn* drop table inh_nn_parent, inh_nn_child, inh_nn_child2; +CREATE TABLE inh_nn_parent (a int, NOT NULL a NO INHERIT); +CREATE TABLE inh_nn_child() INHERITS (inh_nn_parent); +ALTER TABLE inh_nn_parent ADD CONSTRAINT nna NOT NULL a; +ALTER TABLE inh_nn_parent ALTER a SET NOT NULL; +DROP TABLE inh_nn_parent cascade; + +-- Adding a PK at the top level of a hierarchy should cause all descendants +-- to be checked for nulls, even past a no-inherit constraint +CREATE TABLE inh_nn_lvl1 (a int); +CREATE TABLE inh_nn_lvl2 () INHERITS (inh_nn_lvl1); +CREATE TABLE inh_nn_lvl3 (CONSTRAINT foo NOT NULL a NO INHERIT) INHERITS (inh_nn_lvl2); +CREATE TABLE inh_nn_lvl4 () INHERITS (inh_nn_lvl3); +CREATE TABLE inh_nn_lvl5 () INHERITS (inh_nn_lvl4); +INSERT INTO inh_nn_lvl2 VALUES (NULL); +ALTER TABLE inh_nn_lvl1 ADD PRIMARY KEY (a); +DELETE FROM inh_nn_lvl2; +INSERT INTO inh_nn_lvl5 VALUES (NULL); +ALTER TABLE inh_nn_lvl1 ADD PRIMARY KEY (a); +DROP TABLE inh_nn_lvl1 CASCADE; + -- -- test inherit/deinherit --