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

On Thu, May 4, 2023 at 7:22 PM Richard Guo <guofenglinux@gmail.com> wrote:

> On Wed, Mar 1, 2023 at 3:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> Here's said patch.  Although this fixes the described problem and
>> passes check-world, I'm not totally happy with it yet: it feels
>> like the new add_outer_joins_to_relids() function is too expensive
>> to be doing every time we construct a join relation.  I wonder if
>> there is a way we can add more info to the SpecialJoinInfo data
>> structure to make it cheaper.  An obvious improvement is to store
>> commute_below_l explicitly instead of recomputing it over and over,
>> but that isn't going to move the needle all that far.  Is there a
>> way to not have to scan all the SpecialJoinInfos?
>
>
> I'm thinking about the way to improve add_outer_joins_to_relids() and
> here is what I come up with.  When we've completely formed a pushed up
> outer join, actually we only need to consider the pushed-down joins that
> are in its commute_above_l.  But note that this process should be
> recursive, meaning that if an outer join in its commute_above_l is also
> considered qualified to be added to the relid set, we need to consider
> that outer join's commute_above_l too.  By this way we only need to
> check the relevant SpecialJoinInfos, rather than scan the whole
> join_info_list.
>
> To do it we need a way to fetch SpecialJoinInfo by ojrelid.   So 0001
> patch introduces join_info_array for direct lookups of SpecialJoinInfo
> by ojrelid.  I find that it also benefits some existing functions, such
> as clause_is_computable_at() and have_unsafe_outer_join_ref().  So I
> started a new thread for it at [1].
>
> 0002 is the original patch that introduces add_outer_joins_to_relids().
>
> 0003 patch implements the improvement to add_outer_joins_to_relids().
> All the pushed down joins that are needed to check are kept in
> commute_above_relids.  Each time we fetch and remove the first outer
> join from commute_above_relids and check if that outer join is qualified
> to be added to the relid set.  If so we add it and also add its
> commute_above_l to commute_above_relids.  This process continues until
> commute_above_relids becomes empty.
>
> 0004 patch adjusts outer join's target list as described before, but
> leverages the new join_info_array to fetch relevant SpecialJoinInfos.
>

Hi Tom, how do you think about the v3 patch?

Thanks
Richard