Thanks for the review!
On Mon, Feb 2, 2026 at 2:37 PM Alexandra Wang
<[email protected]> wrote:
> 0001:
> The code looks good to me. However, I feel a bit uneasy about not
> seeing a test case for the additional subplan origin display added in
> pg_overexplain. Maybe we could add the following test cases to
> exercise that code:
Done.
> 0002:
> Looks good to me.
Cool.
> 0003:
> in accumulate_append_subpath(), but in get_singleton_append_subpath()
> there are only calls to lappend() and no list_concat(). Is that
> intentional? Do we also want to concatenate the newly pulled up
> child_append_relid_sets with the existing ones in
> get_singleton_append_subpath()?
Oh, good catch! Adjusted.
> In add_paths_to_append_rel():
>
> @@ -1785,13 +1790,16 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo
> *rel,
> {
> Path *path = (Path *) lfirst(l);
> AppendPath *appendpath;
> + AppendPathInput append = {0};
> +
> + append.partial_subpaths = list_make1(path);
> + append.child_append_relid_sets = list_make1(rel->relids);
>
> Could you help me understand why we need to populate
> append.child_append_relid_sets here? I don’t see this child rel being
> pulled up at this point.
Oops, good point. Adjusted this, too.
> 0004:
> I’ve only read through the README and documentation so far; I’ll
> continue reviewing the code in 0004.
Thanks!
I plan to post an updated patch set shortly.
--
Robert Haas
EDB: http://www.enterprisedb.com
