You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "ranflarion (via GitHub)" <gi...@apache.org> on 2026/10/06 11:53:53 UTC
[PR] fix: build the IN list static filter only from literal-only items [datafusion]
ranflarion opened a new pull request, #26083:
URL: https://github.com/apache/datafusion/pull/26083
## Which issue does this PR close?
- Closes #26082.
## Rationale for this change
`n IN (2, 100, 101, CASE WHEN d > 5 THEN 1 ELSE d END * 5)` returns wrong results on every row (repro in #26082). `try_evaluate_constant_list` decides that a list is constant by evaluating it on an empty batch, and `CaseExpr` returns its THEN branch as a scalar for a zero-length WHEN mask, so the CASE item is frozen to `5` in the static filter and never evaluated per row.
## What changes are included in this PR?
`try_evaluate_constant_list` returns `None` unless every leaf of every list item is a `Literal`, so any other list takes the per-batch evaluation `InListExpr` already uses for items that return arrays. Lists of literals, casts of literals and other expressions over literals still build the static filter from their empty-batch evaluation, as before.
Checking leaves rather than `collect_columns` keeps the rule independent of which leaf types read the input (`Column`, `LambdaVariable`, or a leaf defined outside DataFusion). The cost is that an item with a non-literal leaf that is nonetheless constant, such as a stable function the optimizer did not fold, is now evaluated per batch instead of once.
## What is the testing strategy for this PR?
A new section in `in_list.slt` runs the repro as a projection and as a filter, and `test_in_list_case_item_reading_a_column` builds the same expression through `in_list()`. Both fail on main and pass with this change. The other `in_list` unit tests, the `physical-expr` lib tests, the full sqllogictest suite and the `filter_pushdown` integration tests pass unchanged.
## Are there any user-facing changes?
No API changes. IN lists whose items have a non-literal leaf return correct results, evaluated per batch rather than through the static filter.
--
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: build the IN list static filter only from literal-only items [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #26083:
URL: https://github.com/apache/datafusion/pull/26083#discussion_r4220113970
##########
datafusion/physical-expr/src/expressions/in_list.rs:
##########
@@ -117,6 +119,13 @@ fn try_evaluate_constant_list(
list: &[Arc<dyn PhysicalExpr>],
schema: &Schema,
) -> Result<Option<ArrayRef>> {
+ // An item can read a column yet return a scalar on an empty batch (a CASE whose
+ // WHEN matches all zero rows returns its THEN), so require literal leaves.
+ for expr in list {
+ if expr.exists(|e| Ok(e.children().is_empty() && !e.is::<Literal>()))? {
Review Comment:
```suggestion
if expr.exists(|e| {
Ok(e.is_volatile_node() || (e.children().is_empty() && !e.is::<Literal>()))
})? {
```
--
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: build the IN list static filter only from literal-only items [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #26083:
URL: https://github.com/apache/datafusion/pull/26083
--
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: build the IN list static filter only from literal-only items [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #26083:
URL: https://github.com/apache/datafusion/pull/26083#issuecomment-6087988731
Thank you @ranflarion and @jayzhan211
--
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: build the IN list static filter only from literal-only items [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #26083:
URL: https://github.com/apache/datafusion/pull/26083#issuecomment-6085154182
FYI @geoffreyclaude
--
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: build the IN list static filter only from literal-only items [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #26083:
URL: https://github.com/apache/datafusion/pull/26083#issuecomment-6016113251
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/26083?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.83333%` with `7 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 82.71%. Comparing base ([`264ee3d`](https://app.codecov.io/gh/apache/datafusion/commit/264ee3dc674688081dffe13a2d26a7032d777319?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2fe3253`](https://app.codecov.io/gh/apache/datafusion/commit/2fe3253ab6ea6bd494ebe81a7bd1ead7b54e3f75?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/26083?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...atafusion/physical-expr/src/expressions/in\_list.rs](https://app.codecov.io/gh/apache/datafusion/pull/26083?src=pr&el=tree&filepath=datafusion%2Fphysical-expr%2Fsrc%2Fexpressions%2Fin_list.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9pbl9saXN0LnJz) | 70.83% | [1 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/26083?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 #26083 +/- ##
==========================================
- Coverage 82.72% 82.71% -0.01%
==========================================
Files 1147 1147
Lines 448013 448037 +24
Branches 448013 448037 +24
==========================================
+ Hits 370602 370612 +10
- Misses 54920 54924 +4
- Partials 22491 22501 +10
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/26083?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