Skip to content

[4.1-dev] fix: resolve comma-joined tables in nested explicit JOIN ON conditions#25360

Open
aunjgr wants to merge 3 commits into
matrixorigin:4.1-devfrom
aunjgr:fix/24411-comma-join-cte-4.1
Open

[4.1-dev] fix: resolve comma-joined tables in nested explicit JOIN ON conditions#25360
aunjgr wants to merge 3 commits into
matrixorigin:4.1-devfrom
aunjgr:fix/24411-comma-join-cte-4.1

Conversation

@aunjgr

@aunjgr aunjgr commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • BUG

Which issue(s) this PR fixes:

issue #24411

What this PR does / why we need it:

FROM a, b JOIN c ON a.x = c.y failed with "missing FROM-clause entry". Cherry-pick of #25353 to 4.1-dev.

buildJoinTable now accepts extLeftCtx, temporarily adds bindings for ON condition resolution, then removes them before the outer mergeContexts.

File Change
pkg/sql/plan/query_builder.go +33 / -2 lines
pkg/sql/plan/build_test.go +8 lines
test/.../join/join.sql +12 lines

🤖 Generated with Claude Code

aunjgr and others added 2 commits July 2, 2026 14:17
When comma (CROSS) join syntax is mixed with explicit JOIN
(e.g. FROM a, b JOIN c ON a.x = c.y), the parser produces
CROSS(a, INNER(b, c, ON a.x = c.y)). The outer left context
containing 'a' was lost when buildTable dispatched to
buildJoinTable for the nested INNER join.

Fix: pass extLeftCtx through to buildJoinTable, temporarily
add its bindings to ctx for ON condition resolution, then
remove them so the outer mergeContexts doesn't see a duplicate.

Co-Authored-By: Claude <noreply@anthropic.com>
The extBindingsRestore was only called inside OnJoinCond, but comma
joins have no ON condition — extLeftCtx bindings leaked into ctx and
collided at outer mergeContexts. Move restore after the switch so all
join types clean up.

Co-Authored-By: Claude <noreply@anthropic.com>
@aunjgr aunjgr requested review from heni02 and ouyuanning as code owners July 2, 2026 06:18
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@matrix-meow matrix-meow added the size/M Denotes a PR that changes [100,499] lines label Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants