Skip to content

fix: remap SortMergeJoin filter during projection pushdown - #24415

Merged
kosiew merged 8 commits into
apache:mainfrom
haohuaijin:codex/fix-sort-merge-join-filter-projection
Aug 24, 2026
Merged

kosiew merged 8 commits into
apache:mainfrom
haohuaijin:codex/fix-sort-merge-join-filter-projection

Conversation

@haohuaijin

@haohuaijin haohuaijin commented Aug 16, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Projection pushdown through SortMergeJoinExec could reuse stale join-filter indices and return incorrect results.

What changes are included in this PR?

  • Rewrite the JoinFilter during projection pushdown.
  • Reject the swap when a filter column cannot be mapped.
  • Add an SQLLogicTest for the incorrect-result case.

Are these changes tested?

Yes. The SQLLogicTest verifies the physical plan and the expected (1, 1) result. The full Rust lint and extended workspace test suites also pass.

Are there any user-facing changes?

Affected queries now return correct results. There are no API changes.

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Aug 16, 2026
@codecov-commenter

codecov-commenter commented Aug 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.39%. Comparing base (5134a1a) to head (14a2891).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24415      +/-   ##
==========================================
+ Coverage   81.38%   81.39%   +0.01%     
==========================================
  Files        1116     1116              
  Lines      397960   397973      +13     
  Branches   397960   397973      +13     
==========================================
+ Hits       323880   323935      +55     
+ Misses      55120    55075      -45     
- Partials    18960    18963       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kosiew kosiew 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.

@haohuaijin,

Thanks for working on this. The fix looks good to me. Remapping the join filter alongside the join keys addresses the stale column index issue, and the added unit and SQL regression coverage looks solid.

I left one optional suggestion for extending the SQL regression coverage.

@kosiew kosiew 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.

@haohuaijin,

Thanks for the follow-up. The latest change addresses the review comment nicely.

The new LEFT JOIN regression covers the important case where a potential match is rejected by the join filter, so the result correctly contains a null-extended right side even though the filter columns are projected away. The surrounding configuration uses prefer_hash_join = false, and the adjacent EXPLAIN already confirms this query shape goes through SortMergeJoinExec.

Looks good to me. Thanks for adding the focused regression!

@kosiew
kosiew added this pull request to the merge queue Aug 24, 2026
Merged via the queue into apache:main with commit e14bd12 Aug 24, 2026
41 checks passed
@haohuaijin
haohuaijin deleted the codex/fix-sort-merge-join-filter-projection branch August 24, 2026 11:45
@haohuaijin

Copy link
Copy Markdown
Contributor Author

Thanks @kosiew

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt) v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SortMergeJoin projection pushdown can reuse stale JoinFilter indices

4 participants