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.