You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "haohuaijin (via GitHub)" <gi...@apache.org> on 2026/08/16 13:53:54 UTC
[PR] fix: remap SortMergeJoin filter during projection pushdown [datafusion]
haohuaijin opened a new pull request, #24415:
URL: https://github.com/apache/datafusion/pull/24415
## Which issue does this PR close?
- Closes #24414.
## 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: remap SortMergeJoin filter during projection pushdown [datafusion]
Posted by "haohuaijin (via GitHub)" <gi...@apache.org>.
haohuaijin commented on PR #24415:
URL: https://github.com/apache/datafusion/pull/24415#issuecomment-5394732738
Thanks @kosiew
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: remap SortMergeJoin filter during projection pushdown [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew merged PR #24415:
URL: https://github.com/apache/datafusion/pull/24415
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: remap SortMergeJoin filter during projection pushdown [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #24415:
URL: https://github.com/apache/datafusion/pull/24415#discussion_r3835932054
##########
datafusion/sqllogictest/test_files/joins.slt:
##########
@@ -2858,6 +2858,44 @@ NULL 1970-01-04T00:00:00 789 ghi 1970-01-04 NULL 789 qwe
NULL NULL NULL NULL NULL 1970-01-04T00:00:00 0 qwerty
NULL NULL NULL NULL NULL NULL 100000 abcdefg
+# Regression test: projection pushdown through SortMergeJoinExec must not reuse a
+# JoinFilter after its input columns have been projected away.
+statement ok
+set datafusion.optimizer.repartition_joins = true;
+
+query TT
Review Comment:
Could we also add a `LEFT` or `FULL` join case with an unmatched row and a join filter that references projected-away columns? The current regression test covers the inner join case well. An outer join case would also exercise the remapping when null-extended rows are involved. This is just a suggestion and does not need to block this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: remap SortMergeJoin filter during projection pushdown [datafusion]
Posted by "haohuaijin (via GitHub)" <gi...@apache.org>.
haohuaijin commented on code in PR #24415:
URL: https://github.com/apache/datafusion/pull/24415#discussion_r3836384991
##########
datafusion/sqllogictest/test_files/joins.slt:
##########
@@ -2858,6 +2858,44 @@ NULL 1970-01-04T00:00:00 789 ghi 1970-01-04 NULL 789 qwe
NULL NULL NULL NULL NULL 1970-01-04T00:00:00 0 qwerty
NULL NULL NULL NULL NULL NULL 100000 abcdefg
+# Regression test: projection pushdown through SortMergeJoinExec must not reuse a
+# JoinFilter after its input columns have been projected away.
+statement ok
+set datafusion.optimizer.repartition_joins = true;
+
+query TT
Review Comment:
added in [b5c7a48](https://github.com/apache/datafusion/pull/24415/commits/b5c7a4872a8e111982506c531b6f1f34ebd6b6f3)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: remap SortMergeJoin filter during projection pushdown [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24415:
URL: https://github.com/apache/datafusion/pull/24415#issuecomment-5307927019
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/24415?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `70.58824%` with `5 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.23%. Comparing base ([`aff5850`](https://app.codecov.io/gh/apache/datafusion/commit/aff5850c0aa604bb03158a137d5343e4efe73ae3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8ffe628`](https://app.codecov.io/gh/apache/datafusion/commit/8ffe628304c1c4e71ef7daf80668a3c1919695b0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/24415?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...on/physical-plan/src/joins/sort\_merge\_join/exec.rs](https://app.codecov.io/gh/apache/datafusion/pull/24415?src=pr&el=tree&filepath=datafusion%2Fphysical-plan%2Fsrc%2Fjoins%2Fsort_merge_join%2Fexec.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1wbGFuL3NyYy9qb2lucy9zb3J0X21lcmdlX2pvaW4vZXhlYy5ycw==) | 70.58% | [3 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24415?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #24415 +/- ##
==========================================
- Coverage 81.23% 81.23% -0.01%
==========================================
Files 1111 1111
Lines 390208 390221 +13
Branches 390208 390221 +13
==========================================
- Hits 316990 316988 -2
- Misses 54591 54601 +10
- Partials 18627 18632 +5
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/24415?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org