NaN cannot be a min/max value of a primitive array (#3104)

In a followup we will add a stat for nan_count of the array

fixes #1375

Author: robert3005

encodings/alp/src/alp/compute/compare.rs

@@ -80,7 +80,7 @@ where
             Operator::Gt | Operator::Gte => {
                 // Per IEEE 754 totalOrder semantics the ordering is -Nan < -Inf < Inf < Nan.
                 // All values in the encoded array are definitely finite
-                let is_not_finite = value.is_infinite() || NativePType::is_nan(value);
+                let is_not_finite = NativePType::is_infinite(value) || NativePType::is_nan(value);
                 if is_not_finite {
                     Ok(Some(
                         ConstantArray::new(value.is_sign_negative(), alp.len()).into_array(),
@@ -99,7 +99,7 @@ where
             Operator::Lt | Operator::Lte => {
                 // Per IEEE 754 totalOrder semantics the ordering is -Nan < -Inf < Inf < Nan.
                 // All values in the encoded array are definitely finite
-                let is_not_finite = value.is_infinite() || NativePType::is_nan(value);
+                let is_not_finite = NativePType::is_infinite(value) || NativePType::is_nan(value);
                 if is_not_finite {
                     Ok(Some(
                         ConstantArray::new(value.is_sign_positive(), alp.len()).into_array(),

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

@@ -38,10 +38,14 @@ where
 
 fn compute_min_max<'a, T>(iter: impl Iterator<Item = &'a T>, dtype: &DType) -> Option<MinMaxResult>
 where
-    T: Into<ScalarValue> + NativePType + Copy,
+    T: Into<ScalarValue> + NativePType,
 {
-    // this `compare` function provides a total ordering (even for NaN values)
-    match iter.minmax_by(|a, b| a.total_compare(**b)) {
+    // `total_compare` function provides a total ordering (even for NaN values).
+    // However, we exclude NaNs from min max as they're not useful for any purpose where min/max would be used
+    match iter
+        .filter(|v| !v.is_nan())
+        .minmax_by(|a, b| a.total_compare(**b))
+    {
         itertools::MinMaxResult::NoElements => None,
         itertools::MinMaxResult::OneElement(&x) => {
             let scalar = Scalar::new(dtype.clone(), x.into());
@@ -56,3 +60,34 @@ where
         }),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use vortex_buffer::buffer;
+
+    use crate::arrays::PrimitiveArray;
+    use crate::compute::min_max;
+    use crate::validity::Validity;
+
+    #[test]
+    fn min_max_nan() {
+        let array = PrimitiveArray::new(
+            buffer![f32::NAN, -f32::NAN, -1.0, 1.0],
+            Validity::NonNullable,
+        );
+        let min_max = min_max(&array).unwrap().unwrap();
+        assert_eq!(f32::try_from(min_max.min).unwrap(), -1.0);
+        assert_eq!(f32::try_from(min_max.max).unwrap(), 1.0);
+    }
+
+    #[test]
+    fn min_max_inf() {
+        let array = PrimitiveArray::new(
+            buffer![f32::INFINITY, f32::NEG_INFINITY, -1.0, 1.0],
+            Validity::NonNullable,
+        );
+        let min_max = min_max(&array).unwrap().unwrap();
+        assert_eq!(f32::try_from(min_max.min).unwrap(), f32::NEG_INFINITY);
+        assert_eq!(f32::try_from(min_max.max).unwrap(), f32::INFINITY);
+    }
+}

vortex-dtype/src/ptype.rs

@@ -68,6 +68,10 @@ pub trait NativePType:
     /// For integer types, this is always `false`
     fn is_nan(self) -> bool;
 
+    /// Whether this instance (`self`) is Infinite
+    /// For integer types, this is always `false`
+    fn is_infinite(self) -> bool;
+
     /// Compare another instance of this type to `self`, providing a total ordering
     fn total_compare(self, other: Self) -> Ordering;
 
@@ -109,6 +113,11 @@ macro_rules! native_ptype {
                 false
             }
 
+            #[inline]
+            fn is_infinite(self) -> bool {
+                false
+            }
+
             #[inline]
             fn total_compare(self, other: Self) -> Ordering {
                 self.cmp(&other)
@@ -132,6 +141,11 @@ macro_rules! native_float_ptype {
                 <$T>::is_nan(self)
             }
 
+            #[inline]
+            fn is_infinite(self) -> bool {
+                <$T>::is_infinite(self)
+            }
+
             #[inline]
             fn total_compare(self, other: Self) -> Ordering {
                 self.total_cmp(&other)