Re: Clause accidentally pushed down ( Possible bug in Making Vars outer-join aware)

On Thu, Apr 20, 2023 at 1:06 PM Andrey Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
wrote:

> I have passed through your patch and the
> 'postpone-adding-pushed-down-ojrelid-to-relids' patch step-by-step. It
> looks logically correct.
> Honestly, I don't happy with the top patch, because it looks a bit
> peculiar, but the sqlancer long test (which detected the problem before)
> didn't show any problems here.

Thanks for testing the patches.

Currently in the patch the pushed down outer joins are collected in
add_outer_joins_to_relids() and then passed in build_joinrel_tlist()
where their relids might need to be added to Var's nulling bitmap.
Alternatively their relids can be calculated by removing
both_input_relids as well as sjinfo->ojrelid from joinrel->relids. But
in this way we'd have to loop over the whole join_info_list to get the
pushed down joins, which is less efficient than what the patch does.

> But one thought is bugging me: do we have a guarantee, what with these
> Identity 3 transformations and 'delaying up to completely performed'
> technique we don't lose some joininfo clauses?

Are you worried about upper clauses who reference to C vars? In the
second form when we've formed A/B join, which would have both outer
joins' relids in its relid set, the upper clause will be able to be
applied there, and it is the correct join level.

BTW, cfbot reminds that a rebase is needed. So re-attach the two
patches. Now I think it should be in 'Needs review' again in CF. I'll
go and do the change.

Thanks
Richard