Skip to content

Commit e76a2ba

Browse files
mkleenmkleen
committed
refactor: align float_semantics naming with Comet
Replace the `CanonicalFloat` trait methods with free functions named after datafusion-comet's `float_semantics` module: `normalize_float`, `compare_floats`, `float_lt`, `float_gt` and `hash_input`, plus `normalize_floats` and `normalize_float_scalar` for arrays and scalars. The trait, now `FloatSemantics`, remains as the bound for f16/f32/f64. Rename `HashValue::canonicalize` to `normalize_key` so it does not clash with `FloatSemantics::normalize`, and correct the module docs: DataFusion's hashing already normalizes `-0.0`, and ordering still follows totalOrder. No behavior change.
1 parent 6cdd45d commit e76a2ba

21 files changed

Lines changed: 205 additions & 201 deletions

File tree

‎datafusion/common/src/float_semantics.rs‎

Lines changed: 135 additions & 130 deletions
Large diffs are not rendered by default.

‎datafusion/common/src/hash_utils.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ use crate::cast::{
8989
};
9090
use crate::error::Result;
9191
use crate::error::{_internal_datafusion_err, _internal_err};
92-
use crate::float_semantics::CanonicalFloat;
92+
use crate::float_semantics::hash_input;
9393
#[cfg(not(feature = "force_hash_collisions"))]
9494
use crate::utils::offset_span;
9595
use std::cell::RefCell;
@@ -261,13 +261,13 @@ hash_value!(bool, str, [u8], IntervalDayTime, IntervalMonthDayNano);
261261
macro_rules! hash_float_value {
262262
($($t:ty),+) => {
263263
$(impl HashValue for $t {
264-
// Hash the canonical bits so values SQL considers equal
264+
// Hash the normalized bits so values SQL considers equal
265265
// (e.g. +0.0 and -0.0) hash the same.
266266
fn hash_one<S: BuildHasher>(&self, state: &S) -> u64 {
267-
state.hash_one(self.canonical_bits())
267+
state.hash_one(hash_input(*self).to_bits())
268268
}
269269
fn hash_write(&self, hasher: &mut impl Hasher) {
270-
hasher.write(&self.canonical_bits().to_ne_bytes())
270+
hasher.write(&hash_input(*self).to_bits().to_ne_bytes())
271271
}
272272
})+
273273
};

‎datafusion/common/src/hash_utils/build_hasher.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use crate::cast::{
3232
use crate::error::Result;
3333
use crate::error::{_internal_datafusion_err, _internal_err};
3434
#[cfg(not(feature = "force_hash_collisions"))]
35-
use crate::float_semantics::CanonicalFloat;
35+
use crate::float_semantics::hash_input;
3636
#[cfg(feature = "force_hash_collisions")]
3737
use arrow::array::Array;
3838
#[cfg(not(feature = "force_hash_collisions"))]
@@ -174,7 +174,7 @@ macro_rules! build_hasher_hash_float_value {
174174
$(#[cfg(not(feature = "force_hash_collisions"))]
175175
impl BuildHasherHashValue for $t {
176176
fn hash_one_with_hasher<S: BuildHasher>(&self, state: &S) -> u64 {
177-
state.hash_one(self.canonical_bits())
177+
state.hash_one(hash_input(*self).to_bits())
178178
}
179179
})+
180180
};

‎datafusion/common/src/utils/mod.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1444,10 +1444,10 @@ fn fsl_values_row_number(list_size: i32, array_len: usize) -> Result<Int32Array>
14441444
/// Replace `-0.0` with `+0.0` in any `Float16`, `Float32`, or `Float64` array.
14451445
#[deprecated(
14461446
since = "56.0.0",
1447-
note = "use `datafusion_common::float_semantics::canonicalize_float_array` instead"
1447+
note = "use `datafusion_common::float_semantics::normalize_floats` instead"
14481448
)]
14491449
pub fn normalize_float_zero(array: &ArrayRef) -> ArrayRef {
1450-
crate::float_semantics::canonicalize_float_array(array)
1450+
crate::float_semantics::normalize_floats(array)
14511451
}
14521452

