andygrove opened a new issue, #7242: URL: https://github.com/apache/datafusion-comet/issues/7242
### Describe the bug When Comet runs on Parquet 60 or later, the native Iceberg writer records NaN as the lower and upper bound of a float or double column in a data file's manifest entry whenever one of the file's column chunks holds only NaN values. The Iceberg spec says [NaNs are not permitted as lower or upper bounds](https://iceberg.apache.org/spec/#manifests), and iceberg-java records no bound for such a column. `CometIcebergWriteActionSuite` "native acceleration: NaN float/double manifest metrics match the JVM writer" shows it. The native writer produces `[1,0.0,NaN,2,0.25,NaN,3,NaN,NaN]` where the JVM writer produces `[1,0.0,2.5,2,0.25,1.5,3,null,null]`. This does not happen on current `main`, which is on Parquet 59. It first appears with the DataFusion 56 upgrade (#6410, draft PR #6404), which moves to Parquet 60. ### Cause Parquet 60 writes NaN min and max statistics for a column chunk whose values are all NaN (`get_min_max` in `parquet/src/column/writer/encoder.rs`). Parquet 59 wrote none. iceberg-rust's `MinMaxColAggregator::update` copies the Parquet statistics into the file's lower and upper bounds without filtering out NaN, so the NaN reaches the manifest. NaN is also unordered in the aggregator's comparisons, so once a NaN is stored as a bound it can stay there whatever later chunks hold. ### Steps to reproduce Write a table with the native Iceberg writer on a build that uses Parquet 60, with a float or double column where every value in a data file is NaN, then read the data file's `lower_bounds` and `upper_bounds` from the manifest (for example through the `files` metadata table). The test above does exactly this. ### Expected behavior The manifest entry has no lower or upper bound for a column chunk that holds only NaN, matching iceberg-java, and a file that mixes NaN with other values gets bounds from the non-NaN values. ### Additional context - The fix belongs in iceberg-rust: `MinMaxColAggregator::update` should skip NaN statistics. It cannot be fixed in Comet without post-processing the data files iceberg-rust returns. - The impact is manifests that do not match the JVM writer's and break the spec's rule on bounds. I have not checked every reader. A NaN bound is looser than a real one, so pruning on it should not drop rows. - Until the fix lands, the test is ignored on the DataFusion 56 branch and the limitation is listed in the compatibility guide and the 1.3.0 upgrade guide. ### Willingness to contribute I can contribute a fix for this bug independently -- 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]
