The test case that made that fall over no longer does so, but I'm suspicious that this is still needed. I'm not quite sure though, and I'm even less sure about the corresponding change in remove_nulling_relids:
I'm not sure either. I cannot think of a scenario where this change would make a difference. But I think this change is good to have. It is more consistent with how we check if a PHV belongs to a relid set in build_joinrel_tlist(), where we use
bms_is_subset(phv->phrels, sjinfo->syn_righthand)
to check if the PHV comes from within the syntactically nullable side of the outer join.
2. Can we do anything to tighten make_outerjoininfo's computation of potential commutator pairs, as I speculated in [2]? This isn't a bug hazard anymore (I think), but adding useless bits there clearly does cause us to waste cycles later. If we can tighten that cheaply, it should be a win.
Agreed that we should try our best to get rid of non-commutable outer joins from commute_below_l. Not sure how to do that currently. Maybe we can add a TODO item for now?
3. In general, it seems like what your fixes are doing is computing transitive closures of the commute_above relationships. I wonder if we can save anything by pre-calculating the closure sets. Again, I don't want to add cycles in cases where the whole thing doesn't apply, but maybe we'd not have to.
Maybe we can pre-calculate the commute_above closure sets in make_outerjoininfo and save it in SpecialJoinInfo? I haven't thought through this.
BTW, it seems that there is a minor thinko in the changes. In the outer-join removal logic, we use syn_xxxhand to compute the relid set for the join we are considering to remove. I think this might be not right, because the outer joins may not be performed in syntactic order. Consider the query below
create table t (a int unique, b int);
explain (costs off) select 1 from t t1 left join (t t2 left join t t3 on t2.a = 1) on t2.a = 1; server closed the connection unexpectedly
Attached is a patch to revert this logic to what it looked like before.
