> On Jun 1, 2026, at 23:51, jian he <[email protected]> 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] https://www.postgresql.org/message-id/711B1ED3-1781-4B6C-A573-B58AF20770E5%40gmail.com Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
v5-0001-Prevent-inherited-CHECK-constraints-from-being-we.patch
Description: Binary data
v5-0002-doc-Clarify-inherited-constraint-behavior.patch
Description: Binary data
v5-0003-doc-Clarify-ALTER-CONSTRAINT-enforceability-behav.patch
Description: Binary data
