Re: Fix bug of CHECK constraint enforceability recursion

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Fix bug of CHECK constraint enforceability recursion
Date: 2026-06-08 11:51:33
Message-ID: B41C7375-BA73-4471-9CAA-FA6D8046BDB6@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Jun 8, 2026, at 18:28, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Sat, Jun 6, 2026 at 11:06 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>> PFA v8: 0001 and 0003 unchanged. 0002 addressed Zsolt’s comment.
>>
>

Thanks for the review.

> drop table root_t cascade;
> create table root_t (a int constraint c check (a > 0) enforced);
> create table p2 (a int constraint c check (a > 0) enforced);
> create table d () inherits (root_t, p2);
> create table e () inherits (d);
> CREATE OR REPLACE PROCEDURE create_table(i int) AS $$
> DECLARE
> r int;
> query text;
> BEGIN
> FOR i IN 20..$1 LOOP
> query := 'create table f' || i || '() inherits (e)';
> raise notice 'query: %', query;
> EXECUTE query;
> END LOOP;
> END; $$ LANGUAGE plpgsql;
>
> call create_table(1000);
> alter table root_t alter constraint c not enforced;
>
> ATCheckCheckConstrHasEnforcedParent invoked 996070 times
> ATCheckCheckConstrHasEnforcedParent will call itself, so I think we
> need check_stack_depth() on it.

Good point. Added it in v9.

>
> I also made a minor refactoring to reduce unnecessary work in
> ATCheckCheckConstrHasEnforcedParent().
> A single call to table_open(parentoid, ...) is enough.
>

Agreed. I fine-tuned this code a little bit further. Now in v9, it only opens the parent table when needed and only open at most once.

> Keep errmsg() messages on a single line to improve grepability.
>

I’m not sure this needs to be changed. The message is quite long, and there are existing precedents for splitting long errmsg() strings across multiple lines. For example:
```
if (getExtensionOfObject(NamespaceRelationId, nspOid) == extensionOid)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("cannot move extension \"%s\" into schema \"%s\" "
"because the extension contains the schema",
extensionName, newschema)));
```

So, this does not seem to be a strict rule. I also don’t think this hurts grepability much in practice, since searching for the distinctive part of the message still finds it.

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

Attachment Content-Type Size
v9-0001-Prevent-inherited-CHECK-constraints-from-being-we.patch application/octet-stream 24.9 KB
v9-0002-doc-Clarify-inherited-constraint-behavior.patch application/octet-stream 4.4 KB
v9-0003-doc-Clarify-ALTER-CONSTRAINT-enforceability-behav.patch application/octet-stream 2.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message solai v 2026-06-08 12:41:12 Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
Previous Message Atsushi Torikoshi 2026-06-08 11:48:55 Re: RFC: Logging plan of the running query