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

On Tue, Mar 21, 2023 at 12:21 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> This doesn't look hugely promising to me. We do need to do something
> like "Vars/PHVs referencing join X now need to also reference join Y",
> but I think we need to actually change the individual Vars/PHVs not
> just fake it by hacking the required_relids of RestrictInfos. Otherwise
> we're going to get assertion failures in setrefs.c about nullingrel
> sets not matching up.

Agreed that we need to actually change the individual Vars/PHVs because
I find that with my patch it's not easy to fake the quals deduced from
ECs as there is no 'current JoinTreeItem' there. I'm not sure that
there is nullingrels mis-match issue though, because when forming an
outer join's target list we've ensured that the output Vars are always
marked according to the syntactic tree structure.

> So the idea that I had was to, once we detect that X commutes with Y,
> immediately call a function that recurses through the relevant parts
> of root->parse and performs the required nullingrels updates.

So IIUC the idea is that if we are given the first form of identity 3,
we update all upper C Vars so that they are marked with both OJ relids.
It seems that this may have nullingrels mis-match issue because of how
we form outer join's target list. Consider

A leftjoin B on (Pab) leftjoin C on (Pbc) inner join D on (Pcd)

At first C Vars in Pcd is marked only with B/C join. Then we detect
that A/B join commutes with B/C join so we update C Vars in Pcd as being
marked with B/C join and A/B join. However C Vars in the target list of
joinrel A/B/C would be marked only with B/C join as that is consistent
with the syntactic tree structure. So when we process the joinqual Pcd
for C/D join in setrefs.c, I think we would have assertion failures
about nullingrels mismatch. (This is just my imagination. I haven't
implemented the codes to verify it.)

Thanks
Richard