Re: remove_useless_joins vs. bug #19560

Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> On Tue, Jul 21, 2026 at 3:52 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I have a modest proposal to make instead: let's nuke all that logic
>> from orbit. Revise analyzejoins.c so that it does join removals
>> working strictly on the jointree representation, which is simpler
>> and far more stable than any of the planner's derived data. Then,
>> if we successfully removed any joins, throw away all the derived
>> data and loop back around within query_planner() to redo
>> deconstruct_jointree and all the rest of it.

> I prototyped this proposal to see how it would look in code, mainly
> copying how remove_useless_result_rtes does the removal from the join
> tree. See attached PoC. Please note that this is far from
> review-ready, and it only covers outer-join removal, skipping
> self-join elimination for now, and it lacks badly proper comments and
> test cases.

I spent some time hacking on this with the aid of Claude Code, and
arrived at what seems a complete patch. It's a net deletion of
over 900 lines of code, and the newly-added code is mostly very
straightforward recursions over the jointree.

I had Claude do some performance testing, and the only case that got
noticeably slower was removal of multiple self-joins (about 10%
planning time slowdown for removal of 8 self-joins). I'm not super
concerned about that; it doesn't seem like such cases would be common.

What I'm pretty unclear on is whether we want to risk back-patching
this. It's a big change, and I can't honestly promise that it doesn't
bring some new bugs. Still, it fixes one known bug and very likely
some not-yet-known ones. Perhaps a reasonable choice would be to
back-patch to v18 where self-join elimination came in, because I still
have very little faith in that code.

regards, tom lane