Skip to content

GH-3574: Keep null_count when min/max stats exceed the size limit - #3737

Closed
arimu1 wants to merge 1 commit into
apache:masterfrom
arimu1:fix/3574-null-count-when-stats-truncated
Closed

GH-3574: Keep null_count when min/max stats exceed the size limit#3737
arimu1 wants to merge 1 commit into
apache:masterfrom
arimu1:fix/3574-null-count-when-stats-truncated

Conversation

@arimu1

@arimu1 arimu1 commented Aug 22, 2026

Copy link
Copy Markdown

Rationale for this change

When column statistics exceed the max size for truncation, toParquetStatistics currently omits the entire Statistics payload, including null_count. Dropping min/max is reasonable (there is no way to mark a truncated bound as a lower/upper bound rather than a true min/max). null_count and nan_count are tiny counters and should still be written.

Missing null_count causes downstream readers to treat the column as if it might contain nulls. Snowflake reports:

non-nullable column without default has null values according to file statistics

parquet-format recommends always writing null_count, even when it is zero.

This re-lands the parquet-java side of #3575 after it was reverted in #3688 to unblock a release. The format contract is that min/max and null counts are independent: Statistics.hasNonNullValue() vs Statistics.isNumNullsSet(). Consumers that assumed "all stats or no stats" should treat a missing min/max independently of a present null_count. Iceberg already observes num_nulls: N, min/max not defined on older files.

What changes are included in this PR?

In ParquetMetadataConverter.toParquetStatistics, write null_count (and nan_count when set) whenever stats are non-empty, even if min/max are omitted because they exceed MAX_STATS_SIZE. Min/max are still not written in that case (not truncated to an unmarked bound).

Are these changes tested?

Yes. TestParquetMetadataConverter.testBinaryStatsV1 / testBinaryStatsV2 now assert that oversized binary min/max are omitted while null_count is still written and round-trips.

./mvnw -pl parquet-hadoop -am test -Dtest=TestParquetMetadataConverter -Dsurefire.failIfNoSpecifiedTests=false

TestParquetMetadataConverter: 72 tests, 0 failures (Temurin 21, macOS aarch64).

On current master without this change, the same assertions fail (null_count expected 3004, actual 0).

Are there any user-facing changes?

Yes: files with oversized column min/max now still carry null_count (and nan_count when set) in column-chunk statistics. Readers that previously assumed the statistics object was all-or-nothing should use hasNonNullValue() / isSetMin_value() for bounds and isNumNullsSet() / isSetNull_count() for nulls.

Closes #3574

Omit oversized min/max statistics, but still write null_count and
nan_count. parquet-format recommends always writing null_count even
when it is zero.
@arimu1

arimu1 commented Aug 22, 2026

Copy link
Copy Markdown
Author

Closing: this re-lands #3575, which was reverted in #3688 because Iceberg ParquetMetrics still treats Statistics.isEmpty() as all-or-nothing. Mixed row groups (small strings with min/max + large strings over MAX_STATS_SIZE) would then publish incomplete file-level bounds. Iceberg needs to keep null counts iff every chunk isNumNullsSet() and drop file-level bounds when a chunk has values but !hasNonNullValue(), before this parquet-java split is safe.

Sorry for the noise — #3574 remains valid once that consumer contract is evolved.

@arimu1 arimu1 closed this Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

null_count is omitted for large columns in parquet files

1 participant