Fix bug of CHECK constraint enforceability recursion

Started by Chao Li8 days ago15 messageshackers

li.evan.chao@gmail.com

8 days ago

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/

Chao Li

li.evan.chao@gmail.com

7 days ago

In reply to: Chao Li (#1)

Re: Fix bug of CHECK constraint enforceability recursion

On May 26, 2026, at 11:51, Chao Li <li.evan.chao@gmail.com> 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]/messages/by-id/711B1ED3-1781-4B6C-A573-B58AF20770E5@gmail.com into this thread as they are for the same feature.

[1]: /messages/by-id/711B1ED3-1781-4B6C-A573-B58AF20770E5@gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

jian he

jian.universality@gmail.com

7 days ago

In reply to: Chao Li (#1)

Re: Fix bug of CHECK constraint enforceability recursion

On Tue, May 26, 2026 at 11:51 AM Chao Li <li.evan.chao@gmail.com> 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/

Chao Li

li.evan.chao@gmail.com

7 days ago

In reply to: jian he (#3)

Re: Fix bug of CHECK constraint enforceability recursion

On May 26, 2026, at 14:05, jian he <jian.universality@gmail.com> wrote:

On Tue, May 26, 2026 at 11:51 AM Chao Li <li.evan.chao@gmail.com> 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/

Alvaro Herrera

alvherre@2ndquadrant.com

7 days ago

In reply to: Chao Li (#4)

Re: Fix bug of CHECK constraint enforceability recursion

On 2026-05-26, Chao Li wrote:

On May 26, 2026, at 14:05, jian he <jian.universality@gmail.com> 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

Chao Li

li.evan.chao@gmail.com

7 days ago

In reply to: Chao Li (#4)

Re: Fix bug of CHECK constraint enforceability recursion

On May 26, 2026, at 14:27, Chao Li <li.evan.chao@gmail.com> wrote:

On May 26, 2026, at 14:05, jian he <jian.universality@gmail.com> wrote:

On Tue, May 26, 2026 at 11:51 AM Chao Li <li.evan.chao@gmail.com> 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/

Chao Li

li.evan.chao@gmail.com

7 days ago

In reply to: Alvaro Herrera (#5)

Re: Fix bug of CHECK constraint enforceability recursion

On May 26, 2026, at 15:32, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-05-26, Chao Li wrote:

On May 26, 2026, at 14:05, jian he <jian.universality@gmail.com> 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/

jian he

jian.universality@gmail.com

7 days ago

In reply to: Chao Li (#7)

Re: Fix bug of CHECK constraint enforceability recursion

On Tue, May 26, 2026 at 3:47 PM Chao Li <li.evan.chao@gmail.com> 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/

Chao Li

li.evan.chao@gmail.com

6 days ago

In reply to: jian he (#8)

Re: Fix bug of CHECK constraint enforceability recursion

On May 26, 2026, at 16:32, jian he <jian.universality@gmail.com> wrote:

On Tue, May 26, 2026 at 3:47 PM Chao Li <li.evan.chao@gmail.com> 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/

#10

jian he

jian.universality@gmail.com

6 days ago

In reply to: Chao Li (#9)

Re: Fix bug of CHECK constraint enforceability recursion

On Wed, May 27, 2026 at 2:20 PM Chao Li <li.evan.chao@gmail.com> 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/

#11

Alvaro Herrera

alvherre@2ndquadrant.com

5 days ago

In reply to: jian he (#10)

Re: Fix bug of CHECK constraint enforceability recursion

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)
/messages/by-id/20050809113420.GD2768@phlogiston.dyndns.org

#12

Chao Li

li.evan.chao@gmail.com

5 days ago

In reply to: Alvaro Herrera (#11)

Re: Fix bug of CHECK constraint enforceability recursion

On May 28, 2026, at 16:55, Álvaro Herrera <alvherre@kurilemu.de> 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)
/messages/by-id/20050809113420.GD2768@phlogiston.dyndns.org

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/

#13

jian he

jian.universality@gmail.com

1 day ago

In reply to: Chao Li (#12)

Re: Fix bug of CHECK constraint enforceability recursion

Hi.

I did some refactoring based on your v3, I didn't change your comments.

I don't think ATGetEquivalentCheckConstraintOid is necessary.
ATCheckCheckConstrHasEnforcedParent,
AlterCheckConstrEnforceabilityRecurse is enough for this context.

+ changing_conids = lappend_oid(list_copy(changing_conids),
+  currcon->oid);
Refactor changing_conids to List ** to allow direct modification,
aligning with existing code conventions.

CREATE TABLE ... PARTITION OF automatically copies the parent's status, and
ALTER TABLE ... ATTACH PARTITION already rejects cases where the parent is
enforced but the child is not. If we also reject directly altering a partition's
constraint enforcement status, then we need no longer worry about cases where
parent being enforced while a child is not. Therefore, invoking
ATCheckCheckConstrHasEnforcedParent just once for table partitioning is safe.

--
jian
https://www.enterprisedb.com/

#14

Chao Li

li.evan.chao@gmail.com

about 11 hours ago

In reply to: jian he (#13)

Re: Fix bug of CHECK constraint enforceability recursion

On Jun 1, 2026, at 23:51, jian he <jian.universality@gmail.com> wrote:

Hi.

I did some refactoring based on your v3, I didn't change your comments.

I don't think ATGetEquivalentCheckConstraintOid is necessary.
ATCheckCheckConstrHasEnforcedParent,
AlterCheckConstrEnforceabilityRecurse is enough for this context.

I agree that ATGetEquivalentCheckConstraintOid is not needed. I was too worried that looking up descendant constraints by name alone was not good enough. But now I realize that users have no supported way to define a conflicting constraint on a child with the same name as a parent’s inheritable CHECK constraint. In v5, I removed ATGetEquivalentCheckConstraintOid and switched to get_relation_constraint_oid.

+ changing_conids = lappend_oid(list_copy(changing_conids),
+  currcon->oid);
Refactor changing_conids to List ** to allow direct modification,
aligning with existing code conventions.

The v3 code was a little confusing. In fact, changing_conids only needs to be built at the top level, so list_copy() is not needed, and List ** is not needed either. I refactored this part in v5.

CREATE TABLE ... PARTITION OF automatically copies the parent's status, and
ALTER TABLE ... ATTACH PARTITION already rejects cases where the parent is
enforced but the child is not. If we also reject directly altering a partition's
constraint enforcement status, then we need no longer worry about cases where
parent being enforced while a child is not. Therefore, invoking
ATCheckCheckConstrHasEnforcedParent just once for table partitioning is safe.

--
jian
https://www.enterprisedb.com/
<v4-0001-Prevent-inherited-CHECK-constraints-from-being-weakened.patch>

v4 is similar to my original implementation when I worked on v2, and it does not handle some complicated inheritance cases correctly. The following 2 tests are based on v4.

1
```
evantest=# create table root(a int constraint ck check (a > 0) enforced);
CREATE TABLE
evantest=# create table ext(a int constraint ck check (a > 0) enforced);
CREATE TABLE
evantest=# create table mixed() inherits (root, ext);
NOTICE: merging multiple inherited definitions of column "a"
CREATE TABLE
evantest=# create table ordinary() inherits (root);
CREATE TABLE
evantest=# alter table root alter constraint ck not enforced;
ALTER TABLE
evantest=# select conrelid::regclass, conname, conenforced, coninhcount, conislocal from pg_constraint where conname='ck';
conrelid | conname | conenforced | coninhcount | conislocal
----------+---------+-------------+-------------+------------
root | ck | f | 0 | t
ext | ck | t | 0 | t
mixed | ck | t | 2 | f
ordinary | ck | t | 1 | f
(4 rows)
```

In this test, ordinary only inherits from root, so when root is changed to NOT ENFORCED, ordinary should be changed as well. However, ordinary incorrectly remains ENFORCED.

2
```
evantest=# create table root(a int constraint ck check (a > 0) enforced);
CREATE TABLE
evantest=# create table child() inherits (root);
CREATE TABLE
evantest=# create table intermediate() inherits (root);
CREATE TABLE
evantest=# alter table child inherit intermediate;
ALTER TABLE
evantest=# alter table root alter constraint ck not enforced;
ALTER TABLE
evantest=# select conrelid::regclass, conname, conenforced, coninhcount, conislocal from pg_constraint where conname='ck';
conrelid | conname | conenforced | coninhcount | conislocal
--------------+---------+-------------+-------------+------------
root | ck | f | 0 | t
child | ck | t | 2 | f
intermediate | ck | t | 1 | f
(3 rows)
```

This is a more complicated case. The tricky part is that child is altered to inherit from intermediate, but child was created before intermediate, so child has a smaller OID than intermediate, which affects the recursion order.

Ultimately, both child and intermediate inherit from root, so after root is changed to NOT ENFORCED, they should be changed to NOT ENFORCED as well. But in this test, they incorrectly remain ENFORCED.

Both tests pass with v3 and v5:

PFA v5:

0001:
* Removed ATGetEquivalentCheckConstraintOid
* Refactored how changing_conids is built
* Added two test cases that Jian added to v4
* Added the complicated inheritance test cases shown above

0002: Same as v3-0002; fixes the documentation issue Alvaro pointed out earlier

0003: The documentation change from [1]/messages/by-id/711B1ED3-1781-4B6C-A573-B58AF20770E5@gmail.com, which clarifies the ALTER TABLE doc about constraint enforceability

BTW, Jian, next time, would you mind attaching a diff on top of the previous patch version? That would make your changes easier to identify.

[1]: /messages/by-id/711B1ED3-1781-4B6C-A573-B58AF20770E5@gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

#15

Chao Li

li.evan.chao@gmail.com

about 11 hours ago

In reply to: Chao Li (#14)

Re: Fix bug of CHECK constraint enforceability recursion

On Jun 2, 2026, at 14:24, Chao Li <li.evan.chao@gmail.com> wrote:

On Jun 1, 2026, at 23:51, jian he <jian.universality@gmail.com> wrote:

Hi.

I did some refactoring based on your v3, I didn't change your comments.

I don't think ATGetEquivalentCheckConstraintOid is necessary.
ATCheckCheckConstrHasEnforcedParent,
AlterCheckConstrEnforceabilityRecurse is enough for this context.

I agree that ATGetEquivalentCheckConstraintOid is not needed. I was too worried that looking up descendant constraints by name alone was not good enough. But now I realize that users have no supported way to define a conflicting constraint on a child with the same name as a parent’s inheritable CHECK constraint. In v5, I removed ATGetEquivalentCheckConstraintOid and switched to get_relation_constraint_oid.
+ changing_conids = lappend_oid(list_copy(changing_conids),
+  currcon->oid);
Refactor changing_conids to List ** to allow direct modification,
aligning with existing code conventions.
The v3 code was a little confusing. In fact, changing_conids only needs to be built at the top level, so list_copy() is not needed, and List ** is not needed either. I refactored this part in v5.

CREATE TABLE ... PARTITION OF automatically copies the parent's status, and
ALTER TABLE ... ATTACH PARTITION already rejects cases where the parent is
enforced but the child is not. If we also reject directly altering a partition's
constraint enforcement status, then we need no longer worry about cases where
parent being enforced while a child is not. Therefore, invoking
ATCheckCheckConstrHasEnforcedParent just once for table partitioning is safe.

--
jian
https://www.enterprisedb.com/
<v4-0001-Prevent-inherited-CHECK-constraints-from-being-weakened.patch>

v4 is similar to my original implementation when I worked on v2, and it does not handle some complicated inheritance cases correctly. The following 2 tests are based on v4.

1
```
evantest=# create table root(a int constraint ck check (a > 0) enforced);
CREATE TABLE
evantest=# create table ext(a int constraint ck check (a > 0) enforced);
CREATE TABLE
evantest=# create table mixed() inherits (root, ext);
NOTICE: merging multiple inherited definitions of column "a"
CREATE TABLE
evantest=# create table ordinary() inherits (root);
CREATE TABLE
evantest=# alter table root alter constraint ck not enforced;
ALTER TABLE
evantest=# select conrelid::regclass, conname, conenforced, coninhcount, conislocal from pg_constraint where conname='ck';
conrelid | conname | conenforced | coninhcount | conislocal
----------+---------+-------------+-------------+------------
root | ck | f | 0 | t
ext | ck | t | 0 | t
mixed | ck | t | 2 | f
ordinary | ck | t | 1 | f
(4 rows)
```

In this test, ordinary only inherits from root, so when root is changed to NOT ENFORCED, ordinary should be changed as well. However, ordinary incorrectly remains ENFORCED.

2
```
evantest=# create table root(a int constraint ck check (a > 0) enforced);
CREATE TABLE
evantest=# create table child() inherits (root);
CREATE TABLE
evantest=# create table intermediate() inherits (root);
CREATE TABLE
evantest=# alter table child inherit intermediate;
ALTER TABLE
evantest=# alter table root alter constraint ck not enforced;
ALTER TABLE
evantest=# select conrelid::regclass, conname, conenforced, coninhcount, conislocal from pg_constraint where conname='ck';
conrelid | conname | conenforced | coninhcount | conislocal
--------------+---------+-------------+-------------+------------
root | ck | f | 0 | t
child | ck | t | 2 | f
intermediate | ck | t | 1 | f
(3 rows)
```

This is a more complicated case. The tricky part is that child is altered to inherit from intermediate, but child was created before intermediate, so child has a smaller OID than intermediate, which affects the recursion order.

Ultimately, both child and intermediate inherit from root, so after root is changed to NOT ENFORCED, they should be changed to NOT ENFORCED as well. But in this test, they incorrectly remain ENFORCED.

Both tests pass with v3 and v5:

PFA v5:

0001:
* Removed ATGetEquivalentCheckConstraintOid
* Refactored how changing_conids is built
* Added two test cases that Jian added to v4
* Added the complicated inheritance test cases shown above

0002: Same as v3-0002; fixes the documentation issue Alvaro pointed out earlier

0003: The documentation change from [1], which clarifies the ALTER TABLE doc about constraint enforceability

BTW, Jian, next time, would you mind attaching a diff on top of the previous patch version? That would make your changes easier to identify.

[1] /messages/by-id/711B1ED3-1781-4B6C-A573-B58AF20770E5@gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

<v5-0001-Prevent-inherited-CHECK-constraints-from-being-we.patch><v5-0002-doc-Clarify-inherited-constraint-behavior.patch><v5-0003-doc-Clarify-ALTER-CONSTRAINT-enforceability-behav.patch>

Oops! I just found that I forgot to commit a tiny comment tuning in 0001. So posting v6.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Fix bug of CHECK constraint enforceability recursion

Attachments:

Attachments:

Attachments:

Attachments:

Attachments:

Attachments:

Attachments:

Attachments: