On Sun, Jul 10, 2022 at 12:39 PM Tom Lane <t...@sss.pgh.pa.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