This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-3502-257edd593f52cdc85711e4bf23b375f0acf6f423 in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
commit aa9986fe83d4bdb0d883968b9a105c6e0a9ef745 Author: ump45nose <[email protected]> AuthorDate: Fri Oct 23 17:17:41 2026 +0000 fix(expr): treat NaN row group bounds as might-match for inequalities (#3502) * fix(expr): treat NaN row group bounds as might-match for inequalities * test(expr): cover sign-bit NaN row group bounds * test(expr): pin NaN min with a finite max for less-than --------- Co-authored-by: ump45nose <[email protected]> --- .../expr/visitors/row_group_metrics_evaluator.rs | 86 +++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/crates/iceberg/src/expr/visitors/row_group_metrics_evaluator.rs b/crates/iceberg/src/expr/visitors/row_group_metrics_evaluator.rs index 3a1170d4a..d07d922dd 100644 --- a/crates/iceberg/src/expr/visitors/row_group_metrics_evaluator.rs +++ b/crates/iceberg/src/expr/visitors/row_group_metrics_evaluator.rs @@ -177,6 +177,13 @@ impl<'a> RowGroupMetricsEvaluator<'a> { }?; if let Some(bound) = bound { + if bound.is_nan() { + // NaN indicates unreliable bounds (e.g. parquet-mr <= 1.9 records a + // NaN that starts a row group as both min and max). + // See the InclusiveMetricsEvaluator docs for more. + return ROW_GROUP_MIGHT_MATCH; + } + if cmp_fn(&bound, datum) { return ROW_GROUP_MIGHT_MATCH; } @@ -549,7 +556,7 @@ mod tests { use rand::Rng; use super::RowGroupMetricsEvaluator; - use crate::expr::{Bind, Reference}; + use crate::expr::{Bind, Predicate, Reference}; use crate::spec::{Datum, NestedField, PrimitiveType, Schema, Type}; use crate::{ErrorKind, Result}; @@ -1724,6 +1731,83 @@ mod tests { Ok(()) } + // A NaN with the sign bit set sorts below -Inf under `total_cmp`, so it + // exercises the `>` / `>=` side of the NaN check as well. + const NAN_BOUNDS: [f32; 2] = [f32::NAN, f32::from_bits(0xffc0_0000)]; + + fn eval_float_bounds(filter: &Predicate, min: f32, max: f32) -> Result<bool> { + let row_group_metadata = create_row_group_metadata( + 1, + 1, + Some(Statistics::float( + Some(min), + Some(max), + None, + Some(0), + false, + )), + 1, + None, + )?; + + let (iceberg_schema_ref, field_id_map) = build_iceberg_schema_and_field_map()?; + let filter = filter.clone().bind(iceberg_schema_ref.clone(), false)?; + + RowGroupMetricsEvaluator::eval( + &filter, + &row_group_metadata, + &field_id_map, + iceberg_schema_ref.as_ref(), + ) + } + + #[test] + fn eval_true_for_nan_lower_bound_less_than() -> Result<()> { + let filter = Reference::new("col_float").less_than(Datum::float(5.0_f32)); + for nan in NAN_BOUNDS { + assert!(eval_float_bounds(&filter, nan, nan)?); + } + Ok(()) + } + + #[test] + fn eval_true_for_nan_lower_bound_less_than_or_eq() -> Result<()> { + let filter = Reference::new("col_float").less_than_or_equal_to(Datum::float(5.0_f32)); + for nan in NAN_BOUNDS { + assert!(eval_float_bounds(&filter, nan, nan)?); + } + Ok(()) + } + + #[test] + fn eval_true_for_nan_lower_bound_finite_upper_bound_less_than() -> Result<()> { + let lt = Reference::new("col_float").less_than(Datum::float(5.0_f32)); + let lt_eq = Reference::new("col_float").less_than_or_equal_to(Datum::float(5.0_f32)); + for nan in NAN_BOUNDS { + assert!(eval_float_bounds(<, nan, 2.0)?); + assert!(eval_float_bounds(<_eq, nan, 2.0)?); + } + Ok(()) + } + + #[test] + fn eval_true_for_nan_upper_bound_greater_than() -> Result<()> { + let filter = Reference::new("col_float").greater_than(Datum::float(5.0_f32)); + for nan in NAN_BOUNDS { + assert!(eval_float_bounds(&filter, nan, nan)?); + } + Ok(()) + } + + #[test] + fn eval_true_for_nan_upper_bound_greater_than_or_eq() -> Result<()> { + let filter = Reference::new("col_float").greater_than_or_equal_to(Datum::float(5.0_f32)); + for nan in NAN_BOUNDS { + assert!(eval_float_bounds(&filter, nan, nan)?); + } + Ok(()) + } + #[test] fn eval_true_for_lower_bound_is_nan_filter_is_in() -> Result<()> { // TODO: should this be false, since the max stat