14531453
pub use crate::float_semantics::has_float_leaf;
@@ -1456,10 +1456,10 @@ pub use crate::float_semantics::has_float_leaf;
14561456
/// values.
14571457
#[deprecated(
14581458
since = "56.0.0",
1459-
note = "use `datafusion_common::float_semantics::canonicalize_float_scalar` instead"
1459+
note = "use `datafusion_common::float_semantics::normalize_float_scalar` instead"
14601460
)]
14611461
pub fn normalize_float_zero_scalar(scalar: ScalarValue) -> ScalarValue {
1462-
crate::float_semantics::canonicalize_float_scalar(scalar)
1462+
crate::float_semantics::normalize_float_scalar(scalar)
14631463
}
14641464

14651465
/// Apply a struct's nulls to one of its fields.

‎datafusion/functions-aggregate-common/src/min_max.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use arrow::array::{
3131
};
3232
use arrow::compute;
3333
use arrow::datatypes::{DataType, IntervalUnit, TimeUnit};
34-
use datafusion_common::float_semantics::CanonicalFloat;
34+
use datafusion_common::float_semantics::{FloatSemantics, compare_floats};
3535
use datafusion_common::{
3636
DataFusionError, Result, ScalarValue, downcast_value, internal_err,
3737
};
@@ -65,7 +65,7 @@ fn min_max_option<T: Clone + Ord>(
6565
}
6666
}
6767

