Skip to content

fix: avoid unsafe set rewrites for compound IN lists - #24952

Draft
bvolpato wants to merge 1 commit into
apache:mainfrom
bvolpato:bvolpato/fix-compound-in-list-rewrites
Draft

fix: avoid unsafe set rewrites for compound IN lists#24952
bvolpato wants to merge 1 commit into
apache:mainfrom
bvolpato:bvolpato/fix-compound-in-list-rewrites

Conversation

@bvolpato

@bvolpato bvolpato commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Related to #24258. This covers runtime operands, floating-point literals, and volatile expressions in compound IN-list rewrites. The NULL-semantics changes in that PR remain separate.

Rationale for this change

The optimizer performs set intersection and difference using expression equality. Different expressions can evaluate to the same value, so these rewrites can remove matching rows even when every column is non-nullable:

CREATE TABLE in_list_values(x INT NOT NULL, a INT NOT NULL, b INT NOT NULL)
AS VALUES (1, 1, 1);

SELECT x FROM in_list_values
WHERE x IN (a, 2, 10, 11) AND x IN (b, 5, 12, 13);

The query should return 1; the rewrite produces an empty intersection and returns no rows. Structural comparison also distinguishes positive and negative floating-point zero. Deduplicating volatile operands can change how many times they are evaluated.

What changes are included in this PR?

  • Restrict intersection and difference rewrites to non-null, non-floating literals.
  • Guard compound rewrites against volatile inputs and list items.
  • Preserve valid union rewrites for nonvolatile runtime expressions.
  • Add unit regressions for runtime values, signed zero, mixed integer coercion, and volatility, plus runtime-value cases in predicates.slt.

What is the testing strategy for this PR?

  • All three new unit tests fail against upstream production code at 35f58f53cde2f634c21f6370e385cf1d7e9bc55c and pass with the fix.
  • cargo test --locked -p datafusion-optimizer --lib: 777 passed.
  • cargo fmt --all -- --check and git diff --check passed.
  • Draft pending all-target/all-feature Clippy, the extended workspace suite, and execution of predicates.slt.

Are there any user-facing changes?

Affected compound IN predicates retain matching rows and preserve volatile expression evaluations. No public API changes.

Downsides

Queries excluded by the new guards may perform more comparisons because the optimizer retains their original predicates.

@github-actions github-actions Bot added optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) labels Sep 5, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.22989% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.61%. Comparing base (35f58f5) to head (230a82d).

Files with missing lines Patch % Lines
...imizer/src/simplify_expressions/expr_simplifier.rs 90.22% 4 Missing and 13 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #24952    +/-   ##
========================================
  Coverage   81.61%   81.61%            
========================================
  Files        1124     1124            
  Lines      411978   412151   +173     
  Branches   411978   412151   +173     
========================================
+ Hits       336236   336388   +152     
- Misses      55936    55941     +5     
- Partials    19806    19822    +16     

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

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

Labels

optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants