Re: Fix bug of CHECK constraint enforceability recursion

> On Jun 5, 2026, at 03:08, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
>
> The new version looks correct, I don't see any logic problem with it,
> however, I do have a performance question:
>
> + /*
> + * 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))
>
> 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

* 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’t 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/