fix: resolve comma-joined tables in nested explicit JOIN ON conditions

[aunjgr]

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 for table a". The comma-joined table was not visible in the explicit JOIN ON condition.

Root cause

The parser produces CROSS(a, INNER(b, c, ON a.x = c.y)) for comma + explicit JOIN. The outer CROSS join left context (containing a) was dropped when buildTable dispatched the inner INNER join to buildJoinTable — leftCtx was never passed through.

Fix

1. buildJoinTable now accepts extLeftCtx *BindContext
2. buildTable passes leftCtx through
3. In buildJoinTable, temporarily add extLeftCtx bindings to the ON condition binding context, then remove them so the outer mergeContexts does not see a duplicate

205/205 join BVT pass. Unit test covers the comma+explicit JOIN pattern.

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

🤖 Generated with Claude Code

[qodo-code-review]

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 →

[XuPeng-SH]

Requesting changes. I found a correctness blocker in the current direction.

1. test/distributed/cases/join/join.sql:178-179 / join.result:227-229 shows the new predicate is not actually applied at the intended scope. The data has r rows (1,100),(2,200) and s rows (1,'A'),(2,'B'); where r.val = ld.v keeps only r.id = 2, so if ON r.id = s.id is effective the only valid row is 2/B. The committed result is 2/A and 2/B, which means the binder now accepts r.id but the join predicate is placed on an inner join whose children do not include r, so it cannot enforce the condition correctly.

2. This also conflicts with MySQL join-scope semantics. MySQL documents that JOIN has higher precedence than comma, so t1, t2 JOIN t3 ON t1... is interpreted as t1, (t2 JOIN t3), and the ON clause may refer only to the join operands (t2 and t3), not t1: https://dev.mysql.com/doc/en/join.html. The current fix deliberately makes the outer comma-left context visible in the inner explicit JOIN ON clause, so it changes the language semantics instead of preserving MySQL compatibility.

Suggested direction:

• If MatrixOne wants MySQL-compatible behavior, keep the unparenthesized query as an error and update the issue/test to use either explicit CROSS JOIN or parentheses, e.g. (t_comma_join r, ... ld) JOIN t_comma_dim s ON ....
• If MatrixOne intentionally wants this extension, do not bind external refs into the inner join OnList. Rewrite/lift the predicate to a node whose child subtree actually contains all referenced tables, and add result-level regression coverage proving the expected output is only 2/B for the current BVT data.
• Add unhappy-path coverage for ambiguous/invalid cases: external left refs in LEFT/RIGHT/FULL JOIN ON, duplicate table aliases, ambiguous unqualified columns, and an explicit MySQL-compatible negative test for t1, t2 JOIN t3 ON t1... if compatibility is the goal.

Local verification I ran:

• go test -count=1 -timeout 180s ./pkg/sql/plan -run 'TestCommaJoinWithExplicitJoin|TestSubqueryInJoinOn|TestWrongCases'
• go test -count=1 -timeout 300s ./pkg/sql/plan

Both pass, but they do not cover the actual result correctness issue above.