Cherylgabba opened a new pull request, #25290:
URL: https://github.com/apache/datafusion/pull/25290
Which issue does this PR close?
Closes #25253
Rationale for this change
parquet-rs < 53.1.0 (e.g. the version linked by DataFusion < 42.1.0) did not
record a null_count in row-group statistics when it was zero. Treating a
missing null_count as exactly zero is therefore a safe optimization for those
files — but it is wrong for every other writer that genuinely omits null
counts. Assuming zero for those files lets IS NULL / COUNT row-group pruning,
file statistics, limit pruning and sort pushdown return incorrect results.
What changes are included in this PR?
- Added missing_null_counts_are_zero(file_metadata) which parses the footer
created_by and returns true only for:
- parquet-rs with version < 53.1.0
- datafusion with version < 42.1.0
- everything else (other writers, unparsable/missing created_by) -> false
- statistics_from_parquet_metadata now passes that flag to
StatisticsConverter::with_missing_null_counts_as_zero, so file statistics (used
by aggregates, constant pruning and sort pushdown) treat missing counts as
Exact(0) only for legacy writers and stay Precision::Absent/Inexact otherwise.
- prune_by_statistics_with_metadata passes the footer FileMetaData down to
the row-group pruner so both the forward pruner and the fully-matched
(limit-pruning) pass use the same per-writer interpretation. The full
prune_by_statistics (no-footer, test-only) path keeps its original true default
for forward pruning but uses false for the fully-matched pass to stay sound.
- The runtime RowGroupPruner in push_decoder.rs derives its flag from the
footer created_by instead of hard-coding true.
- Added a 56.0.0 upgrade-guide note.
What is the testing strategy for this PR?
- New unit test missing_null_counts_are_zero in metadata.rs covering
parquet-rs/datafusion version boundaries, non-matching writers, unparsable
versions and absent created_by.
- New fixture-variant test
missing_null_counts_are_zero_only_for_writers_that_omitted_them in
statistics_order_tests.rs. It builds a 3-row-group file whose footer is
rewritten with created_by set to an old parquet-rs writer, an old datafusion
writer, and several other writers, with null_count stripped exactly as
parquet-rs < 53.1.0 wrote it. It asserts:
- file statistics are Exact(3)/Exact(0) for legacy writers,
Inexact(3)/Absent for others
- static row-group pruning skips groups 0 and 2 for IS NULL only for
legacy writers (returns [1]), keeping all groups for other writers
- the runtime pruner in push_decoder.rs matches the static pruning
- 3 matching rows are still found through every pruning level
- cargo test -p datafusion-datasource-parquet --lib: 257 passed, 0 failed
- cargo check/cargo clippy --all-targets --no-deps -- -D warnings: clean
- Note: the datafusion-core-parquet integration suites could not be built on
this Windows machine due to a pre-existing rustc 1.97
STATUS_STACK_BUFFER_OVERRUN crash while compiling the windows crate (also
affects the unmodified base commit).
Are there any user-facing changes?
Yes, an upgrade-guide note was added to
docs/source/library-user-guide/upgrading/56.0.0.md. Users of files written by
old parquet-rs/DataFusion writers gain back exact statistics, row-group pruning
and sort pushdown; files from other writers that omit null counts are now
handled conservatively instead of returning wrong results.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
