diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 88087654de9b7..74a31bf19f75b 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2880,12 +2880,20 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr, * constraint is not, this should be allowed by marking the child * constraint as enforced. In the reverse case, an error would have * already been thrown before reaching this point. + * + * Since the constraint was not enforced before, its existing rows + * have never been checked against it, so it must not become + * validated as a side effect of the merge. Mark it validated only + * if the new constraint says to be initially valid, in which case + * it is the caller's responsibility to verify the existing rows + * (ATAddCheckNNConstraint queues that work); otherwise it remains + * NOT VALID until ALTER TABLE ... VALIDATE CONSTRAINT is run. */ if (is_enforced && !con->conenforced) { Assert(is_local); con->conenforced = true; - con->convalidated = true; + con->convalidated = is_initially_valid; } CatalogTupleUpdate(conDesc, &tup->t_self, tup); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index cb93c3e935a1f..a8c6224e65029 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10053,6 +10053,7 @@ ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ListCell *lcon; List *children; ListCell *child; + Oid enforcing_conoid = InvalidOid; ObjectAddress address = InvalidObjectAddress; /* Guard against stack overflow due to overly deep inheritance tree. */ @@ -10063,15 +10064,51 @@ ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ATSimplePermissions(AT_AddConstraint, rel, ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE); + /* + * If the command names a check constraint and an identically-named NOT + * ENFORCED check constraint already exists on this relation, then (if + * things go well) AddRelationNewConstraints will merge the new + * constraint into the existing one, additionally marking it enforced. + * Existing rows have never been checked against a NOT ENFORCED + * constraint, so unlike ordinary merges this one requires the existing + * rows to be verified when the new constraint is to be valid. Take note + * of the pre-merge state, so that we can queue that work below. + */ + if (constr->contype == CONSTR_CHECK && + constr->conname != NULL && + constr->is_enforced && + constr->initially_valid) + { + Oid conoid; + + conoid = get_relation_constraint_oid(RelationGetRelid(rel), + constr->conname, true); + if (OidIsValid(conoid)) + { + HeapTuple contup; + Form_pg_constraint conform; + + contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(conoid)); + if (!HeapTupleIsValid(contup)) + elog(ERROR, "cache lookup failed for constraint %u", conoid); + conform = (Form_pg_constraint) GETSTRUCT(contup); + if (conform->contype == CONSTRAINT_CHECK && !conform->conenforced) + enforcing_conoid = conoid; + ReleaseSysCache(contup); + } + } + /* * Call AddRelationNewConstraints to do the work, making sure it works on * a copy of the Constraint so transformExpr can't modify the original. It * returns a list of cooked constraints. * * If the constraint ends up getting merged with a pre-existing one, it's - * omitted from the returned list, which is what we want: we do not need - * to do any validation work. That can only happen at child tables, - * though, since we disallow merging at the top level. + * omitted from the returned list. Normally there is then no validation + * work to do, but if the merge marked a previously NOT ENFORCED + * constraint as enforced, existing rows must be verified; that case is + * handled below. Merging can only happen at child tables, though, since + * we disallow merging at the top level. */ newcons = AddRelationNewConstraints(rel, NIL, list_make1(copyObject(constr)), @@ -10125,6 +10162,40 @@ ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, /* Advance command counter in case same table is visited multiple times */ CommandCounterIncrement(); + /* + * If the new constraint was merged into a pre-existing NOT ENFORCED + * constraint, the merge marked that constraint enforced and valid, but + * its existing rows have never been checked; tell Phase 3 to verify + * them, just as ALTER TABLE ... ALTER CONSTRAINT ... ENFORCED would do. + * (If the constraint was instead created afresh, the loop above has + * already taken care of this.) + */ + if (newcons == NIL && OidIsValid(enforcing_conoid) && + rel->rd_rel->relkind == RELKIND_RELATION) + { + NewConstraint *newcon; + HeapTuple contup; + Datum val; + char *conbin; + + contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(enforcing_conoid)); + if (!HeapTupleIsValid(contup)) + elog(ERROR, "cache lookup failed for constraint %u", + enforcing_conoid); + + newcon = palloc0_object(NewConstraint); + newcon->name = pstrdup(constr->conname); + newcon->contype = CONSTR_CHECK; + val = SysCacheGetAttrNotNull(CONSTROID, contup, + Anum_pg_constraint_conbin); + conbin = TextDatumGetCString(val); + newcon->qual = expand_generated_columns_in_expr(stringToNode(conbin), + rel, 1); + ReleaseSysCache(contup); + + tab->constraints = lappend(tab->constraints, newcon); + } + /* * If the constraint got merged with an existing constraint, we're done. * We mustn't recurse to child tables in this case, because they've diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 0136aa53c96a5..265b7b721857c 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1371,7 +1371,8 @@ NOTICE: merging constraint "inh_check_constraint6" with inherited definition alter table p1_c1 add constraint inh_check_constraint9 check (f1 < 10) not valid enforced; alter table p1 add constraint inh_check_constraint9 check (f1 < 10) not enforced; NOTICE: merging constraint "inh_check_constraint9" with inherited definition --- the not-valid state of the child constraint will be ignored here. +-- the not-valid state of the child constraint is preserved here, so the +-- merged constraint becomes enforced but remains NOT VALID. alter table p1 add constraint inh_check_constraint10 check (f1 < 10) not enforced; alter table p1_c1 add constraint inh_check_constraint10 check (f1 < 10) not valid enforced; NOTICE: merging constraint "inh_check_constraint10" with inherited definition @@ -1411,7 +1412,7 @@ order by 1, 2; p1 | inh_check_constraint8 | t | 0 | t | t p1 | inh_check_constraint9 | t | 0 | f | f p1_c1 | inh_check_constraint1 | t | 1 | t | t - p1_c1 | inh_check_constraint10 | t | 1 | t | t + p1_c1 | inh_check_constraint10 | t | 1 | t | f p1_c1 | inh_check_constraint2 | t | 1 | t | t p1_c1 | inh_check_constraint3 | t | 1 | f | f p1_c1 | inh_check_constraint4 | t | 1 | f | f @@ -1479,6 +1480,53 @@ NOTICE: drop cascades to 3 other objects DETAIL: drop cascades to table p1_c1 drop cascades to table p1_c2 drop cascades to table p1_c3 +-- Existing rows must be verified when merging a local ENFORCED constraint +-- into an inherited NOT ENFORCED one +create table p1(f1 int); +create table p1_c1() inherits(p1); +alter table p1 add constraint inh_check_constraint check (f1 > 0) not enforced; +insert into p1_c1 values(-1); +alter table p1_c1 add constraint inh_check_constraint check (f1 > 0); --error +NOTICE: merging constraint "inh_check_constraint" with inherited definition +ERROR: check constraint "inh_check_constraint" of relation "p1_c1" is violated by some row +-- adding it as NOT VALID skips the verification, and the merged constraint +-- must not be marked validated +alter table p1_c1 add constraint inh_check_constraint check (f1 > 0) not valid; --ok +NOTICE: merging constraint "inh_check_constraint" with inherited definition +select conrelid::regclass, conenforced, convalidated +from pg_constraint where conname = 'inh_check_constraint' +order by conrelid::regclass::text collate "C"; + conrelid | conenforced | convalidated +----------+-------------+-------------- + p1 | f | f + p1_c1 | t | f +(2 rows) + +alter table p1_c1 validate constraint inh_check_constraint; --error +ERROR: check constraint "inh_check_constraint" of relation "p1_c1" is violated by some row +delete from p1_c1 where f1 = -1; +alter table p1_c1 validate constraint inh_check_constraint; --ok +drop table p1 cascade; +NOTICE: drop cascades to table p1_c1 +-- with no violating rows the merge succeeds, and the verification allows the +-- merged constraint to be marked validated +create table p1(f1 int); +create table p1_c1() inherits(p1); +alter table p1 add constraint inh_check_constraint check (f1 > 0) not enforced; +insert into p1_c1 values(1); +alter table p1_c1 add constraint inh_check_constraint check (f1 > 0); --ok +NOTICE: merging constraint "inh_check_constraint" with inherited definition +select conrelid::regclass, conenforced, convalidated +from pg_constraint where conname = 'inh_check_constraint' +order by conrelid::regclass::text collate "C"; + conrelid | conenforced | convalidated +----------+-------------+-------------- + p1 | f | f + p1_c1 | t | t +(2 rows) + +drop table p1 cascade; +NOTICE: drop cascades to table p1_c1 -- an inherited CHECK constraint cannot be NOT ENFORCED under an ENFORCED parent create table p1(f1 int constraint p1_a_check check (f1 > 0) enforced); create table p1_c1() inherits(p1); diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 072fca13c1351..8eeea8e1f79f3 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -485,7 +485,8 @@ alter table p1_c1 add constraint inh_check_constraint6 check (f1 < 10) enforced; alter table p1_c1 add constraint inh_check_constraint9 check (f1 < 10) not valid enforced; alter table p1 add constraint inh_check_constraint9 check (f1 < 10) not enforced; --- the not-valid state of the child constraint will be ignored here. +-- the not-valid state of the child constraint is preserved here, so the +-- merged constraint becomes enforced but remains NOT VALID. alter table p1 add constraint inh_check_constraint10 check (f1 < 10) not enforced; alter table p1_c1 add constraint inh_check_constraint10 check (f1 < 10) not valid enforced; @@ -535,6 +536,36 @@ where conname = 'inh_check_constraint3' and contype = 'c' order by conrelid::regclass::text collate "C"; drop table p1 cascade; +-- Existing rows must be verified when merging a local ENFORCED constraint +-- into an inherited NOT ENFORCED one +create table p1(f1 int); +create table p1_c1() inherits(p1); +alter table p1 add constraint inh_check_constraint check (f1 > 0) not enforced; +insert into p1_c1 values(-1); +alter table p1_c1 add constraint inh_check_constraint check (f1 > 0); --error +-- adding it as NOT VALID skips the verification, and the merged constraint +-- must not be marked validated +alter table p1_c1 add constraint inh_check_constraint check (f1 > 0) not valid; --ok +select conrelid::regclass, conenforced, convalidated +from pg_constraint where conname = 'inh_check_constraint' +order by conrelid::regclass::text collate "C"; +alter table p1_c1 validate constraint inh_check_constraint; --error +delete from p1_c1 where f1 = -1; +alter table p1_c1 validate constraint inh_check_constraint; --ok +drop table p1 cascade; + +-- with no violating rows the merge succeeds, and the verification allows the +-- merged constraint to be marked validated +create table p1(f1 int); +create table p1_c1() inherits(p1); +alter table p1 add constraint inh_check_constraint check (f1 > 0) not enforced; +insert into p1_c1 values(1); +alter table p1_c1 add constraint inh_check_constraint check (f1 > 0); --ok +select conrelid::regclass, conenforced, convalidated +from pg_constraint where conname = 'inh_check_constraint' +order by conrelid::regclass::text collate "C"; +drop table p1 cascade; + -- an inherited CHECK constraint cannot be NOT ENFORCED under an ENFORCED parent create table p1(f1 int constraint p1_a_check check (f1 > 0) enforced); create table p1_c1() inherits(p1);