Skip to content

Commit e91265e

Browse files
committed
Use total order for float comparisons in lexicographical compares
It was already used when comparing single-value columns. CubeStore requires this on compactions to produce correct results.
1 parent f6a3075 commit e91265e

File tree

2 files changed

+7
-15
lines changed

2 files changed

+7
-15
lines changed

arrow/src/array/ord.rs

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,12 @@ use crate::datatypes::TimeUnit;
2424
use crate::datatypes::*;
2525
use crate::error::{ArrowError, Result};
2626

27+
use crate::compute::kernels::merge::FloatCmp;
2728
use num::Float;
2829

2930
/// Compare the values at two arbitrary indices in two arrays.
3031
pub type DynComparator = Box<dyn Fn(usize, usize) -> Ordering + Send + Sync>;
3132

32-
/// compares two floats, placing NaNs at last
33-
fn cmp_nans_last<T: Float>(a: &T, b: &T) -> Ordering {
34-
match (a.is_nan(), b.is_nan()) {
35-
(true, true) => Ordering::Equal,
36-
(true, false) => Ordering::Greater,
37-
(false, true) => Ordering::Less,
38-
_ => a.partial_cmp(b).unwrap(),
39-
}
40-
}
41-
4233
fn compare_primitives<T: ArrowPrimitiveType>(left: &Array, right: &Array) -> DynComparator
4334
where
4435
T::Native: Ord,
@@ -57,11 +48,12 @@ fn compare_boolean(left: &Array, right: &Array) -> DynComparator {
5748

5849
fn compare_float<T: ArrowPrimitiveType>(left: &Array, right: &Array) -> DynComparator
5950
where
60-
T::Native: Float,
51+
T::Native: Float + FloatCmp,
6152
{
6253
let left: PrimitiveArray<T> = PrimitiveArray::from(left.data().clone());
6354
let right: PrimitiveArray<T> = PrimitiveArray::from(right.data().clone());
64-
Box::new(move |i, j| cmp_nans_last(&left.value(i), &right.value(j)))
55+
// CubeStore-specific: use `total_cmp` instead of putting NaNs last.
56+
Box::new(move |i, j| left.value(i).total_cmp(right.value(j)))
6557
}
6658

6759
fn compare_string<T>(left: &Array, right: &Array) -> DynComparator
@@ -279,8 +271,8 @@ pub mod tests {
279271

280272
let cmp = build_compare(&array, &array)?;
281273

282-
assert_eq!(Ordering::Equal, (cmp)(0, 1));
283-
assert_eq!(Ordering::Equal, (cmp)(1, 0));
274+
assert_eq!(Ordering::Less, (cmp)(0, 1));
275+
assert_eq!(Ordering::Greater, (cmp)(1, 0));
284276
Ok(())
285277
}
286278

arrow/src/compute/kernels/merge.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ struct FloatComparator<'a, F: ArrowPrimitiveType> {
509509
arrays: Vec<&'a PrimitiveArray<F>>,
510510
}
511511

512-
trait FloatCmp {
512+
pub(crate) trait FloatCmp {
513513
fn total_cmp(self, r: Self) -> Ordering;
514514
}
515515

0 commit comments

Comments
 (0)