Re: remove_useless_joins vs. bug #19560

On Sun, 26 Jul 2026 at 03:15, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Here's a patchset addressing that. 0001-0003 are the same as before,
> and then 0004 fixes this bug and adds tests.
>
> Your Claude session mentioned a different failure scenario, but
> gave no examples so I'm not quite sure if this covers it or not.

Thanks. I've re-tested against these and I'm no longer able to crash it.

The different failure scenario was brought up when Claude also tested
the latest patches:

<claude>

My evidence for a second case was that I'd prototyped a fix covering
only the collapsed-node path and it killed 33 of 46 fuzz crashes,
leaving 13. I described the surviving class but never reduced one,
which wasn't much use to anyone. It is:

a JoinExpr that does not collapse, but whose ON qual referenced the
removed relation, while the kept relation lives outside that JoinExpr.

create table a (a int primary key, b int);
create table b (a int, b int);
create table c (a int, b int);

select a2.a
from ((b b1 join a a1 on true) join c c1 on c1.a = a1.a)
join a a2 on a2.a = a1.a and a2.b = a1.b;

On v2 this trips the same Assert("root->hasLateralRTEs"). Here a1 is
removed and a2 kept. The inner b1 join a1 on true collapses to b1 —
that's the case your v2 code already reasoned about. But the middle
JoinExpr keeps both children (b1 and c1), so nothing collapses there;
its ON qual is simply rewritten from c1.a = a1.a to c1.a = a2.a, and
a2 isn't in that JoinExpr's scope. A fix that only inspects nodes
which lost a child can't see it.

v3 handles it, because you keyed the hoist off "mentions relid" rather
than off collapse:

Nested Loop
-> Seq Scan on b b1
-> Materialize
-> Hash Join
Inner Unique: true
Hash Cond: (c1.a = a2.a)
-> Seq Scan on c c1
-> Hash
-> Seq Scan on a a2
Filter: (a2.b IS NOT NULL)

Your two new regression tests both exercise the collapse path (the
second via semijoin reduction). Something like the above would be
worth adding as a third, since it's the case a narrower fix would have
missed — it needs a second non-self-joined table, so it doesn't drop
straight into the sj-only section.

</claude>

And it still reports the same previous planner regression, although
thinks it may be considered acceptable. It also mentioned adding
jointype assertions because the hoisting code doesn't have any.

But all good as far as I can see.

Thom