Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\))
Subject: Re: Fix bug of CHECK constraint enforceability recursion
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CAN4CZFNjnR-7hPBO=v=9h4CdDogUueYoPqpPvR9k3X5Z9-=+TA@mail.gmail.com>
Date: Fri, 5 Jun 2026 08:35:56 +0800
Cc: pgsql-hackers@lists.postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <684DCC2F-5982-4A5D-93F9-BD640B58B4BB@gmail.com>
References: <E74C57FA-1DD0-4C8E-8FB1-538034752592@gmail.com>
 <CACJufxGRkCSqqRwd+3MMVFbosKXHv=Jc+KyGK+NM3YAHN7eu+g@mail.gmail.com>
 <DCE683F9-CEBF-4EDE-81D1-5E59CCD5CF82@gmail.com>
 <cb4a41fd-67e8-47ed-bcff-d0549dce3f02@app.fastmail.com>
 <33E9C4C2-B6A8-4FCC-BEEA-461EA5FB98C8@gmail.com>
 <CACJufxGxY=1mxcNz8-bjp6R+ZoCRE3O7DYt2ZqObv-Le+aMOqw@mail.gmail.com>
 <7F0EA98A-6DBC-436A-8FF4-4A511A05ABE6@gmail.com>
 <CACJufxFwJH5K1jgL1ryckYX8gxp1sfqz_2ypey1sYtWZ5u45oQ@mail.gmail.com>
 <ahgAsvfSt3kbw88l@alvherre.pgsql>
 <7B7172F4-DB02-4259-997B-6AEF5ADF7FCE@gmail.com>
 <CACJufxHbVuY683j3DHEU4LSswo8DTzLJbTvhv2Uj6gqN8pJs7Q@mail.gmail.com>
 <14E223D8-8425-446A-A36C-6B62BC334656@gmail.com>
 <9A3D388C-1DF4-4C43-9AB6-83529A8F48C8@gmail.com>
 <CACJufxGgSKFGWja=JPv3-zq3whON5cESLp7Z5=OsYvZOaRGnGw@mail.gmail.com>
 <E6956B70-35FF-474D-9F53-FC6E1BD8D9C0@gmail.com>
 <CAN4CZFOOhDAWdnY1OXB1wfG_=C4g_dw0gC2HCApz_3F7EFpqJA@mail.gmail.com>
 <92CAD935-5ECE-46D8-B7D7-D8E3C991CC10@gmail.com>
 <CAN4CZFNjnR-7hPBO=v=9h4CdDogUueYoPqpPvR9k3X5Z9-=+TA@mail.gmail.com>
To: Zsolt Parragi <zsolt.parragi@percona.com>
Archived-At: 
 <https://www.postgresql.org/message-id/684DCC2F-5982-4A5D-93F9-BD640B58B4BB%40gmail.com>
Precedence: bulk


> On Jun 5, 2026, at 03:08, Zsolt Parragi <zsolt.parragi@percona.com> =
wrote:
>=20
> The new version looks correct, I don't see any logic problem with it,
> however, I do have a performance question:
>=20
> + /*
> + * A parent listed in changing_conids is being changed by the
> + * same ALTER, but it may not have been updated yet.  For
> + * regular inheritance, recurse upward to check whether an
> + * equivalent enforced parent outside the ALTER will make it
> + * remain enforced.  Partitions cannot have multiple parents,
> + * so they do not need this check.
> + */
> + if (!rel->rd_rel->relispartition &&
> + list_member_oid(changing_conids, parentcon->oid))
>=20
> Shouldn't the parent lookup use some form of caching? Otherwise we'll
> end up reevaluating the same parents multiple times. I'm not sure if
> it is needed or not, how much of a performance impact this can have in
> a real-world server.

Actually, after sending v7, I spent several hours trying to cache update =
history, which could avoid some recurse-up lookups. But I was =
uncomfortable with the resulting code and gave up on that approach. =
There were several reasons:

* Re-evaluation only happens in an edge case where all of the following =
are true:
   - the constraint is altered from ENFORCED to NOT ENFORCED
   - the target is a regular inherited table, partitioned tables do not =
go through this logic
   - the inheritance graph is complex, as in your test case, only tables =
like e and f hit the re-evaluation case
  =20
* It is common for a partitioned table to have thousands of partitions, =
but I rarely hear of a regular inherited table having thousands of =
descendants.

* This re-evaluation only reads catalog tables. Compared with ALTER =
TABLE operations that rewrite table data, the cost should not be too =
bad.

* More importantly, I don=E2=80=99t think caching update results is the =
right solution. The root cause is that find_all_inheritors() cannot =
ensure that a child appears after all of its parents in the returned =
list. We could add an alternative helper that returns inheritance =
descendants in topological order. With that, the current recurse-up =
logic could be avoided, and maybe the changing_oids list could be =
avoided as well. But it is too late for v19. I have added this to my =
TODO list and will work on it for v20.

In summary, I think the current re-evaluation has very limited =
performance impact and is acceptable for v19. We can improve the =
algorithm with a better solution in v20.

BTW, do you have any comments on the doc changes in 0002 and 0003?

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