diff --git a/datafusion/common/src/float_semantics.rs b/datafusion/common/src/float_semantics.rs index a6998143b726d..d1bfea9503765 100644 --- a/datafusion/common/src/float_semantics.rs +++ b/datafusion/common/src/float_semantics.rs @@ -21,19 +21,20 @@ //! i.e. IEEE 754 totalOrder. DataFusion deviates from that in one //! respect: `+0.0` and `-0.0` are equal, hash the same and fall into the same //! group. See [`normalize_float`], [`normalize_floats`] and -//! [`normalize_float_scalar`]. Ordering still follows totalOrder, so +//! [`ScalarValue::normalize`]. Ordering still follows totalOrder, so //! [`compare_floats`] sorts `-0.0` before `+0.0`. //! //! Every comparison, hash or grouping boundary should be built from these //! helpers rather than inspecting float bits itself: //! //! - Equality and grouping: normalize first ([`normalize_float`], -//! [`normalize_floats`], [`normalize_float_scalar`]), then compare bits or +//! [`normalize_floats`], [`ScalarValue::normalize`]), then compare bits or //! call Arrow's `eq`, `RowConverter` or `make_comparator`. Hash //! [`hash_input`]. //! - Ordering: use [`compare_floats`], [`float_lt`] or [`float_gt`]. Sorting //! does not normalize yet, so `-0.0` still sorts before `+0.0`. //! +//! [`ScalarValue::normalize`]: crate::ScalarValue::normalize use std::cmp::Ordering; use std::sync::Arc; @@ -44,8 +45,6 @@ use arrow::datatypes::{ }; use half::f16; -use crate::ScalarValue; - /// The floating point types the functions in this module are defined for: /// [`f16`](struct@f16), [`f32`] and [`f64`]. pub trait FloatSemantics: Copy { @@ -179,17 +178,6 @@ where Arc::new(arr.unary::<_, T>(normalize_float)) } -/// Applies [`normalize_float`] to a `Float16`, `Float32` or `Float64` scalar. -/// Other scalars are returned as is. -pub fn normalize_float_scalar(scalar: ScalarValue) -> ScalarValue { - match scalar { - ScalarValue::Float16(Some(v)) => ScalarValue::Float16(Some(normalize_float(v))), - ScalarValue::Float32(Some(v)) => ScalarValue::Float32(Some(normalize_float(v))), - ScalarValue::Float64(Some(v)) => ScalarValue::Float64(Some(normalize_float(v))), - other => other, - } -} - /// Whether `data_type` is a float or contains a float at any depth of its /// nested children. pub fn has_float_leaf(data_type: &DataType) -> bool { @@ -320,28 +308,4 @@ mod tests { let array: ArrayRef = Arc::new(list); assert!(Arc::ptr_eq(&normalize_floats(&array), &array)); } - - #[test] - fn normalize_float_scalar_folds_negative_zero() { - assert_eq!( - normalize_float_scalar(ScalarValue::Float64(Some(-0.0))), - ScalarValue::Float64(Some(0.0)) - ); - assert_eq!( - normalize_float_scalar(ScalarValue::Float32(Some(-0.0))), - ScalarValue::Float32(Some(0.0)) - ); - assert_eq!( - normalize_float_scalar(ScalarValue::Float16(Some(f16::NEG_ZERO))), - ScalarValue::Float16(Some(f16::ZERO)) - ); - assert_eq!( - normalize_float_scalar(ScalarValue::Float64(None)), - ScalarValue::Float64(None) - ); - assert_eq!( - normalize_float_scalar(ScalarValue::Int32(Some(0))), - ScalarValue::Int32(Some(0)) - ); - } } diff --git a/datafusion/common/src/scalar/mod.rs b/datafusion/common/src/scalar/mod.rs index ccac8154054c4..1712aed388fb0 100644 --- a/datafusion/common/src/scalar/mod.rs +++ b/datafusion/common/src/scalar/mod.rs @@ -2775,6 +2775,26 @@ impl ScalarValue { ) } + /// Applies [`normalize_float`] to a `Float16`, `Float32` or `Float64` + /// value, so `-0.0` becomes `0.0`. Other values are returned as is. + /// + /// [`normalize_float`]: crate::float_semantics::normalize_float + pub fn normalize(self) -> Self { + use crate::float_semantics::normalize_float; + match self { + ScalarValue::Float16(Some(v)) => { + ScalarValue::Float16(Some(normalize_float(v))) + } + ScalarValue::Float32(Some(v)) => { + ScalarValue::Float32(Some(normalize_float(v))) + } + ScalarValue::Float64(Some(v)) => { + ScalarValue::Float64(Some(normalize_float(v))) + } + other => other, + } + } + /// whether this value is null or not. pub fn is_null(&self) -> bool { match self { @@ -12069,4 +12089,28 @@ mod tests { assert_eq!(arr.null_count(), 0); assert_eq!(arr.num_columns(), 0); } + + #[test] + fn normalize_folds_negative_zero() { + assert_eq!( + ScalarValue::Float64(Some(-0.0)).normalize(), + ScalarValue::Float64(Some(0.0)) + ); + assert_eq!( + ScalarValue::Float32(Some(-0.0)).normalize(), + ScalarValue::Float32(Some(0.0)) + ); + assert_eq!( + ScalarValue::Float16(Some(f16::NEG_ZERO)).normalize(), + ScalarValue::Float16(Some(f16::ZERO)) + ); + assert_eq!( + ScalarValue::Float64(None).normalize(), + ScalarValue::Float64(None) + ); + assert_eq!( + ScalarValue::Int32(Some(0)).normalize(), + ScalarValue::Int32(Some(0)) + ); + } } diff --git a/datafusion/common/src/utils/mod.rs b/datafusion/common/src/utils/mod.rs index 7811f522ea121..6cde2de574324 100644 --- a/datafusion/common/src/utils/mod.rs +++ b/datafusion/common/src/utils/mod.rs @@ -1448,12 +1448,9 @@ pub use crate::float_semantics::has_float_leaf; /// Replace `-0.0` with `+0.0` in `Float16`, `Float32`, or `Float64` scalar /// values. -#[deprecated( - since = "56.0.0", - note = "use `datafusion_common::float_semantics::normalize_float_scalar` instead" -)] +#[deprecated(since = "56.0.0", note = "use `ScalarValue::normalize` instead")] pub fn normalize_float_zero_scalar(scalar: ScalarValue) -> ScalarValue { - crate::float_semantics::normalize_float_scalar(scalar) + scalar.normalize() } /// Apply a struct's nulls to one of its fields. diff --git a/datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs b/datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs index 7a2fab3010bb7..4c7679824aa16 100644 --- a/datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs +++ b/datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs @@ -28,7 +28,6 @@ //! `false` so that later rules can prune the plan they filter. use super::utils::{is_false, is_null}; -use datafusion_common::float_semantics::normalize_float_scalar; use datafusion_common::{Column, DataFusionError, Result, ScalarValue, internal_err}; use datafusion_expr::utils::{ conjunction, disjunction, split_binary_owned, split_conjunction_owned, @@ -422,7 +421,7 @@ fn op_and_literal(predicate: &Expr) -> Result<(Operator, &ScalarValue)> { fn normalize_literal(expr: &mut Expr) { if let Expr::Literal(value, _) = expr { let original = std::mem::replace(value, ScalarValue::Null); - *value = normalize_float_scalar(original); + *value = original.normalize(); } } diff --git a/datafusion/optimizer/src/simplify_expressions/utils.rs b/datafusion/optimizer/src/simplify_expressions/utils.rs index b0ae7a7f17b42..29d0081a906ef 100644 --- a/datafusion/optimizer/src/simplify_expressions/utils.rs +++ b/datafusion/optimizer/src/simplify_expressions/utils.rs @@ -18,7 +18,6 @@ //! Utility functions for expression simplification use datafusion_common::cse::NormalizeEq; -use datafusion_common::float_semantics::normalize_float_scalar; use datafusion_common::{Result, ScalarValue, internal_err}; use datafusion_expr::{ Case, Expr, Like, Operator, @@ -239,8 +238,7 @@ pub fn is_eq_and_ne_with_different_literal(eq_expr: &Expr, ne_expr: &Expr) -> bo Expr::Literal(right_value, right_metadata), ) => { left_metadata != right_metadata - || normalize_float_scalar(left_value.clone()) - != normalize_float_scalar(right_value.clone()) + || left_value.clone().normalize() != right_value.clone().normalize() } _ => left != right, } diff --git a/datafusion/physical-expr-common/src/datum.rs b/datafusion/physical-expr-common/src/datum.rs index 182eebf8802cf..249bca6f4f231 100644 --- a/datafusion/physical-expr-common/src/datum.rs +++ b/datafusion/physical-expr-common/src/datum.rs @@ -23,9 +23,7 @@ use arrow::compute::kernels::cmp::{ }; use arrow::compute::{SortOptions, ilike, like, nilike, nlike}; use arrow::error::ArrowError; -use datafusion_common::float_semantics::{ - has_float_leaf, normalize_float_scalar, normalize_floats, -}; +use datafusion_common::float_semantics::{has_float_leaf, normalize_floats}; use datafusion_common::{Result, ScalarValue}; use datafusion_common::{arrow_datafusion_err, assert_or_internal_err, internal_err}; use datafusion_expr_common::columnar_value::ColumnarValue; @@ -100,9 +98,7 @@ pub fn apply_cmp( fn normalize_cmp_input(cv: &ColumnarValue) -> ColumnarValue { match cv { ColumnarValue::Array(a) => ColumnarValue::Array(normalize_floats(a)), - ColumnarValue::Scalar(s) => { - ColumnarValue::Scalar(normalize_float_scalar(s.clone())) - } + ColumnarValue::Scalar(s) => ColumnarValue::Scalar(s.clone().normalize()), } } diff --git a/datafusion/physical-expr/src/intervals/cp_solver.rs b/datafusion/physical-expr/src/intervals/cp_solver.rs index 08d961f2860c6..a102c43259240 100644 --- a/datafusion/physical-expr/src/intervals/cp_solver.rs +++ b/datafusion/physical-expr/src/intervals/cp_solver.rs @@ -299,11 +299,10 @@ pub fn propagate_arithmetic( /// Compare two scalars the way runtime `Eq` does, instead of with /// `ScalarValue`'s bit-wise equality. `ScalarValue::PartialEq` is bit-wise for /// floats (`to_bits`), under which `-0.0 != +0.0`; runtime `Eq` normalizes -/// them via `normalize_float_scalar` (see +/// them via [`ScalarValue::normalize`] (see /// `physical-expr-common/src/datum.rs`). fn singleton_values_equal(left: &ScalarValue, right: &ScalarValue) -> bool { - use datafusion_common::float_semantics::normalize_float_scalar; - normalize_float_scalar(left.clone()) == normalize_float_scalar(right.clone()) + left.clone().normalize() == right.clone().normalize() } /// This function refines intervals `left_child` and `right_child` by applying