Repository navigation
fix: preserve missing Parquet null counts - #25242
Conversation
Signed-off-by: peterxcli <peterxcli@gmail.com>
jayzhan211
left a comment
There was a problem hiding this comment.
Thanks @peterxcli , there are 2 concerns but they are non-blocking and I will file an issue to track it
1. The slowdown reaches sort pushdown, not only pruning and COUNT. any_file_has_nulls_in_sort_columns (datafusion/datasource/src/file_scan_config/sort_pushdown.rs:196 and :277) requires Exact(0) nulls in every file before it upgrades to Exact. For affected files, ORDER BY on a nullable column can bring back a full SortExec even when the files were written sorted and non-overlapping. That will probably be the most visible change, so please list it under "Are there any user-facing changes?".
2. Please add an upgrade guide entry. The affected files come mostly from parquet-rs < 53.1.0, which left out null_count when it was zero (apache/arrow-rs#6490). That includes every file written by DataFusion < 42.1.0. Users with long-lived data will see plans get slower after upgrading with nothing explaining why. Suggested text for docs/source/library-user-guide/upgrading/56.0.0.md:
### Missing Parquet null counts are treated as unknown
A Parquet column chunk whose statistics leave out `null_count` was previously
treated as having zero nulls. This could return wrong results: `IS NULL`
filters and `ORDER BY ... NULLS FIRST LIMIT` queries could skip rows containing
NULLs, and `COUNT(column)` answered from metadata could be too high. A missing
null count is now treated as unknown.
Results are now correct, but files that leave out null counts can no longer use
optimizations that need a known null count:
- `IS NULL` row-group pruning
- answering `COUNT(column)` from file metadata
- dynamic row-group pruning for `ORDER BY ... NULLS FIRST LIMIT`
- removing the sort for `ORDER BY` on nullable columns of files already
written in sorted order
Files written by parquet-rs before 53.1.0, including files written by
DataFusion before 42.1.0, leave out the null count whenever it is zero and are
affected. Files written by current versions of parquet-rs, parquet-java
(parquet-mr) and Arrow C++ record zero null counts and are not affected.
To check a file, look for column chunks that have min/max statistics but no
null count:
```sql
SELECT row_group_id, path_in_schema
FROM parquet_metadata('data.parquet')
WHERE stats_min IS NOT NULL AND stats_null_count IS NULL;
```
Rewriting affected files with a current DataFusion version (for example with
`COPY ... TO`) restores these optimizations.Signed-off-by: peterxcli <peterxcli@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #25242 +/- ##
==========================================
- Coverage 81.95% 81.88% -0.07%
==========================================
Files 1133 1133
Lines 423799 424576 +777
Branches 423799 424576 +777
==========================================
+ Hits 347307 347675 +368
- Misses 55899 56289 +390
- Partials 20593 20612 +19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks @jayzhan211 @xudong963 @asolimando |
Which issue does this PR close?
Closes #25239. Follows up on #21907 and Arrow #6485.
Rationale for this change
An absent Parquet
null_countis unknown. Treating it as zero can discard NULL rows duringNULLS FIRSTTopK andIS NULLpruning, and let metadata-onlyCOUNT(column)return an incorrect result.What changes are included in this PR?
Preserve missing null counts in the shared row-group statistics adapter and file statistics. Remove the per-caller compatibility flag so static pruning, runtime pruning, and fully matched proofs use the same interpretation. Add an upgrade guide covering affected writers, plan changes, and migration.
What is the testing strategy for this PR?
Regressions cover missing and explicit-zero counts, static pruning, file-statistics precision,
COUNT, and TopK with ASC/DESC, NULLS FIRST/LAST, and dynamic pruning enabled/disabled. The affected cases fail without the fix. Extended workspace tests, strict all-target/all-feature Clippy, and the full lint suite pass.Are there any user-facing changes?
Correct results for Parquet files with missing null counts. Affected files can lose
IS NULLpruning, metadata-onlyCOUNT(column), andNULLS FIRSTTopK pruning. Sort pushdown can also retain a fullSortExecforORDER BYon nullable columns, even when files are sorted and non-overlapping.This includes older files written with parquet-rs before 53.1.0, used by DataFusion releases before 42.1.0. The DataFusion 56 upgrade guide explains how to identify and rewrite affected files. Explicit counts retain their existing behavior. No public API changes.