fix: NativePType reimplements Ord and Eq using it's own semantics (#2550)

A while ago we have added our own `total_compare` and `is_eq` methods to
`NativePType`, those methods provide implementations with definitions
useful for our arrays. However, at the same time we required default
PartialEq and PartialOrd which for float types have different semantics.
We remove usages of NativePTypes PartialOrd and PartialEq and replace it
with our own functions using our own total_compare and is_eq

Author: robert3005

encodings/fastlanes/benches/compute_between.rs

@@ -12,7 +12,7 @@ fn main() {
     divan::main();
 }
 
-fn generate_primitive_array<T: NativePType + NumCast + PartialOrd>(
+fn generate_primitive_array<T: NativePType + NumCast>(
     rng: &mut StdRng,
     len: usize,
 ) -> PrimitiveArray {
@@ -21,7 +21,7 @@ fn generate_primitive_array<T: NativePType + NumCast + PartialOrd>(
         .collect::<PrimitiveArray>()
 }
 
-fn generate_bit_pack_primitive_array<T: NativePType + NumCast + PartialOrd>(
+fn generate_bit_pack_primitive_array<T: NativePType + NumCast>(
     rng: &mut StdRng,
     len: usize,
 ) -> ArrayRef {
@@ -32,7 +32,7 @@ fn generate_bit_pack_primitive_array<T: NativePType + NumCast + PartialOrd>(
     bitpack_to_best_bit_width(&a).vortex_expect("").into_array()
 }
 
-fn generate_alp_bit_pack_primitive_array<T: NativePType + NumCast + PartialOrd>(
+fn generate_alp_bit_pack_primitive_array<T: NativePType + NumCast>(
     rng: &mut StdRng,
     len: usize,
 ) -> ArrayRef {

encodings/fastlanes/src/for/compute/mod.rs

@@ -130,7 +130,7 @@ where
         .vortex_expect("Reference value cannot be null");
     let primitive_value: T = value.cast(array.dtype())?.as_ref().try_into()?;
     // Make sure that smaller values are still smaller and not larger than (which they would be after wrapping_sub)
-    if primitive_value < min {
+    if primitive_value.is_lt(min) {
         return Ok(SearchResult::NotFound(array.invalid_count()?));
     }
 

vortex-array/src/arrays/primitive/compute/between.rs

@@ -34,26 +34,26 @@ fn between_impl<T: NativePType + Copy>(
     match (options.lower_strict, options.upper_strict) {
         // Note: these comparisons are explicitly passed in to allow function impl inlining
         (StrictComparison::Strict, StrictComparison::Strict) => {
-            between_impl_(arr, lower, PartialOrd::lt, upper, PartialOrd::lt)
+            between_impl_(arr, lower, NativePType::is_lt, upper, NativePType::is_lt)
         }
         (StrictComparison::Strict, StrictComparison::NonStrict) => {
-            between_impl_(arr, lower, PartialOrd::lt, upper, PartialOrd::le)
+            between_impl_(arr, lower, NativePType::is_lt, upper, NativePType::is_le)
         }
         (StrictComparison::NonStrict, StrictComparison::Strict) => {
-            between_impl_(arr, lower, PartialOrd::le, upper, PartialOrd::lt)
+            between_impl_(arr, lower, NativePType::is_le, upper, NativePType::is_lt)
         }
         (StrictComparison::NonStrict, StrictComparison::NonStrict) => {
-            between_impl_(arr, lower, PartialOrd::le, upper, PartialOrd::le)
+            between_impl_(arr, lower, NativePType::is_le, upper, NativePType::is_le)
         }
     }
 }
 
 fn between_impl_<T>(
     arr: &PrimitiveArray,
     lower: T,
-    lower_fn: impl Fn(&T, &T) -> bool,
+    lower_fn: impl Fn(T, T) -> bool,
     upper: T,
-    upper_fn: impl Fn(&T, &T) -> bool,
+    upper_fn: impl Fn(T, T) -> bool,
 ) -> ArrayRef
 where
     T: NativePType + Copy,
@@ -62,8 +62,8 @@ where
     BoolArray::new(
         BooleanBuffer::collect_bool(slice.len(), |idx| {
             // We only iterate upto arr len and |arr| == |slice|.
-            let i = unsafe { slice.get_unchecked(idx) };
-            lower_fn(&lower, i) & upper_fn(i, &upper)
+            let i = unsafe { *slice.get_unchecked(idx) };
+            lower_fn(lower, i) & upper_fn(i, upper)
         }),
         arr.validity().clone(),
     )

vortex-array/src/arrays/primitive/compute/is_constant.rs

@@ -23,7 +23,7 @@ fn compute_is_constant<T: NativePType>(values: &[T]) -> bool {
     let first_value = values[0];
 
     for value in &values[1..] {
-        if *value != first_value {
+        if !value.is_eq(first_value) {
             return false;
         }
     }

vortex-dtype/src/ptype.rs

@@ -64,8 +64,6 @@ pub trait NativePType:
     + Copy
     + Debug
     + Display
-    + PartialEq
-    + PartialOrd
     + Default
     + RefUnwindSafe
     + Num
@@ -85,6 +83,30 @@ pub trait NativePType:
     /// Compare another instance of this type to `self`, providing a total ordering
     fn total_compare(self, other: Self) -> Ordering;
 
+    /// Test whether self is less than or equal to the other
+    #[inline]
+    fn is_le(self, other: Self) -> bool {
+        self.total_compare(other).is_le()
+    }
+
+    /// Test whether self is less than the other
+    #[inline]
+    fn is_lt(self, other: Self) -> bool {
+        self.total_compare(other).is_lt()
+    }
+
+    /// Test whether self is greater than or equal to the other
+    #[inline]
+    fn is_ge(self, other: Self) -> bool {
+        self.total_compare(other).is_ge()
+    }
+
+    /// Test whether self is greater than the other
+    #[inline]
+    fn is_gt(self, other: Self) -> bool {
+        self.total_compare(other).is_gt()
+    }
+
     /// Whether another instance of this type (`other`) is bitwise equal to `self`
     fn is_eq(self, other: Self) -> bool;
 }