Re: BUG #19553: Wrong results from nested LEFT JOINs over an empty subquery (regression since v16)

"Matheus Alcantara" <matheusssilv97(at)gmail(dot)com> writes:
> On Thu Jul 16, 2026 at 11:12 AM -03, PG Bug reporting form wrote:
>> The following self-contained query returns wrong results on every release
>> since v16:
>>
>> select * from (values (1),(2)) v(x)
>> left join (select q from (select 7 as q from (select where false) ss1)
>> ss2
>> left join (select 8 as z) ss3 on true) ss4 on true;

> According to my findings, the issue seems to be in
> remove_useless_results_recurse(), specifically in find_dependent_phvs().

Yeah, I had just come to the same conclusion: we are deciding that the
PHV is not dependent on the RTE_RESULT we're considering removing, but
it really is.

> Attached patch seems to fix find_dependent_phvs() to test membership
> rather than exact equality, since that's what actually indicates the
> PHV's value depends on that relation.

I had thought of that too, but I think it is wrong and will result in
not pursuing optimizations that are valid. I instrumented the code
like this:

*** 4309,4314 ****
--- 4309,4319 ----
if (phv->phlevelsup == context->sublevels_up &&
bms_equal(context->relids, phv->phrels))
return true;
+ if (phv->phlevelsup == context->sublevels_up &&
+ bms_is_subset(context->relids, phv->phrels))
+ elog(WARNING, "dubious case detected: looking for %s, PHV has %s",
+ bmsToString(context->relids),
+ bmsToString(phv->phrels));
/* fall through to examine children */
}

and observed that this warning fires in several join.sql cases that
are not giving wrong answers. (Unfortunately, those tests only check
the query results not the plan, so they'd not show any change in
behavior from your patch.)

What I see here is that the PHV in question initially has

:phrels (b 7 9 10) -- lower OJ, RESULT, RESULT
:phnullingrels (b 3) -- upper OJ

and we decide that RTE 10 can be removed, leaving

:phrels (b 7 9) -- lower OJ, RESULT
:phnullingrels (b 3) -- upper OJ

That's fine, but when we come to consider RTE 9, we decide it can be
removed, which is wrong. I think the core of the problem here is that
this code was written back when phrels contained only baserels, and
now that it also contains OJ rels, we're mistakenly concluding that
the presence of those bits indicates there's another place to evaluate
the PHV.

So the simplest fix is probably to mask off OJ bits and consider only
baserels when deciding if phrels equals the target. Unfortunately,
this happens long before we compute root->all_baserels or anything
like that, so remove_useless_result_rtes is on its own to figure out
which those are. I think we can extend it to build a bitmapset of
relevant baserel RT indexes while it is scanning the tree (so that we
don't need an additional recursive scan just to get that). But I've
not tried to write any code yet; do you feel like attacking that?

find_dependent_phvs_in_jointree most likely needs the same fix.
I don't believe your conclusion that it should act differently.

BTW, I think "git bisect"'s finding that the bug started with
commit 3af87736b is mostly accidental. That commit removed a
different limitation preventing the intermediate FromExpr from
getting flattened, allowing the problem to be reached.

regards, tom lane