Horton

Re: Making Vars outer-join aware

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

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Zhihong Yu <zyu@yugabyte.com>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Richard Guo <guofenglinux@gmail.com>, "Finnerty, Jim" <jfinnert@amazon.com>
Date: 2022-07-10T23:51:28Z
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 →

  1. Re-allow INDEX_VAR as rt_index in ChangeVarNodes().

  2. Fix thinkos in have_unsafe_outer_join_ref; reduce to Assert check.

  3. Invent "join domains" to replace the below_outer_join hack.

  4. Do assorted mop-up in the planner.

  5. Make Vars be outer-join-aware.

  6. Invent "multibitmapsets", and use them to speed up antijoin detection.

  7. Add basic regression tests for semi/antijoin recognition.

  8. Improve performance of adjust_appendrel_attrs_multilevel.

  9. Refactor addition of PlaceHolderVars to joinrel targetlists.

  10. Use an explicit state flag to control PlaceHolderInfo creation.

  11. Make PlaceHolderInfo lookup O(1).

Zhihong Yu <zyu@yugabyte.com> writes:
> In remove_unneeded_nulling_relids():

> +   if (removable_relids == NULL)

> Why is bms_is_empty() not used in the above check ?

We initialized that to NULL just a few lines above, and then did
nothing to it other than perhaps bms_add_member, so it's impossible
for it to be empty-and-yet-not-NULL.

> +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.

We generally document these sorts of structs in the using code,
not at the struct declaration.

> +       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?

Like what?  We don't particularly care what the join type is here,
as long as it's not INNER.  In any case there is plenty of nearby
code checking for unsupported join types.

			regards, tom lane