68-
fn min_max_float_option<T: CanonicalFloat>(
68+
fn min_max_float_option<T: FloatSemantics>(
6969
lhs: Option<&T>,
7070
rhs: Option<&T>,
7171
ordering: Ordering,
@@ -74,7 +74,7 @@ fn min_max_float_option<T: CanonicalFloat>(
7474
(None, None) => None,
7575
(Some(a), None) => Some(*a),
7676
(None, Some(b)) => Some(*b),
77-
(Some(a), Some(b)) if a.compare_floats(*b) == ordering => Some(*b),
77+
(Some(a), Some(b)) if compare_floats(*a, *b) == ordering => Some(*b),
7878
(Some(a), Some(_)) => Some(*a),
7979
}
8080
}

‎datafusion/functions-aggregate/src/min_max.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ mod min_max_struct;
2323

2424
use arrow::array::{ArrayRef, ArrowPrimitiveType, downcast_primitive};
2525
use arrow::datatypes::{DataType, Float16Type, Float32Type, Float64Type};
26-
use datafusion_common::float_semantics::CanonicalFloat;
26+
use datafusion_common::float_semantics::{float_gt, float_lt};
2727
use datafusion_common::stats::Precision;
2828
use datafusion_common::{ColumnStatistics, Result, exec_err, internal_err};
2929
use datafusion_functions_aggregate_common::aggregate::groups_accumulator::prim_op::PrimitiveGroupsAccumulator;
@@ -125,7 +125,7 @@ macro_rules! primitive_max_accumulator {
125125
($t:ty, $data_type:ident, total, $bits:ident) => {{
126126
Ok(Box::new(
127127
PrimitiveGroupsAccumulator::<$t, _>::new($data_type, |cur, new| {
128-
if new.float_gt(*cur) {
128+
if float_gt(new, *cur) {
129129
*cur = new
130130
}
131131
})
@@ -161,7 +161,7 @@ macro_rules! primitive_min_accumulator {
161161
($t:ty, $data_type:ident, total, $bits:ident) => {{
162162
Ok(Box::new(
163163
PrimitiveGroupsAccumulator::<$t, _>::new(&$data_type, |cur, new| {
164-
if new.float_lt(*cur) {
164+
if float_lt(new, *cur) {
165165
*cur = new
166166
}
167167
})

‎datafusion/functions-nested/src/set_ops.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use arrow::datatypes::DataType::{LargeList, List, Null};
2828
use arrow::datatypes::{DataType, Field, FieldRef};
2929
use arrow::row::{RowConverter, SortField};
3030
use datafusion_common::cast::{as_large_list_array, as_list_array};
31-
use datafusion_common::float_semantics::canonicalize_float_array;
31+
use datafusion_common::float_semantics::normalize_floats;
3232
use datafusion_common::utils::{ListCoercion, offset_span};
3333
use datafusion_common::{
3434
Result, assert_eq_or_internal_err, exec_err, internal_err, utils::take_function_args,
@@ -335,9 +335,9 @@ pub(crate) fn normalize_visible_values<OffsetSize: OffsetSizeTrait>(
335335
) -> ArrayRef {
336336
let (first, len) = offset_span(array.offsets());
337337
if first == 0 && len == array.values().len() {
338-
canonicalize_float_array(array.values())
338+
normalize_floats(array.values())
339339
} else {
340-
canonicalize_float_array(&array.values().slice(first, len))
340+
normalize_floats(&array.values().slice(first, len))
341341
}
342342
}
343343

‎datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
//! `false` so that later rules can prune the plan they filter.
2929
3030
use super::utils::{is_false, is_null};
31-
use datafusion_common::float_semantics::canonicalize_float_scalar;
31+
use datafusion_common::float_semantics::normalize_float_scalar;
3232
use datafusion_common::{Column, DataFusionError, Result, ScalarValue, internal_err};
3333
use datafusion_expr::utils::{
3434
conjunction, disjunction, split_binary_owned, split_conjunction_owned,
@@ -422,7 +422,7 @@ fn op_and_literal(predicate: &Expr) -> Result<(Operator, &ScalarValue)> {
422422
fn normalize_literal(expr: &mut Expr) {
423423
if let Expr::Literal(value, _) = expr {
424424
let original = std::mem::replace(value, ScalarValue::Null);
425-
*value = canonicalize_float_scalar(original);
425+
*value = normalize_float_scalar(original);
426426
}
427427
}
428428

‎datafusion/optimizer/src/simplify_expressions/utils.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
//! Utility functions for expression simplification
1919
2020
use datafusion_common::cse::NormalizeEq;
21-
use datafusion_common::float_semantics::canonicalize_float_scalar;
21+
use datafusion_common::float_semantics::normalize_float_scalar;
2222
use datafusion_common::{Result, ScalarValue, internal_err};
2323
use datafusion_expr::{
2424
Case, Expr, Like, Operator,
@@ -239,8 +239,8 @@ pub fn is_eq_and_ne_with_different_literal(eq_expr: &Expr, ne_expr: &Expr) -> bo
239239
Expr::Literal(right_value, right_metadata),
240240
) => {
241241
left_metadata != right_metadata
242-
|| canonicalize_float_scalar(left_value.clone())
243-
!= canonicalize_float_scalar(right_value.clone())
242+
|| normalize_float_scalar(left_value.clone())
243+
!= normalize_float_scalar(right_value.clone())
244244
}
245245
_ => left != right,
246246
}

‎datafusion/physical-expr-common/src/datum.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use arrow::compute::kernels::cmp::{
2424
use arrow::compute::{SortOptions, ilike, like, nilike, nlike};
2525
use arrow::error::ArrowError;
2626
use datafusion_common::float_semantics::{
27-
canonicalize_float_array, canonicalize_float_scalar, has_float_leaf,
27+
has_float_leaf, normalize_float_scalar, normalize_floats,
2828
};
2929
use datafusion_common::{Result, ScalarValue};
3030
use datafusion_common::{arrow_datafusion_err, assert_or_internal_err, internal_err};
@@ -99,9 +99,9 @@ pub fn apply_cmp(
9999

100100
fn normalize_cmp_input(cv: &ColumnarValue) -> ColumnarValue {
101101
match cv {
102-
ColumnarValue::Array(a) => ColumnarValue::Array(canonicalize_float_array(a)),
102+
ColumnarValue::Array(a) => ColumnarValue::Array(normalize_floats(a)),
103103
ColumnarValue::Scalar(s) => {
104-
ColumnarValue::Scalar(canonicalize_float_scalar(s.clone()))
104+
ColumnarValue::Scalar(normalize_float_scalar(s.clone()))
105105
}
106106
}
107107
}
@@ -151,7 +151,7 @@ pub fn compare_with_eq(
151151

152152
fn normalize_nested_float_zero(array: &dyn Array) -> Option<ArrayRef> {
153153
has_float_leaf(array.data_type())
154-
.then(|| canonicalize_float_array(&make_array(array.to_data())))
154+
.then(|| normalize_floats(&make_array(array.to_data())))
155155
}
156156

157157
/// Compare on nested type List, Struct, and so on

0 commit comments

Comments
 (0)