arimu1 opened a new pull request, #3737:
URL: https://github.com/apache/parquet-java/pull/3737

   ### 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](https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift)
 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


-- 
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]

Reply via email to