| From: | Tender Wang <tndrwang(at)gmail(dot)com> |
|---|---|
| To: | Richard Guo <guofenglinux(at)gmail(dot)com> |
| Cc: | Nitin Motiani <nitinmotiani(at)google(dot)com>, leis(at)in(dot)tum(dot)de, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: BUG #19765: Wrong result from a chain of LEFT JOINs since v16 |
| Date: | 2026-10-23 03:42:10 |
| Message-ID: | CAHewXN=iAqBRdg-jWSZhjinN3+qjMSsiaLJRGHytqR=4yOcDaQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
Richard Guo <guofenglinux(at)gmail(dot)com> 于2026年10月22日周四 09:50写道:
>
> On Wed, Oct 21, 2026 at 7:30 PM Nitin Motiani <nitinmotiani(at)google(dot)com> wrote:
> > Please take a look and let me know which alternative seems better to you.
>
> Thanks for the patch! It's on my radar, but I'm currently occupied
> with a few other items. I'll do my best to get to it before the minor
> release.
Thanks for the patch. The v2 patch fixes the wrong-result case, but
while reviewing it I noticed that it can retain more join clauses than
necessary.
For example, I forced the following join order using pg_plan_advice:
JOIN_ORDER(t1 t2 (t3 t4))
for the query:
select t1.a, t2.b, t3.c, t4.d
from t1
left join t2 on t1.a = t2.b
left join t3 on t2.b = t3.c
left join t4 on t3.c = t4.d
where t1.a = 1;
With v2, the relevant part of the plan is:
QUERY PLAN
-------------------------------------------
Hash Left Join
Hash Cond: (t2.b = t3.c)
-> Nested Loop Left Join
-> Seq Scan on t1
Filter: (a = 1)
-> Materialize
-> Seq Scan on t2
Filter: (b = 1)
-> Hash
-> Hash Left Join
Hash Cond: (t3.c = t4.d)
-> Seq Scan on t3
Filter: (c = 1)
-> Hash
-> Seq Scan on t4
Filter: (d = 1)
Supplied Plan Advice:
JOIN_ORDER(t1 t2 (t3 t4)) /* matched */
The (t2.b = t3.c) condition is necessary at this join level: t2.b may
already have been null-extended by the t1/t2 outer join.
However, (t3.c = t4.d) is redundant in the directly formed t3/t4 join.
At that level, t3.c is still the value from the preserved base
relation, and constant propagation has already restricted both sides
to 1.
Thus, retaining every affected cloned clause whenever a commuting
outer join exists is somewhat conservative.
I think the underlying issue is that the redundancy of such a clause
is placement-dependent. Constant propagation itself is valid, but it
does not imply that the original strict join clause can be discarded
at every join level at which its clone might be applied.
The attached v3 patch therefore separates those two decisions.
During reconsider_outer_join_clauses(), constant propagation still
proceeds as before.
For a cloned clause whose outer operand can be null-extended by a
commuting lower outer join, however, the patch retains the original
RestrictInfo and records the relevant outer-join relids instead of
replacing the clause unconditionally with a constant-TRUE dummy.
When constructing the restrictlist for a particular joinrel, the
original clause is included only when a recorded outer join: has both
of its minimum inputs available, meaning that it may already have
performed null-extension;
does not yet have its outer-join relid in the available relids; and is
still waiting for a commuting lower outer join to be completed.
This is the state in which the outer operand may already be NULL even
though the least-nulled clone remains applicable, so the strict join
clause is still required. At other join levels the retained clause is
omitted as redundant.
With v3 version:
the original wrong-result query returns 1 | 1 | NULL | NULL;
for the forced t1 t2 (t3 t4) join order, t2.b = t3.c is retained;
t3.c = t4.d is removed from the direct t3/t4 join; and the tree builds
successfully with assertions enabled.
The v3 patch is attached.
Thoughts?
--
Thanks,
Tender Wang
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Fix-wrong-result.patch | text/plain | 7.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Rachitskiy | 2026-10-23 04:49:29 | Re: BUG #19752: GROUP BY on a constant and a cast to a length-limited array type crashes the server while planning |
| Previous Message | Hayato Kuroda (Fujitsu) | 2026-10-23 03:24:43 | RE: START_REPLICATION silently truncates an overlong LSN component |