From a7d4c85db8b4c80e3427674f53751d9ea1477c41 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Wed, 27 May 2026 13:49:29 +0800 Subject: [PATCH v6 1/3] Prevent inherited CHECK constraints from being weakened Disallow marking an inherited CHECK constraint as NOT ENFORCED when an equivalent parent constraint remains ENFORCED. This prevents ALTER CONSTRAINT from producing a child constraint that is weaker than one of its inherited parent definitions. When recursively altering a CHECK constraint to NOT ENFORCED, collect the corresponding constraints in the affected inheritance subtree and ignore those parent constraints while checking descendants. If a descendant also inherits an equivalent ENFORCED constraint from a parent outside the current ALTER, keep the descendant ENFORCED by merging to the stricter state. Add regression coverage for direct child ALTER, ONLY ALTER, mixed-parent inheritance, and a common-ancestor diamond where all equivalent inherited constraints can be changed together. Author: Chao Li Reviewed-by: Jian He Discussion: https://postgr.es/m/E74C57FA-1DD0-4C8E-8FB1-538034752592@gmail.com --- src/backend/commands/tablecmds.c | 217 ++++++++++++++++++++-- src/test/regress/expected/constraints.out | 12 +- src/test/regress/expected/inherit.out | 81 ++++++++ src/test/regress/sql/constraints.sql | 5 +- src/test/regress/sql/inherit.sql | 47 +++++ 5 files changed, 337 insertions(+), 25 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a1845240a98..64a0dacca4f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -437,6 +437,7 @@ static bool ATExecAlterFKConstrEnforceability(List **wqueue, ATAlterConstraint * static bool ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, Relation conrel, HeapTuple contuple, bool recurse, bool recursing, + List *changing_conids, LOCKMODE lockmode); static bool ATExecAlterConstrDeferrability(List **wqueue, ATAlterConstraint *cmdcon, Relation conrel, Relation tgrel, Relation rel, @@ -459,6 +460,7 @@ static void AlterFKConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint static void AlterCheckConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon, Relation conrel, Oid conrelid, bool recurse, bool recursing, + List *changing_conids, LOCKMODE lockmode); static void AlterConstrDeferrabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon, Relation conrel, Relation tgrel, Relation rel, @@ -466,6 +468,10 @@ static void AlterConstrDeferrabilityRecurse(List **wqueue, ATAlterConstraint *cm List **otherrelids, LOCKMODE lockmode); static void AlterConstrUpdateConstraintEntry(ATAlterConstraint *cmdcon, Relation conrel, HeapTuple contuple); +static bool ATCheckCheckConstrHasEnforcedParent(Relation conrel, Relation rel, + HeapTuple contuple, + List *changing_conids, + Oid *enforced_parentoid); static ObjectAddress ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName, bool recurse, bool recursing, LOCKMODE lockmode); @@ -477,6 +483,7 @@ static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relat static void QueueNNConstraintValidation(List **wqueue, Relation conrel, Relation rel, HeapTuple contuple, bool recurse, bool recursing, LOCKMODE lockmode); +static bool constraints_equivalent(HeapTuple a, HeapTuple b, TupleDesc tupleDesc); static int transformColumnNameList(Oid relId, List *colList, int16 *attnums, Oid *atttypids, Oid *attcollids); static int transformFkeyGetPrimaryKey(Relation pkrel, Oid *indexOid, @@ -12484,7 +12491,7 @@ ATExecAlterConstraintInternal(List **wqueue, ATAlterConstraint *cmdcon, else if (currcon->contype == CONSTRAINT_CHECK) changed = ATExecAlterCheckConstrEnforceability(wqueue, cmdcon, conrel, contuple, recurse, false, - lockmode); + NIL, lockmode); } else if (cmdcon->alterDeferrability && ATExecAlterConstrDeferrability(wqueue, cmdcon, conrel, tgrel, rel, @@ -12671,12 +12678,16 @@ ATExecAlterFKConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, static bool ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, Relation conrel, HeapTuple contuple, - bool recurse, bool recursing, LOCKMODE lockmode) + bool recurse, bool recursing, + List *changing_conids, + LOCKMODE lockmode) { Form_pg_constraint currcon; Relation rel; bool changed = false; List *children = NIL; + bool target_enforced = cmdcon->is_enforced; + Oid enforced_parentoid = InvalidOid; /* Since this function recurses, it could be driven to stack overflow */ check_stack_depth(); @@ -12693,16 +12704,53 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, */ rel = table_open(currcon->conrelid, NoLock); - if (currcon->conenforced != cmdcon->is_enforced) + /* + * When setting a constraint to NOT ENFORCED, check whether any matching + * parent constraint remains ENFORCED and is not part of this ALTER. + * + * For a direct ALTER of an inherited constraint, reject the command, + * because the child cannot be weakened while its parent remains enforced. + * + * During recursion, another parent outside this ALTER may still enforce + * the same constraint. In that case, keep the child constraint ENFORCED + * so that its merged enforceability still reflects the remaining enforced + * parent. + */ + if (!cmdcon->is_enforced && + ATCheckCheckConstrHasEnforcedParent(conrel, rel, contuple, + changing_conids, + &enforced_parentoid)) { - AlterConstrUpdateConstraintEntry(cmdcon, conrel, contuple); + if (!recursing) + ereport(ERROR, + errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("cannot mark inherited constraint \"%s\" as NOT ENFORCED because " + "matching constraint on parent table \"%s\" is ENFORCED", + NameStr(currcon->conname), + get_rel_name(enforced_parentoid))); + + target_enforced = true; + } + + /* + * Update to the merged enforceability if needed. This may differ from the + * requested enforceability when another matching parent constraint + * remains enforced. + */ + if (currcon->conenforced != target_enforced) + { + ATAlterConstraint updatecon = *cmdcon; + + updatecon.is_enforced = target_enforced; + AlterConstrUpdateConstraintEntry(&updatecon, conrel, contuple); changed = true; } /* * Note that we must recurse even when trying to change a check constraint * to not enforced if it is already not enforced, in case descendant - * constraints might be enforced and need to be changed to not enforced. + * constraints might be enforced and need to be changed to not enforced, + * unless they still inherit an enforced constraint from another parent. * Conversely, we should do nothing if a constraint is being set to * enforced and is already enforced, as descendant constraints cannot be * different in that case. @@ -12715,28 +12763,68 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, * try to look for it in the children. */ if (!recursing && !currcon->connoinherit) + { + Assert(changing_conids == NIL); + children = find_all_inheritors(RelationGetRelid(rel), lockmode, NULL); + foreach_oid(childoid, children) + { + if (childoid == RelationGetRelid(rel)) + continue; + + /* + * If we are told not to recurse, there had better not be any + * child tables, because we can't change constraint + * enforceability on the parent unless we have changed + * enforceability for all child. + */ + if (!recurse) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("constraint must be altered on child tables too"), + errhint("Do not specify the ONLY keyword.")); + } + + if (!cmdcon->is_enforced) + { + /* + * Build the set of equivalent CHECK constraints that this + * command will attempt to change before visiting descendants. + * The root itself has already been checked above. + */ + changing_conids = list_make1_oid(currcon->oid); + + foreach_oid(childoid, children) + { + if (childoid == RelationGetRelid(rel)) + continue; + + /* + * It is sufficient to look up the constraint by name + * here. Supported DDL ensures that inheritable CHECK + * constraints with the same name have equivalent + * definitions when they are propagated to children or + * when inheritance is established. + */ + changing_conids = + list_append_unique_oid(changing_conids, + get_relation_constraint_oid(childoid, + cmdcon->conname, + false)); + } + } + } + foreach_oid(childoid, children) { if (childoid == RelationGetRelid(rel)) continue; - /* - * If we are told not to recurse, there had better not be any - * child tables, because we can't change constraint enforceability - * on the parent unless we have changed enforceability for all - * child. - */ - if (!recurse) - ereport(ERROR, - errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("constraint must be altered on child tables too"), - errhint("Do not specify the ONLY keyword.")); - AlterCheckConstrEnforceabilityRecurse(wqueue, cmdcon, conrel, childoid, false, true, + changing_conids, lockmode); } } @@ -12748,7 +12836,7 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, */ if (rel->rd_rel->relkind == RELKIND_RELATION && !currcon->conenforced && - cmdcon->is_enforced) + target_enforced) { AlteredTableInfo *tab; NewConstraint *newcon; @@ -12788,6 +12876,7 @@ static void AlterCheckConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon, Relation conrel, Oid conrelid, bool recurse, bool recursing, + List *changing_conids, LOCKMODE lockmode) { SysScanDesc pscan; @@ -12817,11 +12906,101 @@ AlterCheckConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon, cmdcon->conname, get_rel_name(conrelid))); ATExecAlterCheckConstrEnforceability(wqueue, cmdcon, conrel, childtup, - recurse, recursing, lockmode); + recurse, recursing, changing_conids, + lockmode); systable_endscan(pscan); } +/* + * When setting an inherited CHECK constraint to NOT ENFORCED, look for a + * matching parent constraint that remains ENFORCED and is not part of the same + * ALTER. + */ +static bool +ATCheckCheckConstrHasEnforcedParent(Relation conrel, Relation rel, + HeapTuple contuple, + List *changing_conids, + Oid *enforced_parentoid) +{ + Form_pg_constraint currcon; + Relation inhrel; + SysScanDesc scan; + ScanKeyData skey; + HeapTuple inheritsTuple; + + currcon = (Form_pg_constraint) GETSTRUCT(contuple); + Assert(currcon->contype == CONSTRAINT_CHECK); + + if (currcon->coninhcount <= 0) + return false; + + inhrel = table_open(InheritsRelationId, AccessShareLock); + + ScanKeyInit(&skey, + Anum_pg_inherits_inhrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + scan = systable_beginscan(inhrel, InheritsRelidSeqnoIndexId, + true, NULL, 1, &skey); + + while (HeapTupleIsValid(inheritsTuple = systable_getnext(scan))) + { + Oid parentoid; + SysScanDesc pscan; + ScanKeyData pkey[3]; + HeapTuple parenttup; + + parentoid = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhparent; + + ScanKeyInit(&pkey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(parentoid)); + ScanKeyInit(&pkey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&pkey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + NameGetDatum(&currcon->conname)); + + pscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, + true, NULL, 3, pkey); + + while (HeapTupleIsValid(parenttup = systable_getnext(pscan))) + { + Form_pg_constraint parentcon; + + parentcon = (Form_pg_constraint) GETSTRUCT(parenttup); + + if (list_member_oid(changing_conids, parentcon->oid) || + parentcon->contype != CONSTRAINT_CHECK || + parentcon->connoinherit || + !parentcon->conenforced) + continue; + + if (constraints_equivalent(parenttup, contuple, + RelationGetDescr(conrel))) + { + *enforced_parentoid = parentoid; + systable_endscan(pscan); + systable_endscan(scan); + table_close(inhrel, AccessShareLock); + return true; + } + } + + systable_endscan(pscan); + } + + systable_endscan(scan); + table_close(inhrel, AccessShareLock); + + return false; +} + /* * Returns true if the constraint's deferrability is altered. * diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index e54fec7fb57..dada27a4cba 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -446,8 +446,12 @@ alter table parted_ch_2 alter constraint cc_2 enforced; --error ERROR: check constraint "cc_2" of relation "parted_ch_2" is violated by some row delete from parted_ch where a = 16; alter table parted_ch_2 alter constraint cc_2 enforced; -alter table parted_ch_2 alter constraint cc not enforced; -alter table parted_ch_2 alter constraint cc_1 not enforced; +alter table parted_ch_2 alter constraint cc not enforced; --error +ERROR: cannot mark inherited constraint "cc" as NOT ENFORCED because matching constraint on parent table "parted_ch" is ENFORCED +alter table only parted_ch_2 alter constraint cc not enforced; --error +ERROR: cannot mark inherited constraint "cc" as NOT ENFORCED because matching constraint on parent table "parted_ch" is ENFORCED +alter table parted_ch_2 alter constraint cc_1 not enforced; --error +ERROR: cannot mark inherited constraint "cc_1" as NOT ENFORCED because matching constraint on parent table "parted_ch" is ENFORCED alter table parted_ch_2 alter constraint cc_2 not enforced; --check these CHECK constraint status again select * from check_constraint_status; @@ -457,12 +461,12 @@ select * from check_constraint_status; cc | parted_ch_1 | t | t cc | parted_ch_11 | t | t cc | parted_ch_12 | t | t - cc | parted_ch_2 | f | f + cc | parted_ch_2 | t | t cc_1 | parted_ch | t | t cc_1 | parted_ch_1 | t | t cc_1 | parted_ch_11 | t | t cc_1 | parted_ch_12 | t | t - cc_1 | parted_ch_2 | f | f + cc_1 | parted_ch_2 | t | t cc_2 | parted_ch_2 | f | f (11 rows) diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 3d8e8d8afd2..d293279faae 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1479,6 +1479,87 @@ 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 +-- 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); +alter table p1_c1 alter constraint p1_a_check not enforced; --error +ERROR: cannot mark inherited constraint "p1_a_check" as NOT ENFORCED because matching constraint on parent table "p1" is ENFORCED +alter table p1 alter constraint p1_a_check not enforced; --ok +alter table p1_c1 alter constraint p1_a_check not enforced; --ok +drop table p1 cascade; +NOTICE: drop cascades to table p1_c1 +-- recursive NOT ENFORCED merges with ENFORCED constraints from other parents +create table p1(a int constraint p1_a_check check (a > 0) enforced); +create table p2(a int constraint p1_a_check check (a > 0) enforced); +create table p1_c1() inherits (p1, p2); +NOTICE: merging multiple inherited definitions of column "a" +create table p1_c2() inherits (p1); +alter table p1 alter constraint p1_a_check not enforced; --ok +select conname, conenforced, convalidated, conrelid::regclass +from pg_constraint +where conname = 'p1_a_check' and contype = 'c' +order by conrelid::regclass::text collate "C"; + conname | conenforced | convalidated | conrelid +------------+-------------+--------------+---------- + p1_a_check | f | f | p1 + p1_a_check | t | t | p1_c1 + p1_a_check | f | f | p1_c2 + p1_a_check | t | t | p2 +(4 rows) + +alter table p1_c1 alter constraint p1_a_check not enforced; --error +ERROR: cannot mark inherited constraint "p1_a_check" as NOT ENFORCED because matching constraint on parent table "p2" is ENFORCED +alter table p2 alter constraint p1_a_check not enforced; --ok +alter table p1_c1 alter constraint p1_a_check not enforced; --ok +drop table p1, p2 cascade; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table p1_c1 +drop cascades to table p1_c2 +-- recursive NOT ENFORCED can change all matching enforced parents together +create table gp(a int constraint gp_a_check check (a > 0) enforced); +create table p1() inherits (gp); +create table p2() inherits (gp); +create table p1_c1() inherits (p1, p2); +NOTICE: merging multiple inherited definitions of column "a" +alter table gp alter constraint gp_a_check not enforced; --ok +select conname, conenforced, convalidated, conrelid::regclass +from pg_constraint +where conname = 'gp_a_check' and contype = 'c' +order by conrelid::regclass::text collate "C"; + conname | conenforced | convalidated | conrelid +------------+-------------+--------------+---------- + gp_a_check | f | f | gp + gp_a_check | f | f | p1 + gp_a_check | f | f | p1_c1 + gp_a_check | f | f | p2 +(4 rows) + +drop table gp cascade; +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table p1 +drop cascades to table p2 +drop cascades to table p1_c1 +-- recursive NOT ENFORCED can change a direct-plus-indirect diamond together +create table gp(a int constraint gp_a_check check (a > 0) enforced); +create table p1_c1() inherits (gp); +create table p1() inherits (gp); +alter table p1_c1 inherit p1; +alter table gp alter constraint gp_a_check not enforced; --ok +select conname, conenforced, convalidated, conrelid::regclass +from pg_constraint +where conname = 'gp_a_check' and contype = 'c' +order by conrelid::regclass::text collate "C"; + conname | conenforced | convalidated | conrelid +------------+-------------+--------------+---------- + gp_a_check | f | f | gp + gp_a_check | f | f | p1 + gp_a_check | f | f | p1_c1 +(3 rows) + +drop table gp cascade; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table p1 +drop cascades to table p1_c1 --for "no inherit" check constraint, it will not recurse to child table create table p1(f1 int constraint p1_a_check check (f1 > 0) no inherit not enforced); create table p1_c1(f1 int constraint p1_a_check check (f1 > 0) not enforced); diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql index dc133b124bb..9705962eb9f 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -309,8 +309,9 @@ select * from check_constraint_status; alter table parted_ch_2 alter constraint cc_2 enforced; --error delete from parted_ch where a = 16; alter table parted_ch_2 alter constraint cc_2 enforced; -alter table parted_ch_2 alter constraint cc not enforced; -alter table parted_ch_2 alter constraint cc_1 not enforced; +alter table parted_ch_2 alter constraint cc not enforced; --error +alter table only parted_ch_2 alter constraint cc not enforced; --error +alter table parted_ch_2 alter constraint cc_1 not enforced; --error alter table parted_ch_2 alter constraint cc_2 not enforced; --check these CHECK constraint status again diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 8f986904389..a7c618e06e3 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -535,6 +535,53 @@ where conname = 'inh_check_constraint3' and contype = 'c' 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); +alter table p1_c1 alter constraint p1_a_check not enforced; --error +alter table p1 alter constraint p1_a_check not enforced; --ok +alter table p1_c1 alter constraint p1_a_check not enforced; --ok +drop table p1 cascade; + +-- recursive NOT ENFORCED merges with ENFORCED constraints from other parents +create table p1(a int constraint p1_a_check check (a > 0) enforced); +create table p2(a int constraint p1_a_check check (a > 0) enforced); +create table p1_c1() inherits (p1, p2); +create table p1_c2() inherits (p1); +alter table p1 alter constraint p1_a_check not enforced; --ok +select conname, conenforced, convalidated, conrelid::regclass +from pg_constraint +where conname = 'p1_a_check' and contype = 'c' +order by conrelid::regclass::text collate "C"; +alter table p1_c1 alter constraint p1_a_check not enforced; --error +alter table p2 alter constraint p1_a_check not enforced; --ok +alter table p1_c1 alter constraint p1_a_check not enforced; --ok +drop table p1, p2 cascade; + +-- recursive NOT ENFORCED can change all matching enforced parents together +create table gp(a int constraint gp_a_check check (a > 0) enforced); +create table p1() inherits (gp); +create table p2() inherits (gp); +create table p1_c1() inherits (p1, p2); +alter table gp alter constraint gp_a_check not enforced; --ok +select conname, conenforced, convalidated, conrelid::regclass +from pg_constraint +where conname = 'gp_a_check' and contype = 'c' +order by conrelid::regclass::text collate "C"; +drop table gp cascade; + +-- recursive NOT ENFORCED can change a direct-plus-indirect diamond together +create table gp(a int constraint gp_a_check check (a > 0) enforced); +create table p1_c1() inherits (gp); +create table p1() inherits (gp); +alter table p1_c1 inherit p1; +alter table gp alter constraint gp_a_check not enforced; --ok +select conname, conenforced, convalidated, conrelid::regclass +from pg_constraint +where conname = 'gp_a_check' and contype = 'c' +order by conrelid::regclass::text collate "C"; +drop table gp cascade; + --for "no inherit" check constraint, it will not recurse to child table create table p1(f1 int constraint p1_a_check check (f1 > 0) no inherit not enforced); create table p1_c1(f1 int constraint p1_a_check check (f1 > 0) not enforced); -- 2.50.1 (Apple Git-155)