Skip to content

fix: build the IN list static filter only from literal-only items - #26083

Merged
alamb merged 2 commits into
apache:mainfrom
ranflarion:fix-in-list-column-case-items
Oct 9, 2026
Merged

alamb merged 2 commits into
apache:mainfrom
ranflarion:fix-in-list-column-case-items

Conversation

@ranflarion

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

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.

@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Oct 6, 2026
@codecov-commenter

codecov-commenter commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.07692% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.77%. Comparing base (264ee3d) to head (8b38069).
⚠️ Report is 56 commits behind head on main.

Files with missing lines Patch % Lines
...atafusion/physical-expr/src/expressions/in_list.rs 73.07% 1 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26083      +/-   ##
==========================================
+ Coverage   82.72%   82.77%   +0.05%     
==========================================
  Files        1147     1147              
  Lines      448013   450930    +2917     
  Branches   448013   450930    +2917     
==========================================
+ Hits       370602   373252    +2650     
- Misses      54920    54947      +27     
- Partials    22491    22731     +240     

☔ 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.

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

Thanks @ranflarion , overall LGTM

Comment thread datafusion/physical-expr/src/expressions/in_list.rs Outdated
@alamb

alamb commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

FYI @geoffreyclaude

add is_volatile_node

Co-authored-by: Jay Zhan <jayzhan211@gmail.com>
@alamb
alamb added this pull request to the merge queue Oct 9, 2026
@alamb

alamb commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Thank you @ranflarion and @jayzhan211

Merged via the queue into apache:main with commit d137137 Oct 9, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong results from IN when a list item is a CASE that reads a column

4 participants