andygrove opened a new issue, #3620: URL: https://github.com/apache/iceberg-rust/issues/3620
### Apache Iceberg Rust version main with #3257 (Bump to Arrow 60) applied ### Describe the bug With Parquet 60, `ParquetWriter` records NaN as the lower and upper bound of a float or double column whose values in the file are all NaN. The [spec](https://iceberg.apache.org/spec/#manifests) does not allow NaN as a lower or upper bound, and iceberg-java records no bound for such a column. On main, which is on Parquet 59, it does not happen. It shows up with the Arrow 60 bump in #3257. ### Cause Parquet 60 implements the `IEEE_754_TOTAL_ORDER` column order, under which a NaN min and max mean the chunk holds only NaN. Its writer returns the NaN min and max for an all-NaN chunk (`fallback_min_max` in `parquet/src/column/writer/min_max.rs`), where Parquet 59 wrote none. So this is intended Parquet behavior, not a parquet-rs bug. `MinMaxColAggregator::update` in `crates/iceberg/src/writer/file_writer/parquet_writer.rs` copies the min and max statistics into the bounds without filtering out NaN, so the NaN reaches the `DataFile`. NaN is also unordered in `update_state_min` / `update_state_max`, so once a NaN is stored as a bound, a later chunk with real values may not replace it. ### To Reproduce Add this test next to `test_nan_val_cnts_primitive_type` on a branch with #3257: ```rust #[tokio::test] async fn test_all_nan_column_has_no_bounds() -> Result<()> { let temp_dir = TempDir::new().unwrap(); let file_io = FileIO::new_with_fs(); let location_gen = DefaultLocationGenerator::with_data_location( temp_dir.path().to_str().unwrap().to_string(), ); let file_name_gen = DefaultFileNameGenerator::new("test".to_string(), None, DataFileFormat::Parquet); let arrow_schema = Arc::new(arrow_schema::Schema::new(vec![ Field::new("col", DataType::Float32, false).with_metadata(HashMap::from([( PARQUET_FIELD_ID_META_KEY.to_string(), "0".to_string(), )])), Field::new("col2", DataType::Float64, false).with_metadata(HashMap::from([( PARQUET_FIELD_ID_META_KEY.to_string(), "1".to_string(), )])), ])); let float_32_col = Arc::new(Float32Array::from(vec![f32::NAN, f32::NAN])) as ArrayRef; let float_64_col = Arc::new(Float64Array::from(vec![f64::NAN, f64::NAN])) as ArrayRef; let to_write = RecordBatch::try_new(arrow_schema.clone(), vec![float_32_col, float_64_col]).unwrap(); let output_file = file_io.new_output( location_gen.generate_location(None, &file_name_gen.generate_file_name()), )?; let mut pw = ParquetWriterBuilder::new( WriterProperties::builder().build(), Arc::new(to_write.schema().as_ref().try_into().unwrap()), ) .build(output_file) .await?; pw.write(&to_write).await?; let data_file = pw .close() .await? .into_iter() .next() .unwrap() .content(DataContentType::Data) .partition(Struct::empty()) .partition_spec_id(0) .build() .unwrap(); assert_eq!(*data_file.nan_value_counts(), HashMap::from([(0, 2), (1, 2)])); assert!(data_file.lower_bounds().is_empty()); assert!(data_file.upper_bounds().is_empty()); Ok(()) } ``` It fails with: ``` lower_bounds: {0: Datum { type: Float, literal: Float(NaN) }, 1: Datum { type: Double, literal: Double(NaN) }} upper_bounds: {0: Datum { type: Float, literal: Float(NaN) }, 1: Datum { type: Double, literal: Double(NaN) }} assertion failed: data_file.lower_bounds().is_empty() ``` ### Expected behavior No lower or upper bound for a float or double column whose values are all NaN, as in iceberg-java. A file that mixes NaN with other values gets its bounds from the non-NaN values. ### Additional context Suggested fix: in `MinMaxColAggregator::update`, skip a min or max statistic that is NaN. It probably belongs with the Arrow 60 bump, since that is where the behavior changes. Found while upgrading DataFusion Comet to Arrow 60 (apache/datafusion-comet#7242). ### 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]
