| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | "Matheus Alcantara" <matheusssilv97(at)gmail(dot)com> |
| Cc: | leis(at)in(dot)tum(dot)de, pgsql-bugs(at)lists(dot)postgresql(dot)org, "Richard Guo" <guofenglinux(at)gmail(dot)com> |
| Subject: | Re: BUG #19553: Wrong results from nested LEFT JOINs over an empty subquery (regression since v16) |
| Date: | 2026-07-17 17:20:23 |
| Message-ID: | 3957687.1784308823@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
"Matheus Alcantara" <matheusssilv97(at)gmail(dot)com> writes:
> On Thu Jul 16, 2026 at 8:23 PM -03, Tom Lane wrote:
>> So the simplest fix is probably to mask off OJ bits and consider only
>> baserels when deciding if phrels equals the target.
> Attached patch does this: remove_useless_result_rtes() now precomputes a
> Relids of all non-join RT indexes by scanning root->parse->rtable once
> up front (rather than doing a second recursive walk), and threads it
> through remove_useless_results_recurse() down to both
> find_dependent_phvs() and find_dependent_phvs_in_jointree().
I think that Richard's suggestion of using get_relids_in_jointree is
superior: the rtable may contain RTEs that are no longer relevant,
such as views that have been expanded. In principle including those
in the mask wouldn't make a difference, but it seems messy.
I was thinking yesterday that we could calculate the mask on the fly
because by the time we are making decisions at a join node, we've
already recursively visited all its children, and those should include
all the baserels that are relevant. But I do agree with Richard that
that "should" feels maybe a wee bit shaky, and it's not like the join
tree would be so big here that we can't afford one more recursive
traversal.
On the third hand, I also find Richard's suggestion of relying on
dropped_outer_joins to be shaky. It's far from clear to me that
not-dropped outer joins couldn't also break this test. This code
was designed to consider only baserels, so I think the safest route
to a fix is to restore it to doing that. (Also, dropped_outer_joins
seems like it suffers from the same objection that maybe it doesn't
*yet* include every outer join that is relevant.)
So here's a v3 that is basically Matheus' code, but with the rtable
scan replaced with get_relids_in_jointree, and some other cosmetic
changes. (Notably, I changed argument order to preserve our usual
convention that output arguments come last.) I preferred Richard's
commentary on the test case though.
If no objections, I'll push and backpatch soon.
Another thought for the future: at least in this example, it seems
like removing the second RTE_RESULT and its parent outer join would be
perfectly valid, if what we did to clean up is replace the dependent
PHV(s) with null Consts. I'm not sure that such cases arise often
enough to be worth the trouble (if they did we'd likely have heard
about this bug long ago), but it's interesting to think about.
regards, tom lane
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Fix-edge-case-in-remove_useless_result_rtes-with-.patch | text/x-diff | 11.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-07-17 17:42:32 | Re: CREATE INDEX with an expression in an INCLUDE column fails with XX000 "unrecognized node type" instead of 0A000 on master |
| Previous Message | Maaz Syed Adeeb | 2026-07-17 16:32:10 | Re: CREATE INDEX with an expression in an INCLUDE column fails with XX000 "unrecognized node type" instead of 0A000 on master |