| From: | solai v <solai(dot)cdac(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, jian he <jian(dot)universality(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Subject: | Re: Introducing find_all_inheritors_ordered() |
| Date: | 2026-09-04 07:15:19 |
| Message-ID: | CAF0whue-TdKNruMrgFYhL5+rJVQoXHLHjUe_RRpzBY_B0PjhEg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi all,
On Mon, Aug 31, 2026 at 2:34 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
> Hi,
>
> This is follow-up work to patch [1], which fixed a bug when altering a CHECK constraint's enforceability. The fix was not very elegant. It had to use upward recursion to traverse all ancestors when deciding a child table's enforceability. This was because the existing function find_all_inheritors() returns a list of descendants without ensuring that parents precede their children.
>
> This patch introduces a new function, find_all_inheritors_ordered(), which guarantees that every ancestor in the returned list appears before its descendants. With this new function, the original fix in commit 0cd17fdd3c0 is significantly simplified. The function could also potentially benefit other features that need to traverse inheritance trees in parent-before-child order.
>
> This patch also strengthens an existing test by adding another level of inheritance. My first version of the implementation failed with the following case:
> ```
> Root ———————————————> child
> \ /
> \ ——————> a —————> b /
> ```
> (The diagram might not display well. Basically, “child" has parents “b" and “root", “b" has parent “a”, “a” has parent “root")
>
> The current version uses Kahn's topological sorting algorithm, which handles this case correctly. Please see the attached patch for details.
>
> [1] https://postgr.es/m/E74C57FA-1DD0-4C8E-8FB1-538034752592@gmail.com
>
I reviewed and tested the patch. I was able to reproduce the ordering
issue with find_all_inheritors() using a multiple-inheritance
hierarchy where a descendant can be returned before one of its
ancestors. After the patch, verified that the
find_all_inheritors_ordered() returns the relations in
ancestor-before-descendant order for the same hierarchy. And also
reviewed the changes in tablecmds.c. The removal of the
changing_conids handling and the upward recursive check looks correct
to me. The ordered traversal ensures that the affected parent
constraints are processed before their descendants, and the
CommandCounterIncrement() makes the updated parent constraint state
visible to subsequent processing. The updated regression test provides
coverage for a deeper inheritance hierarchy with multiple inheritance.
I also verified that the existing CHECK constraint enforceability
behavior is preserved. The inherit and constraints regression test
suites were executed successfully. The complete check-world test suite
was also run and all tests passed. I did not find any correctness
issues with the patch and the patch looks good to me.
Regards,
Solai
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nisha Moond | 2026-09-04 07:26:57 | Re: Follow-up review items for update_deleted |
| Previous Message | Hayato Kuroda (Fujitsu) | 2026-09-04 06:59:38 | RE: Fix resource leak in FindConflictTuple() retry path |