Skip to content

Commit fa32325

Browse files
alambclaude
alamb
and
claude
committed
docs: link filter docs to InListExpr and float_semantics, rename to ColumnarValue::normalize
Co-Authored-By: Claude Fable 5.1 <[email protected]>
1 parent ac752db commit fa32325

6 files changed

Lines changed: 38 additions & 26 deletions

File tree

‎datafusion/common/src/float_semantics.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,8 @@ pub fn normalize_floats(array: &ArrayRef) -> ArrayRef {
155155

156156
/// Like [`normalize_floats`], but takes ownership of the array.
157157
///
158-
/// When nothing needs to change the array is returned as is, and when it
159-
/// does, the values are rewritten in place if nothing else refers to them.
160-
/// Only nested arrays and shared buffers pay for a copy.
158+
/// This avoids any copying (including arc ref counts) if there is nothing, and
159+
/// will reuse the underlying allocation if possible
161160
pub fn normalize_floats_owned(array: ArrayRef) -> ArrayRef {
162161
match array.data_type() {
163162
DataType::Float16 => normalize_primitive_owned::<Float16Type>(array),
@@ -180,6 +179,8 @@ where
180179
Arc::new(arr.unary::<_, T>(normalize_float))
181180
}
182181

182+
/// Like [`normalize_primitive`] except it takes an owned array
183+
/// and tries to reuse the allocation when possible;
183184
fn normalize_primitive_owned<T>(array: ArrayRef) -> ArrayRef
184185
where
185186
T: ArrowPrimitiveType,
@@ -194,8 +195,8 @@ where
194195
let arr = arr.clone();
195196
drop(array);
196197
let normalized = match arr.unary_mut(normalize_float) {
197-
Ok(normalized) => normalized,
198-
Err(arr) => arr.unary::<_, T>(normalize_float),
198+
Ok(normalized) => normalized, // updated in place
199+
Err(arr) => arr.unary::<_, T>(normalize_float), // needed to copy
199200
};
200201
Arc::new(normalized)
201202
}

‎datafusion/expr-common/src/columnar_value.rs‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,11 @@ impl ColumnarValue {
114114
}
115115
}
116116

