Horton

Re: Making Vars outer-join aware

Anton A. Melnikov <aamelnikov@inbox.ru>

View thread
From: "Anton A. Melnikov" <aamelnikov@inbox.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Richard Guo <guofenglinux@gmail.com>, pgsql-hackers@lists.postgresql.org, "Finnerty, Jim" <jfinnert@amazon.com>
Date: 2023-05-29T12:01:03Z
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).

Attachments

Hello and sorry for the big delay in reply!

On 04.05.2023 15:22, Tom Lane wrote:
> AFAICS the change you propose would serve only to mask bugs.

Yes, it's a bad decision.

> Under what circumstances would you be trying to inject INDEX_VAR
> into a nullingrel set?  Only outer-join relids should ever appear there.

The thing is that i don't try to push INDEX_VAR into a nullingrel set at all,
i just try to replace the existing rt_index that equals to INDEX_VAR in Var nodes with
the defined positive indexes by using ChangeVarNodes_walker() function call. It checks
if the nullingrel contains the existing rt_index and does it need to be updated too.
It calls bms_is_member() for that, but the last immediately throws an error
if the value to be checked is negative like INDEX_VAR.

But we are not trying to corrupt the existing nullingrel with this bad index,
so it doesn't seem like an serious error.
And this index certainly cannot be a member of the Bitmapset.

Therefore it also seems better and more logical to me in the case of an index that
cannot possibly be a member of the Bitmapset, immediately return false.

Here is a patch like that.

With the best regards,

-- 
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company