I wrote:
> ... which is the wrong join order: the filter condition can't be
> applied at that join level. So the nullingrel cross-check
> has caught a real bug, but why the bug? Pre-v16, this would
> have been prevented by the delay_upper_joins mechanism.
> I convinced myself that we didn't need that anymore, but
> maybe I was mistaken. It could also be some smaller problem.
> It's curious that the bug doesn't reproduce if you remove the
> visibly-useless join to ss2:
Ah-hah, I now understand why that is. Without the join to ss2,
the FROM/WHERE clause is directly below the left join to int4_tbl,
and remove_useless_result_rtes will hoist the problematic WHERE
qual up into the upper left join's quals -- see the para beginning
"This pass also replaces single-child FromExprs with their child node"
in prepjointree.c. After that, we can see that the left join's quals
reference both sides of the lower left join so identity 3 cannot apply.
With the join to ss2, that intervening join prevents the hoisting from
happening and then we incorrectly conclude that identity 3 can be used.
I'm inclined to think that temporarily hoisting such quals into the
upper left join's qual list is still the best solution, as anything
else would require weird and bug-prone action-at-a-distance checks
during deconstruct_jointree. However, we need to make it happen in
this case where the hoisting needs to pass a qual from a lower WHERE
in a left join's LHS up to the RHS of a higher left join.
(I think that this is the only missing case. Intermediate joins
that aren't LEFT will prevent commutation anyway, as will multi-
member FROM joins.)
I'm not sure if the best way is to extend that logic in
remove_useless_result_rtes, or to rip it out and handle the
problem during deconstruct_jointree. The latter would probably
involve more new code, but it might end up cleaner. This whole
business of removing trivial FromExprs is a bit outside what
you'd expect remove_useless_result_rtes to do. If memory serves,
I wrote that logic before inventing the new multi-pass architecture
for deconstruct_jointree; it's possible that that change would
make it easier to deal with this in deconstruct_jointree.
I have no time to write any code today, but that seems like the
direction to pursue.
regards, tom lane
