Re: Making Vars outer-join aware

Zhihong Yu <zyu@yugabyte.com>

From: Zhihong Yu <zyu@yugabyte.com>

To: Tom Lane <tgl@sss.pgh.pa.us>

Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Richard Guo <guofenglinux@gmail.com>, "Finnerty, Jim" <jfinnert@amazon.com>

Date: 2022-07-10T21:04:41Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Re-allow INDEX_VAR as rt_index in ChangeVarNodes().
- fbf80421ead5 16.0 landed
Fix thinkos in have_unsafe_outer_join_ref; reduce to Assert check.
- f50f029c497d 16.0 landed
Invent "join domains" to replace the below_outer_join hack.
- 3bef56e11650 16.0 landed
Do assorted mop-up in the planner.
- b448f1c8d83f 16.0 landed
Make Vars be outer-join-aware.
- 2489d76c4906 16.0 landed
Invent "multibitmapsets", and use them to speed up antijoin detection.
- e9e26b5e7166 16.0 landed
Add basic regression tests for semi/antijoin recognition.
- 0043aa6b8597 16.0 landed
Improve performance of adjust_appendrel_attrs_multilevel.
- 2f17b57017e5 16.0 landed
Refactor addition of PlaceHolderVars to joinrel targetlists.
- afa0ec30bfd1 16.0 landed
Use an explicit state flag to control PlaceHolderInfo creation.
- b3ff6c742f6c 16.0 landed
Make PlaceHolderInfo lookup O(1).
- 6569ca43973b 16.0 landed

On Sun, Jul 10, 2022 at 12:39 PM Tom Lane <tgl@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