Skip to content

fix: prevent incorrect Parquet pruning with NaNs - #24804

Draft
sunchao wants to merge 1 commit into
apache:mainfrom
sunchao:dev/chao/codex/oss-parquet-nan-pruning-20260829
Draft

fix: prevent incorrect Parquet pruning with NaNs#24804
sunchao wants to merge 1 commit into
apache:mainfrom
sunchao:dev/chao/codex/oss-parquet-nan-pruning-20260829

Conversation

@sunchao

@sunchao sunchao commented Aug 30, 2026

Copy link
Copy Markdown
Member

Why are the changes needed?

DataFusion can return wrong results for floating-point Parquet columns: it can discard rows that match a predicate, or skip filtering rows that do not match.

The cause is incomplete bounds. Parquet excludes NaNs from floating-point min/max statistics, but DataFusion needs bounds over every non-null value to justify these optimizations.

For example, a non-null floating-point column in one row group can have these values and statistics:

Actual values:       [1.0, NaN]
Parquet statistics:  min = 1.0, max = 1.0, null_count = 0

Here, NaN is a positive-sign NaN. Under DataFusion/Arrow ordering it compares greater than finite values, and it is not NULL. Trusting the bounds can therefore lead to opposite kinds of incorrect conclusions:

Predicate Values that should match Unsafe conclusion from the metadata
x > 2.0 NaN The maximum is 1.0, so skip the row group.
x = 1.0 1.0 only The minimum and maximum are both 1.0, and there are no nulls, so every row matches and filtering is unnecessary.

In the first case, a downstream filter cannot recover the discarded NaN row. In the second, the reader can omit a filter that is still necessary. The Float16 query in #15812 reproduces the first kind of error.

Negative-sign NaNs can invalidate the lower bound too. The problem affects Float16/32/64 and IN/NOT IN as well as comparisons, including default list-size limits. Neither a NaN literal in the predicate nor a large IN list is required to trigger it.

What changes were proposed in this PR?

The fix is to treat floating-point Parquet min/max bounds as unknown in the file statistics exposed to optimizers and in row-group/page pruning. DataFusion's existing conservative behavior then applies: keep data unless trustworthy statistics prove it can be skipped, and evaluate the predicate against the actual values when necessary.

This is handled in the existing statistics-validity guard, so readers and optimizers share the same interpretation of these bounds. That protects both decisions in the example: skipping data and declaring it fully matched. It also prevents other invalid deductions from the inferred bounds, such as replacing a column containing [1.0, NaN] with the constant 1.0. A predicate-specific exception would not address all of these uses of the same incomplete information.

The change affects which metadata can justify an optimization, not floating-point comparison semantics. Exact filtering, null counts, and usable non-floating bounds retain their existing behavior.

This is deliberately conservative: the current reader's metadata does not prove that NaNs are absent, so NaN-free floating-point files lose these min/max optimizations too. Preserving those opportunities would require additional trustworthy information or a more precise statistics contract; finite bounds alone are insufficient.

How was this PR tested?

The regressions write real Parquet files with signed NaN payloads and compare pruning results with unpruned predicate evaluation. Each pruning level is tested independently, including fully-matched classifications, so one working guard cannot mask a broken path. They also verify that null-count and integer/string pruning remain available.

Validation Result
Three new Rust regressions Fail without the production fix; pass with it
Parquet crate unit tests 253 passed
Extended workspace Rust tests 10,983 passed; 8 ignored
SQL logic tests, including the original Float16 reproducer All 505 files passed
Formatting, strict all-target/all-feature Clippy, and the standard lint suite Passed
Validation environment and benchmark notes

Validation used Linux, Rust 1.97.0, locked Arrow/Parquet 59.2.0 dependencies, and base 61bf6b96cc07d6a0518014dec7fe86fbef76576e. Extended tests enabled avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption. The successful run used a process-local file-descriptor limit of 65,536 after an unrelated sort fuzz test exhausted the host's default of 1,024. Lint used the upstream-pinned HawkEye 7.0.0.

All 27 parquet_metadata_statistics benchmark cases ran before and after the fix in the unoptimized CI profile. Results included slower no-statistics controls; these short measurements do not establish query performance or absence of regressions. The query-performance cost of losing floating-point min/max optimizations remains unmeasured.

Which issue does this PR close?

Closes #15812.

Are there any user-facing changes?

Queries no longer use NaN-incomplete Parquet bounds to discard matching data or skip necessary filtering. Floating-point workloads may scan more data and lose some statistics-based optimizations, including metadata-only MIN/MAX evaluation and constant-column substitution; selectivity estimates may also become less precise. There are no public API or configuration changes.

@github-actions github-actions Bot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) datasource Changes to the datasource crate labels Aug 30, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.52%. Comparing base (61bf6b9) to head (7cc3c0e).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #24804    +/-   ##
========================================
  Coverage   81.52%   81.52%            
========================================
  Files        1123     1123            
  Lines      405970   406151   +181     
  Branches   405970   406151   +181     
========================================
+ Hits       330978   331132   +154     
- Misses      55627    55652    +25     
- Partials    19365    19367     +2     

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

@sunchao
sunchao marked this pull request as draft August 31, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate datasource Changes to the datasource crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pruning of floating point Parquet columns is incorrect when NaN is present

2 participants