On Thu, Nov 17, 2022 at 4:46 AM Tom Lane <t...@sss.pgh.pa.us> wrote:

> So we've marked the 4 and 7 joins as possibly commuting, but they
> cannot commute according to 7's min_lefthand set.  I don't think
> the extra clone condition is terribly harmful --- it's useless
> but shouldn't cause any problems.  However, if these joins should be
> able to commute then the min_lefthand marking is preventing us
> from considering legal join orders (and has been doing so all along,
> that's not new in this patch).  It looks to me like they should be
> able to commute (giving your third form), so this is a pre-existing
> planning deficiency.


Yeah.  This is an issue that can also be seen on HEAD and is discussed
in [1].  It happens because when building SpecialJoinInfo for 7, we find
A/B join 5 is in our LHS, and our join condition (Pcd) uses 5's
syn_righthand while is not strict for 5's min_righthand.  So we decide
to preserve the ordering of 7 and 5, by adding 5's full syntactic relset
to 7's min_lefthand.  As discussed in [1], maybe we should consider 5's
min_righthand rather than syn_righthand when checking if Pcd uses 5's
RHS.


> > Looking at the two forms again, it seems the expected two versions for
> > Pcd should be
> >     Version 1: C Vars with nullingrels as {B/C}
> >     Version 2: C Vars with nullingrels as {B/C, A/B}
> > With this we may have another problem that the two versions are both
> > applicable at the C/D join according to clause_is_computable_at(), in
> > both forms.
>
> At least when I tried it just now, clause_is_computable_at correctly
> rejected the first version, because we've already computed A/B when
> we are trying to form the C/D join so we expect it to be listed in
> varnullingrels.


For the first version of Pcd, which is C Vars with nullingrels as {B/C}
here, although at the C/D join level A/B join has been computed and
meanwhile is not listed in varnullingrels, but since Pcd does not
mention any nullable Vars in A/B's min_righthand, it seems to me this
version cannot be rejected by clause_is_computable_at().  But maybe I'm
missing something.

[1]
https://www.postgresql.org/message-id/flat/CAMbWs4_8n5ANh_aX2PinRZ9V9mtBguhnRd4DOVt9msPgHmEMOQ%40mail.gmail.com

Thanks
Richard

Reply via email to