public inbox for [email protected]help / color / mirror / Atom feed
Fix bug of CHECK constraint enforceability recursion 12+ messages / 3 participants [nested] [flat]
* Fix bug of CHECK constraint enforceability recursion @ 2026-05-26 03:51 Chao Li <[email protected]> 0 siblings, 2 replies; 12+ messages in thread From: Chao Li @ 2026-05-26 03:51 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; jian he <[email protected]> Hi, I just tested “Add support for altering CHECK constraint enforceability” and found an issue where recursion is not handled properly. Here is a repro with inheritance tables: ``` evantest=# create table p(a int constraint ck check (a > 0) enforced); CREATE TABLE evantest=# create table c() inherits (p); CREATE TABLE evantest=# alter table c alter constraint ck not enforced; ALTER TABLE evantest=# insert into c values (-1); INSERT 0 1 evantest=# alter table p alter constraint ck enforced; ALTER TABLE evantest=# insert into c values (-2); INSERT 0 1 evantest=# select * from p; a ---- -1 -2 (2 rows) ``` In this repro, the constraint on parent table p is already ENFORCED, but the constraint on child table c was altered to NOT ENFORCED. So when altering p to ENFORCED again, it didn't recurse to c. The same problem can happen with partitioned tables as well: ``` evantest=# create table p (a int, constraint ck check (a > 0) enforced) partition by range (a); CREATE TABLE evantest=# create table p1 partition of p for values from (-100) to (100); CREATE TABLE evantest=# insert into p1 values (-1); ERROR: new row for relation "p1" violates check constraint "ck" DETAIL: Failing row contains (-1). evantest=# alter table p1 alter constraint ck not enforced; ALTER TABLE evantest=# insert into p1 values (-1); INSERT 0 1 evantest=# alter table p alter constraint ck enforced; ALTER TABLE evantest=# insert into p1 values (-2); INSERT 0 1 evantest=# evantest=# select * from p; a ---- -1 -2 (2 rows) ``` For the solution, I think we should always recurse to descendant tables unless the constraint is NO INHERIT, because both partitioned tables and inheritance children can currently be altered to have different enforceability. So we cannot rely on whether the parent constraint itself was changed. See the attached patch for details. I also added regress test cases for the fix. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v1-0001-Fix-CHECK-constraint-enforceability-recursion.patch (8.3K, 2-v1-0001-Fix-CHECK-constraint-enforceability-recursion.patch) download | inline diff: From 2755f0abcda9e0dfb38b715b7938ca50d9194920 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Tue, 26 May 2026 11:03:15 +0800 Subject: [PATCH v1] Fix CHECK constraint enforceability recursion ALTER TABLE ... ALTER CONSTRAINT ... ENFORCED skipped recursion when the constraint on the target table was already enforced. That assumed descendant CHECK constraints could not have different enforceability, but both regular inheritance and some partition cases can have descendants with different conenforced states. Recurse for inheritable CHECK constraints regardless of whether the target constraint row itself changed, while still skipping NO INHERIT constraints. This ensures descendant constraints are updated and validated when needed. Add regression tests for both regular inheritance and partitioning: change a descendant CHECK constraint to NOT ENFORCED while the parent remains ENFORCED, then re-enforce the parent and verify that the operation recurses to the descendant. Author: Chao Li <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/ --- src/backend/commands/tablecmds.c | 15 ++++++------- src/test/regress/expected/constraints.out | 26 +++++++++++++++++++++++ src/test/regress/expected/inherit.out | 24 +++++++++++++++++++++ src/test/regress/sql/constraints.sql | 18 ++++++++++++++++ src/test/regress/sql/inherit.sql | 15 +++++++++++++ 5 files changed, 90 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1e0bacf85fc..05289207305 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12700,14 +12700,13 @@ 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. - * 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. - */ - if (!cmdcon->is_enforced || changed) + * Recurse for inheritable constraints even when this constraint already + * has the requested enforceability. For both partitioning and regular + * inheritance, descendant constraints can have different enforceability + * and still need to be updated. Only NO INHERIT constraints do not + * recurse. + */ + if (!currcon->connoinherit) { /* * If we're recursing, the parent has already done this, so skip it. diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index e54fec7fb57..579882e75f6 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -468,6 +468,32 @@ select * from check_constraint_status; drop table parted_ch; drop view check_constraint_status; +-- an already enforced partitioned parent must still recurse to partitions +create table parted_ch_recurse( + a int, + constraint cc check (a > 0) enforced +) partition by range(a); +create table parted_ch_recurse_1 partition of parted_ch_recurse for values from (-100) to (100); +alter table parted_ch_recurse_1 alter constraint cc not enforced; +insert into parted_ch_recurse_1 values(-1); +alter table parted_ch_recurse alter constraint cc enforced; --error +ERROR: check constraint "cc" of relation "parted_ch_recurse_1" is violated by some row +delete from parted_ch_recurse_1 where a = -1; +alter table parted_ch_recurse alter constraint cc enforced; +select conrelid::regclass, conenforced, convalidated +from pg_constraint +where conname = 'cc' and conrelid::regclass::text like 'parted_ch_recurse%' +order by conrelid::regclass::text; + conrelid | conenforced | convalidated +---------------------+-------------+-------------- + parted_ch_recurse | t | t + parted_ch_recurse_1 | t | t +(2 rows) + +insert into parted_ch_recurse_1 values(-1); --error +ERROR: new row for relation "parted_ch_recurse_1" violates check constraint "cc" +DETAIL: Failing row contains (-1). +drop table parted_ch_recurse; -- -- Primary keys -- diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 3d8e8d8afd2..05a1604d609 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1479,6 +1479,30 @@ 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 already enforced parent must still recurse to regular inheritance children +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; +insert into p1_c1 values(-1); +alter table p1 alter constraint p1_a_check enforced; --error +ERROR: check constraint "p1_a_check" of relation "p1_c1" is violated by some row +delete from p1_c1; +alter table p1 alter constraint p1_a_check 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 | t | t | p1 + p1_a_check | t | t | p1_c1 +(2 rows) + +insert into p1_c1 values(-1); --error +ERROR: new row for relation "p1_c1" violates check constraint "p1_a_check" +DETAIL: Failing row contains (-1). +drop table p1 cascade; +NOTICE: 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..c84eaa44b9c 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -318,6 +318,24 @@ select * from check_constraint_status; drop table parted_ch; drop view check_constraint_status; +-- an already enforced partitioned parent must still recurse to partitions +create table parted_ch_recurse( + a int, + constraint cc check (a > 0) enforced +) partition by range(a); +create table parted_ch_recurse_1 partition of parted_ch_recurse for values from (-100) to (100); +alter table parted_ch_recurse_1 alter constraint cc not enforced; +insert into parted_ch_recurse_1 values(-1); +alter table parted_ch_recurse alter constraint cc enforced; --error +delete from parted_ch_recurse_1 where a = -1; +alter table parted_ch_recurse alter constraint cc enforced; +select conrelid::regclass, conenforced, convalidated +from pg_constraint +where conname = 'cc' and conrelid::regclass::text like 'parted_ch_recurse%' +order by conrelid::regclass::text; +insert into parted_ch_recurse_1 values(-1); --error +drop table parted_ch_recurse; + -- -- Primary keys -- diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 8f986904389..8a7417765aa 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -535,6 +535,21 @@ where conname = 'inh_check_constraint3' and contype = 'c' order by conrelid::regclass::text collate "C"; drop table p1 cascade; +-- an already enforced parent must still recurse to regular inheritance children +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; +insert into p1_c1 values(-1); +alter table p1 alter constraint p1_a_check enforced; --error +delete from p1_c1; +alter table p1 alter constraint p1_a_check 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"; +insert into p1_c1 values(-1); --error +drop table p1 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) ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-26 05:48 Chao Li <[email protected]> parent: Chao Li <[email protected]> 1 sibling, 0 replies; 12+ messages in thread From: Chao Li @ 2026-05-26 05:48 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; jian he <[email protected]> > On May 26, 2026, at 11:51, Chao Li <[email protected]> wrote: > > Hi, > > I just tested “Add support for altering CHECK constraint enforceability” and found an issue where recursion is not handled properly. > > Here is a repro with inheritance tables: > ``` > evantest=# create table p(a int constraint ck check (a > 0) enforced); > CREATE TABLE > evantest=# create table c() inherits (p); > CREATE TABLE > evantest=# alter table c alter constraint ck not enforced; > ALTER TABLE > evantest=# insert into c values (-1); > INSERT 0 1 > evantest=# alter table p alter constraint ck enforced; > ALTER TABLE > evantest=# insert into c values (-2); > INSERT 0 1 > evantest=# select * from p; > a > ---- > -1 > -2 > (2 rows) > ``` > > In this repro, the constraint on parent table p is already ENFORCED, but the constraint on child table c was altered to NOT ENFORCED. So when altering p to ENFORCED again, it didn't recurse to c. > > The same problem can happen with partitioned tables as well: > ``` > evantest=# create table p (a int, constraint ck check (a > 0) enforced) partition by range (a); > CREATE TABLE > evantest=# create table p1 partition of p for values from (-100) to (100); > CREATE TABLE > evantest=# insert into p1 values (-1); > ERROR: new row for relation "p1" violates check constraint "ck" > DETAIL: Failing row contains (-1). > evantest=# alter table p1 alter constraint ck not enforced; > ALTER TABLE > evantest=# insert into p1 values (-1); > INSERT 0 1 > evantest=# alter table p alter constraint ck enforced; > ALTER TABLE > evantest=# insert into p1 values (-2); > INSERT 0 1 > evantest=# > evantest=# select * from p; > a > ---- > -1 > -2 > (2 rows) > ``` > > For the solution, I think we should always recurse to descendant tables unless the constraint is NO INHERIT, because both partitioned tables and inheritance children can currently be altered to have different enforceability. So we cannot rely on whether the parent constraint itself was changed. > > See the attached patch for details. I also added regress test cases for the fix. > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > > > > > <v1-0001-Fix-CHECK-constraint-enforceability-recursion.patch> Merged the doc change from [1] into this thread as they are for the same feature. [1] https://postgr.es/m/[email protected] Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v2-0001-Fix-CHECK-constraint-enforceability-recursion.patch (8.4K, 2-v2-0001-Fix-CHECK-constraint-enforceability-recursion.patch) download | inline diff: From c28ecbe5ac850fb77c9f0b859c7d6a2b867cc4b9 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Tue, 26 May 2026 11:03:15 +0800 Subject: [PATCH v2 1/2] Fix CHECK constraint enforceability recursion ALTER TABLE ... ALTER CONSTRAINT ... ENFORCED skipped recursion when the constraint on the target table was already enforced. That assumed descendant CHECK constraints could not have different enforceability, but both regular inheritance and some partition cases can have descendants with different conenforced states. Recurse for inheritable CHECK constraints regardless of whether the target constraint row itself changed, while still skipping NO INHERIT constraints. This ensures descendant constraints are updated and validated when needed. Add regression tests for both regular inheritance and partitioning: change a descendant CHECK constraint to NOT ENFORCED while the parent remains ENFORCED, then re-enforce the parent and verify that the operation recurses to the descendant. Author: Chao Li <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/tablecmds.c | 15 ++++++------- src/test/regress/expected/constraints.out | 26 +++++++++++++++++++++++ src/test/regress/expected/inherit.out | 24 +++++++++++++++++++++ src/test/regress/sql/constraints.sql | 18 ++++++++++++++++ src/test/regress/sql/inherit.sql | 15 +++++++++++++ 5 files changed, 90 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1e0bacf85fc..05289207305 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12700,14 +12700,13 @@ 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. - * 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. - */ - if (!cmdcon->is_enforced || changed) + * Recurse for inheritable constraints even when this constraint already + * has the requested enforceability. For both partitioning and regular + * inheritance, descendant constraints can have different enforceability + * and still need to be updated. Only NO INHERIT constraints do not + * recurse. + */ + if (!currcon->connoinherit) { /* * If we're recursing, the parent has already done this, so skip it. diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index e54fec7fb57..579882e75f6 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -468,6 +468,32 @@ select * from check_constraint_status; drop table parted_ch; drop view check_constraint_status; +-- an already enforced partitioned parent must still recurse to partitions +create table parted_ch_recurse( + a int, + constraint cc check (a > 0) enforced +) partition by range(a); +create table parted_ch_recurse_1 partition of parted_ch_recurse for values from (-100) to (100); +alter table parted_ch_recurse_1 alter constraint cc not enforced; +insert into parted_ch_recurse_1 values(-1); +alter table parted_ch_recurse alter constraint cc enforced; --error +ERROR: check constraint "cc" of relation "parted_ch_recurse_1" is violated by some row +delete from parted_ch_recurse_1 where a = -1; +alter table parted_ch_recurse alter constraint cc enforced; +select conrelid::regclass, conenforced, convalidated +from pg_constraint +where conname = 'cc' and conrelid::regclass::text like 'parted_ch_recurse%' +order by conrelid::regclass::text; + conrelid | conenforced | convalidated +---------------------+-------------+-------------- + parted_ch_recurse | t | t + parted_ch_recurse_1 | t | t +(2 rows) + +insert into parted_ch_recurse_1 values(-1); --error +ERROR: new row for relation "parted_ch_recurse_1" violates check constraint "cc" +DETAIL: Failing row contains (-1). +drop table parted_ch_recurse; -- -- Primary keys -- diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 3d8e8d8afd2..05a1604d609 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1479,6 +1479,30 @@ 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 already enforced parent must still recurse to regular inheritance children +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; +insert into p1_c1 values(-1); +alter table p1 alter constraint p1_a_check enforced; --error +ERROR: check constraint "p1_a_check" of relation "p1_c1" is violated by some row +delete from p1_c1; +alter table p1 alter constraint p1_a_check 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 | t | t | p1 + p1_a_check | t | t | p1_c1 +(2 rows) + +insert into p1_c1 values(-1); --error +ERROR: new row for relation "p1_c1" violates check constraint "p1_a_check" +DETAIL: Failing row contains (-1). +drop table p1 cascade; +NOTICE: 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..c84eaa44b9c 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -318,6 +318,24 @@ select * from check_constraint_status; drop table parted_ch; drop view check_constraint_status; +-- an already enforced partitioned parent must still recurse to partitions +create table parted_ch_recurse( + a int, + constraint cc check (a > 0) enforced +) partition by range(a); +create table parted_ch_recurse_1 partition of parted_ch_recurse for values from (-100) to (100); +alter table parted_ch_recurse_1 alter constraint cc not enforced; +insert into parted_ch_recurse_1 values(-1); +alter table parted_ch_recurse alter constraint cc enforced; --error +delete from parted_ch_recurse_1 where a = -1; +alter table parted_ch_recurse alter constraint cc enforced; +select conrelid::regclass, conenforced, convalidated +from pg_constraint +where conname = 'cc' and conrelid::regclass::text like 'parted_ch_recurse%' +order by conrelid::regclass::text; +insert into parted_ch_recurse_1 values(-1); --error +drop table parted_ch_recurse; + -- -- Primary keys -- diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 8f986904389..8a7417765aa 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -535,6 +535,21 @@ where conname = 'inh_check_constraint3' and contype = 'c' order by conrelid::regclass::text collate "C"; drop table p1 cascade; +-- an already enforced parent must still recurse to regular inheritance children +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; +insert into p1_c1 values(-1); +alter table p1 alter constraint p1_a_check enforced; --error +delete from p1_c1; +alter table p1 alter constraint p1_a_check 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"; +insert into p1_c1 values(-1); --error +drop table p1 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) [application/octet-stream] v2-0002-doc-Clarify-ALTER-CONSTRAINT-enforceability-behav.patch (2.2K, 3-v2-0002-doc-Clarify-ALTER-CONSTRAINT-enforceability-behav.patch) download | inline diff: From 060abd87eddc336fe4a73c980f095aeda6522b3c Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Mon, 25 May 2026 16:59:21 +0800 Subject: [PATCH v2 2/2] doc: Clarify ALTER CONSTRAINT enforceability behavior The ALTER TABLE documentation said that FOREIGN KEY and CHECK constraints may be altered, but did not distinguish between deferrability and enforceability attributes. Clarify that deferrability attributes can currently be altered only for FOREIGN KEY constraints, while enforceability can be altered for both FOREIGN KEY and CHECK constraints. Also document that setting a constraint to ENFORCED verifies existing rows and resumes checking new or updated rows. Author: Chao Li <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/[email protected] Discussion: https://postgr.es/m/[email protected] --- doc/src/sgml/ref/alter_table.sgml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index dec34337d1a..af247d82902 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -586,8 +586,18 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM <listitem> <para> This form alters the attributes of a constraint that was previously - created. Currently <literal>FOREIGN KEY</literal> and <literal>CHECK</literal> - constraints may be altered in this fashion, but see below. + created. Currently, the deferrability attributes can be altered only + for <literal>FOREIGN KEY</literal> constraints. The enforceability + attribute can be altered for <literal>FOREIGN KEY</literal> and + <literal>CHECK</literal> constraints. + </para> + + <para> + Setting a constraint to <literal>NOT ENFORCED</literal> causes the + database system to stop checking it for new or updated rows. Setting + a constraint to <literal>ENFORCED</literal> causes the database system + to verify that existing rows satisfy the constraint and to check it + for new or updated rows. </para> </listitem> </varlistentry> -- 2.50.1 (Apple Git-155) ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-26 06:05 jian he <[email protected]> parent: Chao Li <[email protected]> 1 sibling, 1 reply; 12+ messages in thread From: jian he @ 2026-05-26 06:05 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Andrew Dunstan <[email protected]> On Tue, May 26, 2026 at 11:51 AM Chao Li <[email protected]> wrote: > > Hi, > > I just tested “Add support for altering CHECK constraint enforceability” and found an issue where recursion is not handled properly. > > Here is a repro with inheritance tables: > ``` > evantest=# create table p(a int constraint ck check (a > 0) enforced); > CREATE TABLE > evantest=# create table c() inherits (p); > CREATE TABLE > evantest=# alter table c alter constraint ck not enforced; > ALTER TABLE > evantest=# insert into c values (-1); > INSERT 0 1 > evantest=# alter table p alter constraint ck enforced; > ALTER TABLE > evantest=# insert into c values (-2); > INSERT 0 1 > evantest=# select * from p; > a > ---- > -1 > -2 > (2 rows) > ``` > > In this repro, the constraint on parent table p is already ENFORCED, but the constraint on child table c was altered to NOT ENFORCED. So when altering p to ENFORCED again, it didn't recurse to c. > > The same problem can happen with partitioned tables as well: > ``` Hi. In MergeConstraintsIntoExisting, we have: /* * A NOT ENFORCED child constraint cannot be merged with an * ENFORCED parent constraint. However, the reverse is allowed, * where the child constraint is ENFORCED. */ if (parent_con->conenforced && !child_con->conenforced) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), errmsg("constraint \"%s\" conflicts with NOT ENFORCED constraint on child table \"%s\"", NameStr(child_con->conname), RelationGetRelationName(child_rel)))); MergeWithExistingConstraint, we have comments like: /* * If the child constraint is required to be enforced while the parent * 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. */ So other commands (CREATE TABLE, ALTER TABLE ATTACH PARTITION) do not expect a state where the parent constraint is enforced but the child constraint is not. We can now reach this state via ALTER TABLE ALTER CONSTRAINT. We don't need to worry about Foreign Key Constraints because the foreign key constraint's conparentid is valid, therefore we cannot directly alter a partition's FK constraint. StoreRelCheck->CreateConstraintEntry comments ``/* no parent constraint */`` means that each CHECK constraint is on its own. Overall, i tend to think that we should reject ALTER TABLE ALTER CONSTRAINT if it would result in the parent constraint being enforced while the child constraint is not enforced. -- jian https://www.enterprisedb.com/ ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-26 06:27 Chao Li <[email protected]> parent: jian he <[email protected]> 0 siblings, 2 replies; 12+ messages in thread From: Chao Li @ 2026-05-26 06:27 UTC (permalink / raw) To: jian he <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Andrew Dunstan <[email protected]> > On May 26, 2026, at 14:05, jian he <[email protected]> wrote: > > On Tue, May 26, 2026 at 11:51 AM Chao Li <[email protected]> wrote: >> >> Hi, >> >> I just tested “Add support for altering CHECK constraint enforceability” and found an issue where recursion is not handled properly. >> >> Here is a repro with inheritance tables: >> ``` >> evantest=# create table p(a int constraint ck check (a > 0) enforced); >> CREATE TABLE >> evantest=# create table c() inherits (p); >> CREATE TABLE >> evantest=# alter table c alter constraint ck not enforced; >> ALTER TABLE >> evantest=# insert into c values (-1); >> INSERT 0 1 >> evantest=# alter table p alter constraint ck enforced; >> ALTER TABLE >> evantest=# insert into c values (-2); >> INSERT 0 1 >> evantest=# select * from p; >> a >> ---- >> -1 >> -2 >> (2 rows) >> ``` >> >> In this repro, the constraint on parent table p is already ENFORCED, but the constraint on child table c was altered to NOT ENFORCED. So when altering p to ENFORCED again, it didn't recurse to c. >> >> The same problem can happen with partitioned tables as well: >> ``` > > Hi. > > In MergeConstraintsIntoExisting, we have: > /* > * A NOT ENFORCED child constraint cannot be merged with an > * ENFORCED parent constraint. However, the reverse is allowed, > * where the child constraint is ENFORCED. > */ > if (parent_con->conenforced && !child_con->conenforced) > ereport(ERROR, > (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), > errmsg("constraint \"%s\" conflicts with NOT ENFORCED > constraint on child table \"%s\"", > NameStr(child_con->conname), > RelationGetRelationName(child_rel)))); > > MergeWithExistingConstraint, we have comments like: > /* > * If the child constraint is required to be enforced while the parent > * 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. > */ > > So other commands (CREATE TABLE, ALTER TABLE ATTACH PARTITION) do not expect a > state where the parent constraint is enforced but the child constraint is not. > We can now reach this state via ALTER TABLE ALTER CONSTRAINT. > > We don't need to worry about Foreign Key Constraints because the > foreign key constraint's conparentid is valid, therefore we cannot > directly alter a partition's FK constraint. > StoreRelCheck->CreateConstraintEntry comments ``/* no parent > constraint */`` means that each CHECK constraint is on its own. > > Overall, i tend to think that we should reject ALTER TABLE ALTER > CONSTRAINT if it > would result in the parent constraint being enforced while the child constraint > is not enforced. > I am not against the idea of "rejecting ALTER TABLE ALTER CONSTRAINT if it would result in the parent constraint being enforced while the child constrain is not enforced", but I’m afraid it’s too late for PG19. So, I guess we still need to fix the issue for 19, right? Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-26 07:32 Álvaro Herrera <[email protected]> parent: Chao Li <[email protected]> 1 sibling, 1 reply; 12+ messages in thread From: Álvaro Herrera @ 2026-05-26 07:32 UTC (permalink / raw) To: Chao Li <[email protected]>; jian he <[email protected]>; +Cc: L. pgsql-hackers <[email protected]>; Andrew Dunstan <[email protected]> On 2026-05-26, Chao Li wrote: >> On May 26, 2026, at 14:05, jian he <[email protected]> wrote: >> Overall, i tend to think that we should reject ALTER TABLE ALTER >> CONSTRAINT if it >> would result in the parent constraint being enforced while the child constraint >> is not enforced. Yeah. > I am not against the idea of "rejecting ALTER TABLE ALTER CONSTRAINT if > it would result in the parent constraint being enforced while the child > constrain is not enforced", but I’m afraid it’s too late for PG19. So, > I guess we still need to fix the issue for 19, right? I think this is a bug that we need to fix in 19 as well — I mean we should reject the ALTER TABLE. -- Álvaro Herrera ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-26 07:44 Chao Li <[email protected]> parent: Chao Li <[email protected]> 1 sibling, 0 replies; 12+ messages in thread From: Chao Li @ 2026-05-26 07:44 UTC (permalink / raw) To: jian he <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Andrew Dunstan <[email protected]> > On May 26, 2026, at 14:27, Chao Li <[email protected]> wrote: > > > >> On May 26, 2026, at 14:05, jian he <[email protected]> wrote: >> >> On Tue, May 26, 2026 at 11:51 AM Chao Li <[email protected]> wrote: >>> >>> Hi, >>> >>> I just tested “Add support for altering CHECK constraint enforceability” and found an issue where recursion is not handled properly. >>> >>> Here is a repro with inheritance tables: >>> ``` >>> evantest=# create table p(a int constraint ck check (a > 0) enforced); >>> CREATE TABLE >>> evantest=# create table c() inherits (p); >>> CREATE TABLE >>> evantest=# alter table c alter constraint ck not enforced; >>> ALTER TABLE >>> evantest=# insert into c values (-1); >>> INSERT 0 1 >>> evantest=# alter table p alter constraint ck enforced; >>> ALTER TABLE >>> evantest=# insert into c values (-2); >>> INSERT 0 1 >>> evantest=# select * from p; >>> a >>> ---- >>> -1 >>> -2 >>> (2 rows) >>> ``` >>> >>> In this repro, the constraint on parent table p is already ENFORCED, but the constraint on child table c was altered to NOT ENFORCED. So when altering p to ENFORCED again, it didn't recurse to c. >>> >>> The same problem can happen with partitioned tables as well: >>> ``` >> >> Hi. >> >> In MergeConstraintsIntoExisting, we have: >> /* >> * A NOT ENFORCED child constraint cannot be merged with an >> * ENFORCED parent constraint. However, the reverse is allowed, >> * where the child constraint is ENFORCED. >> */ >> if (parent_con->conenforced && !child_con->conenforced) >> ereport(ERROR, >> (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), >> errmsg("constraint \"%s\" conflicts with NOT ENFORCED >> constraint on child table \"%s\"", >> NameStr(child_con->conname), >> RelationGetRelationName(child_rel)))); >> >> MergeWithExistingConstraint, we have comments like: >> /* >> * If the child constraint is required to be enforced while the parent >> * 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. >> */ >> >> So other commands (CREATE TABLE, ALTER TABLE ATTACH PARTITION) do not expect a >> state where the parent constraint is enforced but the child constraint is not. >> We can now reach this state via ALTER TABLE ALTER CONSTRAINT. >> >> We don't need to worry about Foreign Key Constraints because the >> foreign key constraint's conparentid is valid, therefore we cannot >> directly alter a partition's FK constraint. >> StoreRelCheck->CreateConstraintEntry comments ``/* no parent >> constraint */`` means that each CHECK constraint is on its own. >> >> Overall, i tend to think that we should reject ALTER TABLE ALTER >> CONSTRAINT if it >> would result in the parent constraint being enforced while the child constraint >> is not enforced. >> > > I am not against the idea of "rejecting ALTER TABLE ALTER CONSTRAINT if it would result in the parent constraint being enforced while the child constrain is not enforced", but I’m afraid it’s too late for PG19. So, I guess we still need to fix the issue for 19, right? > I thought this over, and I changed my mind. The same rule should apply to both partitioned tables and regular inheritance: * parent CHECK enforced + child CHECK not enforced = reject * parent CHECK not enforced + child CHECK enforced = allow That matches the existing merge/attach behavior. Also, this invariant could not be broken through normal SQL in PG18, because PG18 does not support ALTER TABLE ... ALTER CONSTRAINT ... [NOT] ENFORCED for CHECK constraints. So we should not introduce a new way to break it in PG19. I will rework the patch forwards the “reject” direction. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-26 07:46 Chao Li <[email protected]> parent: Álvaro Herrera <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Chao Li @ 2026-05-26 07:46 UTC (permalink / raw) To: Álvaro Herrera <[email protected]>; +Cc: jian he <[email protected]>; L. pgsql-hackers <[email protected]>; Andrew Dunstan <[email protected]> > On May 26, 2026, at 15:32, Álvaro Herrera <[email protected]> wrote: > > On 2026-05-26, Chao Li wrote: >>> On May 26, 2026, at 14:05, jian he <[email protected]> wrote: > >>> Overall, i tend to think that we should reject ALTER TABLE ALTER >>> CONSTRAINT if it >>> would result in the parent constraint being enforced while the child constraint >>> is not enforced. > > Yeah. > >> I am not against the idea of "rejecting ALTER TABLE ALTER CONSTRAINT if >> it would result in the parent constraint being enforced while the child >> constrain is not enforced", but I’m afraid it’s too late for PG19. So, >> I guess we still need to fix the issue for 19, right? > > I think this is a bug that we need to fix in 19 as well — I mean we should reject the ALTER TABLE. > > -- > Álvaro Herrera Thanks for your comment. Let me rework the patch. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-26 08:32 jian he <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: jian he @ 2026-05-26 08:32 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; L. pgsql-hackers <[email protected]>; Andrew Dunstan <[email protected]> On Tue, May 26, 2026 at 3:47 PM Chao Li <[email protected]> wrote: > > > > > I think this is a bug that we need to fix in 19 as well — I mean we should reject the ALTER TABLE. > > > > -- > > Álvaro Herrera > > Thanks for your comment. Let me rework the patch. > Hi. Here are the comments placed in ATExecAlterCheckConstrEnforceability I came up with: + /* + * If the check constraint qual definitions match but their enforcement + * statuses conflict (parent enforced, child unenforced), it creates + * ambiguity around how insert operations should handle the mismatch. + * Therefore, we should avoid states where the parent check constraint is + * enforced while the child is not. We actually enforced this within + * MergeConstraintsIntoExisting and MergeWithExistingConstraint. + */ + if (currcon->coninhcount > 0 && !recursing) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot alter inherited constraint \"%s\" of relation \"%s\" enforciability", + NameStr(currcon->conname), RelationGetRelationName(rel))); -- jian https://www.enterprisedb.com/ Attachments: [text/x-patch] v1-0001-disallow-alter-enforciability-of-inherited-check-constraint.patch (5.4K, 2-v1-0001-disallow-alter-enforciability-of-inherited-check-constraint.patch) download | inline diff: From 22c41ac02130f099ba13b0fe91f14ba68e29f9ac Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Tue, 26 May 2026 16:24:32 +0800 Subject: [PATCH v1 1/1] disallow alter enforciability of inherited check constraint If the check constraint qual definitions match but their enforcement statuses conflict (parent enforced, child unenforced), it creates ambiguity around how insert operations should handle the mismatch. Therefore, we should avoid states where the parent check constraint is enforced while the child is not. We actually enforced this within MergeConstraintsIntoExisting and MergeWithExistingConstraint. --- src/backend/commands/tablecmds.c | 14 ++++++++++++++ src/test/regress/expected/constraints.out | 9 +++++++-- src/test/regress/expected/inherit.out | 2 +- src/test/regress/sql/constraints.sql | 2 ++ 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1e0bacf85fc..38250fd0573 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12693,6 +12693,20 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, */ rel = table_open(currcon->conrelid, NoLock); + /* + * If the check constraint qual definitions match but their enforcement + * statuses conflict (parent enforced, child unenforced), it creates + * ambiguity around how insert operations should handle the mismatch. + * Therefore, we should avoid states where the parent check constraint is + * enforced while the child is not. We actually enforced this within + * MergeConstraintsIntoExisting and MergeWithExistingConstraint. + */ + if (currcon->coninhcount > 0 && !recursing) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot alter inherited constraint \"%s\" of relation \"%s\" enforciability", + NameStr(currcon->conname), RelationGetRelationName(rel))); + if (currcon->conenforced != cmdcon->is_enforced) { AlterConstrUpdateConstraintEntry(cmdcon, conrel, contuple); diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index e54fec7fb57..89b33b66cf8 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -446,8 +446,13 @@ 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; +-- error, cannot alter inherited check constraint alter table parted_ch_2 alter constraint cc not enforced; +ERROR: cannot alter inherited constraint "cc" of relation "parted_ch_2" enforciability +alter table only parted_ch_2 alter constraint cc not enforced; -- error +ERROR: cannot alter inherited constraint "cc" of relation "parted_ch_2" enforciability alter table parted_ch_2 alter constraint cc_1 not enforced; +ERROR: cannot alter inherited constraint "cc_1" of relation "parted_ch_2" enforciability alter table parted_ch_2 alter constraint cc_2 not enforced; --check these CHECK constraint status again select * from check_constraint_status; @@ -457,12 +462,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..66d08c4d746 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1458,7 +1458,7 @@ alter table p1 alter constraint inh_check_constraint3 enforced; --error ERROR: check constraint "inh_check_constraint3" of relation "p1_c1" is violated by some row delete from only p1_c1 where f1 = -2; alter table p1_c1 alter constraint inh_check_constraint3 enforced; --error -ERROR: check constraint "inh_check_constraint3" of relation "p1_c3" is violated by some row +ERROR: cannot alter inherited constraint "inh_check_constraint3" of relation "p1_c1" enforciability delete from only p1_c3 where f1 = -3; alter table p1 alter constraint inh_check_constraint3 enforced; --ok alter table p1 alter constraint inh_check_constraint3 not enforced; --ok diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql index dc133b124bb..f60263fa1f5 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -309,7 +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; +-- error, cannot alter inherited check constraint alter table parted_ch_2 alter constraint cc not enforced; +alter table only parted_ch_2 alter constraint cc not enforced; -- error alter table parted_ch_2 alter constraint cc_1 not enforced; alter table parted_ch_2 alter constraint cc_2 not enforced; base-commit: d40aed554227e5f8203798fda25b936a264638ec -- 2.34.1 ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-27 06:20 Chao Li <[email protected]> parent: jian he <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Chao Li @ 2026-05-27 06:20 UTC (permalink / raw) To: jian he <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; L. pgsql-hackers <[email protected]>; Andrew Dunstan <[email protected]> > On May 26, 2026, at 16:32, jian he <[email protected]> wrote: > > On Tue, May 26, 2026 at 3:47 PM Chao Li <[email protected]> wrote: >> >>> >>> I think this is a bug that we need to fix in 19 as well — I mean we should reject the ALTER TABLE. >>> >>> -- >>> Álvaro Herrera >> >> Thanks for your comment. Let me rework the patch. >> > > Hi. > Here are the comments placed in ATExecAlterCheckConstrEnforceability I > came up with: > > + /* > + * If the check constraint qual definitions match but their enforcement > + * statuses conflict (parent enforced, child unenforced), it creates > + * ambiguity around how insert operations should handle the mismatch. > + * Therefore, we should avoid states where the parent check constraint is > + * enforced while the child is not. We actually enforced this within > + * MergeConstraintsIntoExisting and MergeWithExistingConstraint. > + */ > + if (currcon->coninhcount > 0 && !recursing) > + ereport(ERROR, > + errcode(ERRCODE_INVALID_TABLE_DEFINITION), > + errmsg("cannot alter inherited constraint \"%s\" of > relation \"%s\" enforciability", > + NameStr(currcon->conname), > RelationGetRelationName(rel))); > > > > -- > jian > https://www.enterprisedb.com/ > <v1-0001-disallow-alter-enforciability-of-inherited-check-constraint.patch> Hi Jian, Thanks for your help. Your implementation is simple and clever: ``` + if (currcon->coninhcount > 0 && !recursing) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot alter inherited constraint \"%s\" of relation \"%s\" enforciability", + NameStr(currcon->conname), RelationGetRelationName(rel))); ``` Basically, it disallows all enforceability changes on inherited constraints (currcon->coninhcount > 0) at the recursion root (!recursing), or in other words, it disallows the operation on any child table. But I see several problems with this implementation: 1. As you pointed out earlier, when a parent is ENFORCED, changing a child from ENFORCED to NOT ENFORCED should not be allowed. But when a parent is NOT ENFORCED, changing a child from NOT ENFORCED to ENFORCED should be allowed. The existing phase 3 checking also proves that. 2. Suppose a parent table is NOT ENFORCED, and a user changes a child from NOT ENFORCED to ENFORCED, which is allowed. Later, if the user wants to change the child back from ENFORCED to NOT ENFORCED, that should also be allowed. But with your v1 patch, the user would have to do the change through the parent table, which I think hurts the user experience. 3. Suppose a child table is already ENFORCED, and a user issues a command to change it to ENFORCED again, which is actually a no-op. PostgreSQL usually allows this kind of no-op, but with your v1 patch, this no-op would get an error as well, which I think also hurts the user experience. 4. It cannot handle some complicated inheritance hierarchies. For example, the following test passes with your v1: ``` evantest=# CREATE TABLE p1 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); CREATE TABLE evantest=# CREATE TABLE p2 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); CREATE TABLE evantest=# evantest=# CREATE TABLE ch () INHERITS (p1, p2); NOTICE: merging multiple inherited definitions of column "a" CREATE TABLE evantest=# ALTER TABLE p1 ALTER CONSTRAINT c NOT ENFORCED; ALTER TABLE ``` I originally thought this should fail, but it now changes ch.c to NOT ENFORCED, so it breaks the rule because its parent p2 is still ENFORCED: ``` evantest=# SELECT conrelid::regclass, conname, conenforced, coninhcount, conislocal evantest-# FROM pg_constraint WHERE conname = 'c'; conrelid | conname | conenforced | coninhcount | conislocal ----------+---------+-------------+-------------+------------ p1 | c | f | 0 | t p2 | c | t | 0 | t ch | c | f | 2 | f (3 rows) ``` Then I realized that the initial CREATE TABLE case passes: ``` evantest=# CREATE TABLE p1 (a int CONSTRAINT c CHECK (a > 0) NOT ENFORCED); CREATE TABLE evantest=# CREATE TABLE p2 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); CREATE TABLE evantest=# CREATE TABLE ch () INHERITS (p1, p2); NOTICE: merging multiple inherited definitions of column "a" CREATE TABLE evantest=# SELECT conrelid::regclass, conname, conenforced, coninhcount, conislocal evantest-# FROM pg_constraint WHERE conname = ‘c'; conrelid | conname | conenforced | coninhcount | conislocal ----------+---------+-------------+-------------+------------ ch | c | t | 2 | f p1 | c | f | 0 | t p2 | c | t | 0 | t (3 rows) ``` When the two parents have different enforceability, the stricter one is applied to the child. So I think the test above in item 4 should also perform similar merge logic rather than fail. This seems to uncover a new issue in the original feature patch. For the fix, my design is: * Directly reject changing an inherited child CHECK constraint to NOT ENFORCED if an equivalent parent constraint remains ENFORCED. * Changing a child to ENFORCED is allowed. * During recursing, if a child also inherits an equivalent ENFORCED constraint from another parent outside the current ALTER, the child keeps the stricter ENFORCED state. Please see my implementation in the attached v2 patch. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v2-0001-Prevent-inherited-CHECK-constraints-from-being-we.patch (22.5K, 2-v2-0001-Prevent-inherited-CHECK-constraints-from-being-we.patch) download | inline diff: From 3db18b528e579ef6b7bf77a5af79b33e23a15cdb Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Wed, 27 May 2026 13:49:29 +0800 Subject: [PATCH v2] 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 <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/tablecmds.c | 266 ++++++++++++++++++++-- src/test/regress/expected/constraints.out | 12 +- src/test/regress/expected/inherit.out | 54 +++++ src/test/regress/sql/constraints.sql | 5 +- src/test/regress/sql/inherit.sql | 32 +++ 5 files changed, 344 insertions(+), 25 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a1845240a98..5c3a09cc666 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,13 @@ static void AlterConstrDeferrabilityRecurse(List **wqueue, ATAlterConstraint *cm List **otherrelids, LOCKMODE lockmode); static void AlterConstrUpdateConstraintEntry(ATAlterConstraint *cmdcon, Relation conrel, HeapTuple contuple); +static Oid ATGetEquivalentCheckConstraintOid(Relation conrel, Oid conrelid, + const char *conname, + 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 +486,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 +12494,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 +12681,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 +12707,52 @@ 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 +12765,63 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, * try to look for it in the children. */ if (!recursing && !currcon->connoinherit) + { 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. + * 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(list_copy(changing_conids), + currcon->oid); + + foreach_oid(childoid, children) + { + if (childoid == RelationGetRelid(rel)) + continue; + + changing_conids = + list_append_unique_oid(changing_conids, + ATGetEquivalentCheckConstraintOid(conrel, + childoid, + cmdcon->conname, + contuple)); + } + } + } + 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 +12833,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 +12873,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 +12903,153 @@ 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); } +/* + * Look up a CHECK constraint by relation OID and constraint name that is + * equivalent to the given constraint tuple. + */ +static Oid +ATGetEquivalentCheckConstraintOid(Relation conrel, Oid conrelid, + const char *conname, HeapTuple contuple) +{ + SysScanDesc scan; + HeapTuple tup; + ScanKeyData skey[3]; + Oid conid = InvalidOid; + + ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(conrelid)); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(conname)); + + scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true, + NULL, 3, skey); + + while (HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tup); + + if (con->contype == CONSTRAINT_CHECK && + constraints_equivalent(tup, contuple, RelationGetDescr(conrel))) + { + conid = con->oid; + break; + } + } + + systable_endscan(scan); + + if (!OidIsValid(conid)) + ereport(ERROR, + errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("equivalent CHECK constraint \"%s\" of relation \"%s\" does not exist", + conname, get_rel_name(conrelid))); + + return conid; +} + +/* + * 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..86e5b892a98 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1479,6 +1479,60 @@ 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 +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..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..3803fb5c769 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -535,6 +535,38 @@ 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 +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.50.1 (Apple Git-155) ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-28 02:31 jian he <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: jian he @ 2026-05-28 02:31 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; L. pgsql-hackers <[email protected]>; Andrew Dunstan <[email protected]> On Wed, May 27, 2026 at 2:20 PM Chao Li <[email protected]> wrote: > > 4. It cannot handle some complicated inheritance hierarchies. For example, the following test passes with your v1: > ``` > evantest=# CREATE TABLE p1 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); > CREATE TABLE > evantest=# CREATE TABLE p2 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); > CREATE TABLE > evantest=# > evantest=# CREATE TABLE ch () INHERITS (p1, p2); > NOTICE: merging multiple inherited definitions of column "a" > CREATE TABLE > evantest=# ALTER TABLE p1 ALTER CONSTRAINT c NOT ENFORCED; > ALTER TABLE > ``` > > I originally thought this should fail, but it now changes ch.c to NOT ENFORCED, so it breaks the rule because its parent p2 is still ENFORCED: > ``` > evantest=# SELECT conrelid::regclass, conname, conenforced, coninhcount, conislocal > evantest-# FROM pg_constraint WHERE conname = 'c'; > conrelid | conname | conenforced | coninhcount | conislocal > ----------+---------+-------------+-------------+------------ > p1 | c | f | 0 | t > p2 | c | t | 0 | t > ch | c | f | 2 | f > (3 rows) > ``` > > Then I realized that the initial CREATE TABLE case passes: > ``` > evantest=# CREATE TABLE p1 (a int CONSTRAINT c CHECK (a > 0) NOT ENFORCED); > CREATE TABLE > evantest=# CREATE TABLE p2 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); > CREATE TABLE > evantest=# CREATE TABLE ch () INHERITS (p1, p2); > NOTICE: merging multiple inherited definitions of column "a" > CREATE TABLE > evantest=# SELECT conrelid::regclass, conname, conenforced, coninhcount, conislocal > evantest-# FROM pg_constraint WHERE conname = ‘c'; > conrelid | conname | conenforced | coninhcount | conislocal > ----------+---------+-------------+-------------+------------ > ch | c | t | 2 | f > p1 | c | f | 0 | t > p2 | c | t | 0 | t > (3 rows) > ``` > > When the two parents have different enforceability, the stricter one is applied to the child. So I think the test above in item 4 should also perform similar merge logic rather than fail. This seems to uncover a new issue in the original feature patch. > > For the fix, my design is: > > * Directly reject changing an inherited child CHECK constraint to NOT ENFORCED if an equivalent parent constraint remains ENFORCED. > * Changing a child to ENFORCED is allowed. > * During recursing, if a child also inherits an equivalent ENFORCED constraint from another parent outside the current ALTER, the child keeps the stricter ENFORCED state. > > Please see my implementation in the attached v2 patch. CREATE TABLE p1 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); CREATE TABLE p2 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); CREATE TABLE ch () INHERITS (p1, p2); ALTER TABLE p1 ALTER CONSTRAINT c NOT ENFORCED; The v2 patch marks check constraint c on table ch as ENFORCED, which seems to contradict the documentation's wording: https://www.postgresql.org/docs/devel/ddl-inherit.html <<>> ALTER TABLE will propagate any changes in column data definitions and check constraints down the inheritance hierarchy. Again, dropping columns that are depended on by other tables is only possible when using the CASCADE option. ALTER TABLE follows the same rules for duplicate column merging and rejection that apply during CREATE TABLE <<>> The wording (https://www.postgresql.org/docs/devel/ddl-inherit.html) below also discourages directly altering check constraints on child tables. <<>> A parent table cannot be dropped while any of its children remain. Neither can columns or check constraints of child tables be dropped or altered if they are inherited from any parent tables. If you wish to remove a table and all of its descendants, one easy way is to drop the parent table with the CASCADE option (see Section 5.17). <<>> -- jian https://www.enterprisedb.com/ ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-28 08:55 Álvaro Herrera <[email protected]> parent: jian he <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Álvaro Herrera @ 2026-05-28 08:55 UTC (permalink / raw) To: jian he <[email protected]>; +Cc: Chao Li <[email protected]>; L. pgsql-hackers <[email protected]>; Andrew Dunstan <[email protected]> On 2026-May-28, jian he wrote: > CREATE TABLE p1 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); > CREATE TABLE p2 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); > CREATE TABLE ch () INHERITS (p1, p2); > ALTER TABLE p1 ALTER CONSTRAINT c NOT ENFORCED; > > The v2 patch marks check constraint c on table ch as ENFORCED, which > seems to contradict the documentation's wording: I think what v2 is doing in this case is correct. The child's constraint must preserve whatever the strictest of the inherited constraints status is. In this case, because the constraint on p2 remains ENFORCED, then it must remain ENFORCED in the child as well. Changing it to NOT ENFORCED after the ALTER TABLE would be wrong. > https://www.postgresql.org/docs/devel/ddl-inherit.html > <<>> > ALTER TABLE will propagate any changes in column data definitions and check > constraints down the inheritance hierarchy. Again, dropping columns that are > depended on by other tables is only possible when using the CASCADE option. > ALTER TABLE follows the same rules for duplicate column merging and rejection > that apply during CREATE TABLE > <<>> I think this text is a bit vague in that it isn't really considering multiple inheritance -- it appears to be written with the perspective of each child table having a single parent. It may be a bit obsolete also; it talks about "check constraints" but we also allow not-null constraints to have these kind of properties as well (which we didn't when this was written). > The wording (https://www.postgresql.org/docs/devel/ddl-inherit.html) > below also discourages directly altering check constraints on child tables. > <<>> > A parent table cannot be dropped while any of its children remain. Neither can > columns or check constraints of child tables be dropped or altered if they are > inherited from any parent tables. If you wish to remove a table and all of its > descendants, one easy way is to drop the parent table with the CASCADE option > (see Section 5.17). > <<>> Hmm, I think this text is borderline obsolete, in the sense that we know allow changing some properties of some constraints in child tables. I'm not really sure to what extent it is useful to make it more explicit about that. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "You're _really_ hosed if the person doing the hiring doesn't understand relational systems: you end up with a whole raft of programmers, none of whom has had a Date with the clue stick." (Andrew Sullivan) https://postgr.es/m/[email protected] ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix bug of CHECK constraint enforceability recursion @ 2026-05-28 22:56 Chao Li <[email protected]> parent: Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 12+ messages in thread From: Chao Li @ 2026-05-28 22:56 UTC (permalink / raw) To: Álvaro Herrera <[email protected]>; +Cc: jian he <[email protected]>; L. pgsql-hackers <[email protected]>; Andrew Dunstan <[email protected]> > On May 28, 2026, at 16:55, Álvaro Herrera <[email protected]> wrote: > > On 2026-May-28, jian he wrote: > >> CREATE TABLE p1 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); >> CREATE TABLE p2 (a int CONSTRAINT c CHECK (a > 0) ENFORCED); >> CREATE TABLE ch () INHERITS (p1, p2); >> ALTER TABLE p1 ALTER CONSTRAINT c NOT ENFORCED; >> >> The v2 patch marks check constraint c on table ch as ENFORCED, which >> seems to contradict the documentation's wording: > > I think what v2 is doing in this case is correct. The child's > constraint must preserve whatever the strictest of the inherited > constraints status is. In this case, because the constraint on p2 > remains ENFORCED, then it must remain ENFORCED in the child as well. > Changing it to NOT ENFORCED after the ALTER TABLE would be wrong. > >> https://www.postgresql.org/docs/devel/ddl-inherit.html >> <<>> >> ALTER TABLE will propagate any changes in column data definitions and check >> constraints down the inheritance hierarchy. Again, dropping columns that are >> depended on by other tables is only possible when using the CASCADE option. >> ALTER TABLE follows the same rules for duplicate column merging and rejection >> that apply during CREATE TABLE >> <<>> > > I think this text is a bit vague in that it isn't really considering > multiple inheritance -- it appears to be written with the perspective of > each child table having a single parent. > > It may be a bit obsolete also; it talks about "check constraints" but we > also allow not-null constraints to have these kind of properties as > well (which we didn't when this was written). > >> The wording (https://www.postgresql.org/docs/devel/ddl-inherit.html) >> below also discourages directly altering check constraints on child tables. >> <<>> >> A parent table cannot be dropped while any of its children remain. Neither can >> columns or check constraints of child tables be dropped or altered if they are >> inherited from any parent tables. If you wish to remove a table and all of its >> descendants, one easy way is to drop the parent table with the CASCADE option >> (see Section 5.17). >> <<>> > > Hmm, I think this text is borderline obsolete, in the sense that we know > allow changing some properties of some constraints in child tables. > I'm not really sure to what extent it is useful to make it more explicit > about that. > > -- > Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ > "You're _really_ hosed if the person doing the hiring doesn't understand > relational systems: you end up with a whole raft of programmers, none of > whom has had a Date with the clue stick." (Andrew Sullivan) > https://postgr.es/m/[email protected] Thank Jian for pointing out the doc. Thank Álvaro for guiding the direction. I just tried to update the doc in v3-0002, while 0001 is unchanged. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v3-0001-Prevent-inherited-CHECK-constraints-from-being-we.patch (22.5K, 2-v3-0001-Prevent-inherited-CHECK-constraints-from-being-we.patch) download | inline diff: From 7a5f027ed4434102d0df73ecae55b05aaaf7e35c Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Wed, 27 May 2026 13:49:29 +0800 Subject: [PATCH v3 1/2] 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 <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/tablecmds.c | 266 ++++++++++++++++++++-- src/test/regress/expected/constraints.out | 12 +- src/test/regress/expected/inherit.out | 54 +++++ src/test/regress/sql/constraints.sql | 5 +- src/test/regress/sql/inherit.sql | 32 +++ 5 files changed, 344 insertions(+), 25 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a1845240a98..5c3a09cc666 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,13 @@ static void AlterConstrDeferrabilityRecurse(List **wqueue, ATAlterConstraint *cm List **otherrelids, LOCKMODE lockmode); static void AlterConstrUpdateConstraintEntry(ATAlterConstraint *cmdcon, Relation conrel, HeapTuple contuple); +static Oid ATGetEquivalentCheckConstraintOid(Relation conrel, Oid conrelid, + const char *conname, + 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 +486,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 +12494,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 +12681,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 +12707,52 @@ 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 +12765,63 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, * try to look for it in the children. */ if (!recursing && !currcon->connoinherit) + { 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. + * 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(list_copy(changing_conids), + currcon->oid); + + foreach_oid(childoid, children) + { + if (childoid == RelationGetRelid(rel)) + continue; + + changing_conids = + list_append_unique_oid(changing_conids, + ATGetEquivalentCheckConstraintOid(conrel, + childoid, + cmdcon->conname, + contuple)); + } + } + } + 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 +12833,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 +12873,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 +12903,153 @@ 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); } +/* + * Look up a CHECK constraint by relation OID and constraint name that is + * equivalent to the given constraint tuple. + */ +static Oid +ATGetEquivalentCheckConstraintOid(Relation conrel, Oid conrelid, + const char *conname, HeapTuple contuple) +{ + SysScanDesc scan; + HeapTuple tup; + ScanKeyData skey[3]; + Oid conid = InvalidOid; + + ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(conrelid)); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(conname)); + + scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true, + NULL, 3, skey); + + while (HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tup); + + if (con->contype == CONSTRAINT_CHECK && + constraints_equivalent(tup, contuple, RelationGetDescr(conrel))) + { + conid = con->oid; + break; + } + } + + systable_endscan(scan); + + if (!OidIsValid(conid)) + ereport(ERROR, + errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("equivalent CHECK constraint \"%s\" of relation \"%s\" does not exist", + conname, get_rel_name(conrelid))); + + return conid; +} + +/* + * 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..86e5b892a98 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1479,6 +1479,60 @@ 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 +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..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..3803fb5c769 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -535,6 +535,38 @@ 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 +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.50.1 (Apple Git-155) [application/octet-stream] v3-0002-doc-Clarify-inherited-constraint-behavior.patch (3.7K, 3-v3-0002-doc-Clarify-inherited-constraint-behavior.patch) download | inline diff: From b11f63718c50a672ee9435c66a3adef6c4799c06 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Fri, 29 May 2026 06:47:07 +0800 Subject: [PATCH v3 2/2] doc: Clarify inherited constraint behavior Update the table inheritance documentation to mention not-null constraints alongside check constraints where inherited constraints are discussed. Also clarify that some properties of inherited constraints can now be altered directly on child tables, while the resulting constraint must remain compatible with its inherited parent constraints. For multiple inheritance, say explicitly that when a column or constraint is inherited from more than one parent, the stricter definition applies. Author: Chao Li <[email protected]> --- doc/src/sgml/ddl.sgml | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 747f929aee3..afcc642bcb7 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -4104,8 +4104,9 @@ VALUES ('Albany', NULL, NULL, 'NY'); To do this the new child table must already include columns with the same names and types as the columns of the parent. It must also include check constraints with the same names and check expressions as those of the - parent. Similarly an inheritance link can be removed from a child using the - <literal>NO INHERIT</literal> variant of <command>ALTER TABLE</command>. + parent, as well as matching not-null constraints. Similarly an inheritance + link can be removed from a child using the <literal>NO INHERIT</literal> + variant of <command>ALTER TABLE</command>. Dynamically adding and removing inheritance links like this can be useful when the inheritance relationship is being used for table partitioning (see <xref linkend="ddl-partitioning"/>). @@ -4124,21 +4125,25 @@ VALUES ('Albany', NULL, NULL, 'NY'); <para> A parent table cannot be dropped while any of its children remain. Neither - can columns or check constraints of child tables be dropped or altered - if they are inherited - from any parent tables. If you wish to remove a table and all of its - descendants, one easy way is to drop the parent table with the - <literal>CASCADE</literal> option (see <xref linkend="ddl-depend"/>). + can inherited columns or inherited check and not-null constraints of child + tables be dropped directly. Some properties of inherited constraints can + be altered, but each resulting constraint must remain compatible with all + parent constraints from which it is inherited. If you wish to remove a + table and all of its descendants, one easy way is to drop the parent table + with the <literal>CASCADE</literal> option (see <xref linkend="ddl-depend"/>). </para> <para> <command>ALTER TABLE</command> will - propagate any changes in column data definitions and check - constraints down the inheritance hierarchy. Again, dropping + propagate changes in column definitions and in inheritable constraints + (check and not-null constraints) down the inheritance hierarchy. With + multiple inheritance, if a column or constraint is inherited from more + than one parent, the stricter definition applies. Again, dropping columns that are depended on by other tables is only possible when using the <literal>CASCADE</literal> option. <command>ALTER - TABLE</command> follows the same rules for duplicate column merging - and rejection that apply during <command>CREATE TABLE</command>. + TABLE</command> follows the same rules for merging or rejecting duplicate + inherited column and constraint definitions that apply during + <command>CREATE TABLE</command>. </para> <para> -- 2.50.1 (Apple Git-155) ^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2026-05-28 22:56 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-05-26 03:51 Fix bug of CHECK constraint enforceability recursion Chao Li <[email protected]> 2026-05-26 05:48 ` Chao Li <[email protected]> 2026-05-26 06:05 ` jian he <[email protected]> 2026-05-26 06:27 ` Chao Li <[email protected]> 2026-05-26 07:32 ` Álvaro Herrera <[email protected]> 2026-05-26 07:46 ` Chao Li <[email protected]> 2026-05-26 08:32 ` jian he <[email protected]> 2026-05-27 06:20 ` Chao Li <[email protected]> 2026-05-28 02:31 ` jian he <[email protected]> 2026-05-28 08:55 ` Álvaro Herrera <[email protected]> 2026-05-28 22:56 ` Chao Li <[email protected]> 2026-05-26 07:44 ` Chao Li <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox