Skip to content

[4.1-dev] fix: allow scalar subquery in INNER JOIN ON condition#25359

Open
aunjgr wants to merge 3 commits into
matrixorigin:4.1-devfrom
aunjgr:fix/24996-subquery-in-join-on-4.1
Open

[4.1-dev] fix: allow scalar subquery in INNER JOIN ON condition#25359
aunjgr wants to merge 3 commits into
matrixorigin:4.1-devfrom
aunjgr:fix/24996-subquery-in-join-on-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 #24996

What this PR does / why we need it:

Scalar subqueries in JOIN ON conditions failed with NYI. Cherry-pick of #25339 to 4.1-dev.

TableBinder.BindSubquery now delegates to baseBindSubquery. Subquery conditions are flattened via flattenSubqueries and wrapped in FILTER above the join.

File Change
pkg/sql/plan/table_binder.go +1 / -1 line
pkg/sql/plan/build_test.go +8 lines
test/.../func_aggr_sum.result regenerated
test/.../join/join.sql +10 lines

🤖 Generated with Claude Code

Replace the NYI error in TableBinder.BindSubquery with the normal
baseBindSubquery path. Subquery conditions are detected after binding,
flattened via flattenSubqueries, and wrapped in a FILTER above the join
(same flow as bindWhere). No binder swapping needed — TableBinder
correctly handles aggregates and windows for JOIN ON context.

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 →

@XuPeng-SH XuPeng-SH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this still needs changes.

  1. JOIN ... ON subqueries are accepted by the binder now, but the join path still does not lower them into an executable form. buildTable() stores the bound ON predicates directly in node.OnList, and unlike WHERE / HAVING / projection paths there is still no flattenSubqueries() step for join predicates. Constant folding also does not rescue this: rule.IsConstant() returns false for Expr_Sub, so the scalar subquery stays inside the ON expression. At compile time non-equi ON predicates are forwarded to colexec.NewExpressionExecutor (extraJoinConditions() / constructLoopJoin()), and NewExpressionExecutor still has no Expr_Sub case. So removing the NYI in TableBinder.BindSubquery() makes plan building pass, but the execution path is still incomplete.

  2. This change is broader than JOIN ON. TableBinder is also used by buildTableFunction() and buildResultScan(). Those call sites still treat their arguments as plain expressions; result_scan in particular evaluates them through GetReadonlyResultFromNoColumnExpression() -> NewExpressionExecutor(), which still rejects Expr_Sub. So this PR also starts accepting subqueries in non-JOIN TableBinder callers that still do not support them.

The new unit test only exercises runOneStmt / plan construction, so it does not catch either runtime gap. I think the fix needs to be scoped to real JOIN-ON support end-to-end (including lowering/compile/runtime), or the old guard should stay in place until that support exists.

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/S Denotes a PR that changes [10,99] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants