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

On Wed, Feb 22, 2023 at 6:24 PM Richard Guo <guofenglinux@gmail.com> wrote:

> ISTM that for outer join identity 3, if we are given form
>     (A leftjoin B on (Pab)) leftjoin C on (Pbc)
> then references to C Vars in higher qual levels would be marked with the
> B/C join.  If we've transformed it to form
>     A leftjoin (B leftjoin C on (Pbc)) on (Pab)
> then references to C Vars in higher qual levels should be adjusted to
> include both B/C join and A/B join in their varnullingrels.
>

A quick hack that comes to my mind is that for a pushed down clause we
check all outer join relids it mentions and add the outer joins'
commute_below to the clause's required_relids, so that after we've
commuted the outer joins, the clause would still be placed in the right
place.

--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -2349,12 +2349,27 @@ distribute_qual_to_rels(PlannerInfo *root, Node
*clause,
    }
    else
    {
+       ListCell   *l;
+
        /*
         * Normal qual clause or degenerate outer-join clause.  Either way,
we
         * can mark it as pushed-down.
         */
        is_pushed_down = true;

+       /*
+        * Add in commute_below of outer joins mentioned within the clause,
so
+        * that after we've commuted the outer joins, the clause would
still be
+        * placed correctly.
+        */
+       foreach(l, root->join_info_list)
+       {
+           SpecialJoinInfo *sji = (SpecialJoinInfo *) lfirst(l);
+
+           if (bms_is_member(sji->ojrelid, relids))
+               relids = bms_add_members(relids, sji->commute_below);
+       }
+

For a formal fix, I wonder if we need to generate multiple versions of
such a clause and apply the appropriate one depending on which join
order is chosen, just like what we do for left join quals in
deconstruct_distribute_oj_quals.

Thanks
Richard