117-
/// Applies DataFusion's float equality semantics to this value: `-0.0`
118-
/// becomes `+0.0`, so that comparing bit patterns afterwards treats the
119-
/// two zeros as equal. Arrays are normalized at any nesting depth and
120-
/// returned as they are when they contain no `-0.0`; scalars are
121-
/// normalized when they are plain floats.
117+
/// Applies DataFusion's [float equality semantics] to this value (e.g.
118+
/// `-0.0` becomes `+0.0`)
122119
///
123-
/// See [`normalize_floats_owned`] and [`ScalarValue::normalize`].
124-
///
125-
/// [`normalize_floats_owned`]: datafusion_common::float_semantics::normalize_floats_owned
126-
pub fn normalize_floats(self) -> Self {
120+
/// [float equality semantics]: datafusion_common::float_semantics
121+
pub fn normalize(self) -> Self {
127122
match self {
128123
ColumnarValue::Array(array) => {
129124
ColumnarValue::Array(normalize_floats_owned(array))

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,19 @@ pub fn apply_cmp(
7676
// floats, which treats `-0.0` and `+0.0` as distinct. Normalize float
7777
// operands so SQL semantics (`+0.0 == -0.0`) hold. No-op for
7878
// non-float types.
79-
let lhs = lhs.clone().normalize_floats();
80-
let rhs = rhs.clone().normalize_floats();
79+
let lhs = normalize_cmp_input(lhs);
80+
let rhs = normalize_cmp_input(rhs);
8181
apply(&lhs, &rhs, |l, r| Ok(Arc::new(f(l, r)?)))
8282
}
8383
}
8484

85+
fn normalize_cmp_input(cv: &ColumnarValue) -> ColumnarValue {
86+
match cv {
87+
ColumnarValue::Array(a) => ColumnarValue::Array(normalize_floats(a)),
88+
ColumnarValue::Scalar(s) => ColumnarValue::Scalar(s.clone().normalize()),
89+
}
90+
}
91+
8592
/// Applies a binary [`Datum`] comparison operator `op` to `lhs` and `rhs` for nested type like
8693
/// List, FixedSizeList, LargeList, Struct, Union, Map, or a dictionary of a nested type
8794
pub fn apply_cmp_for_nested(

‎datafusion/physical-expr/src/expressions/in_list.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ impl PhysicalExpr for InListExpr {
324324
fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
325325
let num_rows = batch.num_rows();
326326
let value = self.expr.evaluate(batch)?;
327-
let value = value.normalize_floats(); // so 0.0 == -0.0
327+
let value = value.normalize(); // so 0.0 == -0.0
328328
let r = match &self.static_filter {
329329
Some(filter) => {
330330
match value {
@@ -374,7 +374,7 @@ impl PhysicalExpr for InListExpr {
374374

375375
// Helper: compare value against a single list expression
376376
let compare_one = |expr: &Arc<dyn PhysicalExpr>| -> Result<BooleanArray> {
377-
let list_value = expr.evaluate(batch)?.normalize_floats(); // 0.0 == -0.0
377+
let list_value = expr.evaluate(batch)?.normalize(); // 0.0 == -0.0
378378
match list_value {
379379
ColumnarValue::Array(array) => {
380380
if lhs_supports_arrow_eq

‎datafusion/physical-expr/src/expressions/in_list/branchless_filter.rs‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,15 @@
2525
//! `Float32` and a `UInt32` both use four bytes per value. The filter compares
2626
//! those stored bits through an unsigned type of the same size, without copying
2727
//! the value buffer. A bit pattern is simply the bytes Arrow uses to store a
28-
//! value. Comparing it preserves details such as different NaN payloads.
29-
//! `InListExpr` normalizes `-0.0` to `+0.0` in both the list and the input
30-
//! before they reach this filter, so signed zeros still compare equal, as
31-
//! they do for `=` (see `datafusion_common::float_semantics`).
32-
//! [`BranchlessFilterType`] defines these safe, same-sized mappings and
33-
//! checks their sizes at compile time.
28+
//! value. Comparing it preserves details such as `0.0` versus `-0.0` and
29+
//! different NaN values. [`BranchlessFilterType`] defines these safe,
30+
//! same-sized mappings and checks their sizes at compile time.
31+
//!
32+
//! It is important therefore for [`InListExpr`] to normalize `-0.0` to `+0.0` in
33+
//! both the list and the input before they reach this filter, so signed zeros
34+
//! compare equal, as they do for `=` (see [`datafusion_common::float_semantics`]).
35+
//!
36+
//! [`InListExpr`]: crate::expressions::InListExpr
3437
//!
3538
//! The fast path is limited to short lists:
3639
//!

‎datafusion/physical-expr/src/expressions/in_list/primitive_filter.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,10 @@ where
322322
/// Wrapper for f32 that implements Hash and Eq using bit comparison.
323323
/// This treats NaN values as equal to each other when they have the same bit pattern.
324324
///
325-
/// Note: `InListExpr` must have normalized via `datafusion_common::float_semantics`
325+
/// Note: [`InListExpr`] must normalize the values first, see
326+
/// [`datafusion_common::float_semantics`].
327+
///
328+
/// [`InListExpr`]: crate::expressions::InListExpr
326329
#[derive(Clone, Copy)]
327330
struct OrderedFloat32(f32);
328331

@@ -349,7 +352,10 @@ impl From<f32> for OrderedFloat32 {
349352
/// Wrapper for f64 that implements Hash and Eq using bit comparison.
350353
/// This treats NaN values as equal to each other when they have the same bit pattern.
351354
///
352-
/// Note: `InListExpr` must have normalized via `datafusion_common::float_semantics`
355+
/// Note: [`InListExpr`] must normalize the values first, see
356+
/// [`datafusion_common::float_semantics`].
357+
///
358+
/// [`InListExpr`]: crate::expressions::InListExpr
353359
#[derive(Clone, Copy)]
354360
struct OrderedFloat64(f64);
355361

0 commit comments

Comments
 (0)