> On Jun 5, 2026, at 03:08, Zsolt Parragi <[email protected]> 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/
