Re: plan shape work

On Wed, Sep 3, 2025 at 5:07 AM Robert Haas <robertmhaas@gmail.com> wrote:
> Thanks for the review. Responding just briefly to avoid quoting too much text:
>
> - I'm glad to hear that you like 0002 and consider it an improvement
> independent of what follows.
> - I'm glad to hear that you're OK with the current split between 0001 and 0002.
> - I would like opinions on those topics from more people.
> - I have attempted to fix all of the other mistakes that you pointed
> out in the attached v3, which is also rebased.

I've reviewed 0001 to 0003, and they all make sense to me.  The
changes to the EXPLAIN output in 0001 and 0002 are very nice
improvements.

I found some issues with 0003 though.  It seems get_scanned_rtindexes
is intended to return RTI sets with outer join relids excluded.  For
some node types, such as Append and MergeAppend, it fails to do so,
which can cause the assertion in assert_join_preserves_scan_rtis to
fail.  For example:

create table p (a int, b int) partition by range(a);
create table p1 partition of p for values from (0) to (10);
create table p2 partition of p for values from (10) to (20);

set enable_partitionwise_join to on;

explain (costs off)
select * from p t1
    left join p t2 on t1.a = t2.a
    left join p t3 on t2.b = t3.b;
server closed the connection unexpectedly

Besides, to exclude outer join relids, it iterates over the RTI sets,
checks each RTE for type RTE_JOIN, and bms_del_member it if found (cf.
remove_join_rtis).  I think a simpler approach would be to leverage
PlannerInfo.outer_join_rels:

    scanrelids = bms_difference(scanrelids, root->outer_join_rels);

Therefore, I suggest that we don't try to remove the join RTIs in
get_scanned_rtindexes.  Instead, we do that in
assert_join_preserves_scan_rtis -- before comparing the RTIs from the
outer and inner subplans with the join's RTIs -- by leveraging
PlannerInfo.outer_join_rels.  And remove_join_rtis can be retired.

- Richard