From 53604bd9b5f9164000ad4b36e859f3df7b606c89 Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 1 Jun 2026 21:00:54 +0800 Subject: [PATCH v4 1/1] 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 equivalent 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: Discussion: https://postgr.es/m/E74C57FA-1DD0-4C8E-8FB1-538034752592@gmail.com --- src/backend/commands/tablecmds.c | 185 +++++++++++++++++++++- src/test/regress/expected/constraints.out | 12 +- src/test/regress/expected/inherit.out | 56 +++++++ src/test/regress/sql/constraints.sql | 5 +- src/test/regress/sql/inherit.sql | 34 ++++ 5 files changed, 282 insertions(+), 10 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a1845240a98..75ade053b38 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, @@ -12482,9 +12489,13 @@ ATExecAlterConstraintInternal(List **wqueue, ATAlterConstraint *cmdcon, InvalidOid, InvalidOid, InvalidOid, InvalidOid); else if (currcon->contype == CONSTRAINT_CHECK) + { + List *changing_conids = NIL; + changed = ATExecAlterCheckConstrEnforceability(wqueue, cmdcon, conrel, contuple, recurse, false, - lockmode); + &changing_conids, lockmode); + } } else if (cmdcon->alterDeferrability && ATExecAlterConstrDeferrability(wqueue, cmdcon, conrel, tgrel, rel, @@ -12671,7 +12682,9 @@ 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; @@ -12693,6 +12706,66 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, */ rel = table_open(currcon->conrelid, NoLock); + /* + * 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) + { + Oid enforced_parentoid = InvalidOid; + + bool enforced_parent_constr = false; + + /* + * For partitioned tables, a one-time verification is needed to ensure + * that a partition's constraint cannot be marked as unenforced if the + * parent's constraint remains enforced. + * + * For traditional table inheritance, if a constraint is unenforced + * but its parent constraint is enforced, both this constraint and its + * descendants must be marked as enforced. + */ + if ((rel->rd_rel->relispartition && !recursing) || + (!rel->rd_rel->relispartition && rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)) + { + enforced_parent_constr = ATCheckCheckConstrHasEnforcedParent(conrel, rel, contuple, + changing_conids, + &enforced_parentoid); + + if (enforced_parent_constr) + cmdcon->is_enforced = true; + } + + if (enforced_parent_constr && !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))); + + /* + * Build the set of equivalent CHECK constraints that this command + * will attempt to change before visiting descendants. Each descendant + * is compared to the original target constraint, not only by name. + * The root itself has already been checked above. + */ + *changing_conids = lappend_oid(*changing_conids, currcon->oid); + } + + /* + * Update to the merged enforceability if needed. This may differ from the + * requested enforceability when another matching parent constraint + * remains enforced. + */ if (currcon->conenforced != cmdcon->is_enforced) { AlterConstrUpdateConstraintEntry(cmdcon, conrel, contuple); @@ -12702,7 +12775,8 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, /* * 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. @@ -12737,6 +12811,7 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, AlterCheckConstrEnforceabilityRecurse(wqueue, cmdcon, conrel, childoid, false, true, + changing_conids, lockmode); } } @@ -12788,6 +12863,7 @@ static void AlterCheckConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon, Relation conrel, Oid conrelid, bool recurse, bool recursing, + List **changing_conids, LOCKMODE lockmode) { SysScanDesc pscan; @@ -12816,12 +12892,113 @@ AlterCheckConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon, errmsg("constraint \"%s\" of relation \"%s\" does not exist", cmdcon->conname, get_rel_name(conrelid))); + if (!cmdcon->is_enforced) + { + Form_pg_constraint currcon; + + currcon = (Form_pg_constraint) GETSTRUCT(childtup); + + if (currcon->conenforced) + *changing_conids = list_append_unique_oid(*changing_conids, + currcon->oid); + } + 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..b649d10e36b 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..5b87083a870 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1479,6 +1479,62 @@ 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" +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 | t | t | p2 +(3 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. Since two parents are not enforced too. +drop table p1, p2 cascade; +NOTICE: drop cascades to table p1_c1 +-- 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 --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..26a9ac78c96 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..679b0b2d969 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -535,6 +535,40 @@ 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); +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. Since two parents are not enforced too. +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; + --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.34.1