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

On Fri, Feb 24, 2023 at 3:48 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I wonder whether we could instead fix things by deeming that the result
> of the pushed-down B/C join does not yet produce correct C* variables,
> so that we won't allow conditions involving them to drop below the
> pushed-up A/B join. This would be a little bit messy in some places
> because now we'd consider that the A/B join is adding two OJ relids
> not just one to the output join relid set, while the B/C join is adding
> no relids even though it must execute an outer join. (But we already
> have that notion for semijoins and some antijoins, so maybe it's fine.)

I think I see your points. Semijoins and antijoins derived from
semijoins do not have rtindex, so they do not add any OJ relids to the
output join relid set. Do you mean we do the similar thing to the
pushed-down B/C join here by not adding B/C's ojrelid to the output B/C
join, but instead add it later when we've formed the pushed-up A/B join?

I tried the codes below to adjust joinrelids and then use the modified
joinrelids when constructing restrict and join clause lists for the
joinrel. It seems to be able to solve the presented case. But I'm not
sure if it'd break other cases.

+ ListCell *lc;
+ Relids commute_below_l = NULL;
+
+ foreach(lc, root->join_info_list)
+ {
+ SpecialJoinInfo *otherinfo = (SpecialJoinInfo *) lfirst(lc);
+
+ if (otherinfo->jointype != JOIN_LEFT)
+ continue;
+
+ /* collect commute_below_l */
+ if (bms_is_member(otherinfo->ojrelid, sjinfo->commute_below) &&
+ bms_overlap(otherinfo->syn_righthand, sjinfo->syn_lefthand))
+ commute_below_l =
+ bms_add_member(commute_below_l, otherinfo->ojrelid);
+
+ /*
+ * add the pushed-down B/C join's relid to A/B join's relid set
+ */
+ if (bms_is_member(otherinfo->ojrelid, sjinfo->commute_above_l) &&
+ bms_overlap(otherinfo->min_righthand, joinrelids))
+ joinrelids = bms_add_member(joinrelids, otherinfo->ojrelid);
+ }
+
+ /*
+ * delete the pushed-down B/C join's relid from B/C join
+ */
+ if (!bms_is_empty(commute_below_l) &&
+ !bms_overlap(commute_below_l, joinrelids))
+ joinrelids = bms_del_member(joinrelids, sjinfo->ojrelid);

Also I'm wondering whether we can just copy what we once did in
check_outerjoin_delay() to update required_relids of a pushed-down
clause. This seems to be a lazy but workable way.

Thanks
Richard