Re: Making Vars outer-join aware

On Sun, Jul 10, 2022 at 12:39 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Here's v2 of this patch series. It's functionally identical to v1,
> but I've rebased it over the recent auto-node-support-generation
> changes, and also extracted a few separable bits in hopes of making
> the main planner patch smaller. (It's still pretty durn large,
> unfortunately.) Unlike the original submission, each step will
> compile on its own, though the intermediate states mostly don't
> pass all regression tests.
>
> regards, tom lane
>
> Hi,
For v2-0004-cope-with-nullability-in-planner.patch.
In remove_unneeded_nulling_relids():

+ if (removable_relids == NULL)

Why is bms_is_empty() not used in the above check ?
Earlier there is `if (bms_is_empty(old_nulling_relids))`

+typedef struct reduce_outer_joins_partial_state

Since there are already reduce_outer_joins_pass1_state
and reduce_outer_joins_pass2_state, a comment
above reduce_outer_joins_partial_state would help other people follow its
purpose.

+ if (j->rtindex)
+ {
+ if (j->jointype == JOIN_INNER)
+ {
+ if (include_inner_joins)
+ result = bms_add_member(result, j->rtindex);
+ }
+ else
+ {
+ if (include_outer_joins)

Since there are other join types beside JOIN_INNER, should there be an
assertion in the else block ? e.g. jointype wouldn't be JOIN_UNIQUE_INNER.

Cheers