Skip to content

Add configuration option to StatisticsConverter to control interpretation of missing null counts in Parquet statistics - #6485

Merged
alamb merged 5 commits into
apache:masterfrom
etseidl:null_counts_read_behavior
Oct 1, 2024
Merged

alamb merged 5 commits into
apache:masterfrom
etseidl:null_counts_read_behavior

Conversation

@etseidl

@etseidl etseidl commented Sep 30, 2024

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #6256.

Rationale for this change

In #6257 @alamb changes the behavior of the parquet writer to always write Some(0) when the null count is known to be zero. Part of #6257 allows users of StatisticsConverter to choose whether or not to treat missing null counts as 0.

What changes are included in this PR?

This PR consists of @alamb's changes to StatisticsConverter without the changes to writer behavior. By default no behavior is changed, but users can prepare for later changes to how the Statistics::null_count field is written.
See the conversation here for the rationale.

Are there any user-facing changes?

No.

@github-actions github-actions Bot added the parquet Changes to the parquet crate label Sep 30, 2024
@etseidl

etseidl commented Sep 30, 2024

Copy link
Copy Markdown
Contributor Author

cc @alamb @Michael-J-Ward

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

Thank you very much @etseidl

.run();
}

#[test]

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.

❤️

@alamb
alamb merged commit 2d67c4c into apache:master Oct 1, 2024
@etseidl
etseidl deleted the null_counts_read_behavior branch October 1, 2024 20:49
peterxcli added a commit to peterxcli/datafusion that referenced this pull request Sep 14, 2026
## Which issue does this PR close?

Closes apache#25239. Follows up on apache#21907 and [Arrow
apache#6485](apache/arrow-rs#6485).

## Rationale for this change

An absent Parquet `null_count` is unknown. Treating it as zero can
discard NULL rows during `NULLS FIRST` TopK and `IS NULL` pruning, and
let metadata-only `COUNT(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 NULL` pruning, metadata-only `COUNT(column)`, and
`NULLS FIRST` TopK pruning. Sort pushdown can also retain a full
`SortExec` for `ORDER BY` on 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.

---------

Signed-off-by: peterxcli <peterxcli@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parquet Changes to the parquet crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants