fix[array]: min_max of nan is none

Signed-off-by: Joe Isaacs <[email protected]>

Author: joseph-isaacs

fuzz/fuzz_targets/array_ops.rs

@@ -8,7 +8,7 @@ use std::backtrace::Backtrace;
 
 use libfuzzer_sys::{Corpus, fuzz_target};
 use vortex_array::arrays::ConstantArray;
-use vortex_array::compute::{cast, compare, fill_null, filter, min_max, sum, take};
+use vortex_array::compute::{cast, compare, fill_null, filter, mask, min_max, sum, take};
 use vortex_array::search_sorted::{SearchResult, SearchSorted, SearchSortedSide};
 use vortex_array::{Array, ArrayRef, IntoArray};
 use vortex_btrblocks::BtrBlocksCompressor;
@@ -99,7 +99,6 @@ fuzz_target!(|fuzz_action: FuzzArrayAction| -> Corpus {
                 assert_array_eq(&expected.array(), &current_array, i).unwrap();
             }
             Action::Mask(mask_val) => {
-                use vortex_array::compute::mask;
                 current_array = mask(&current_array, &mask_val).vortex_unwrap();
                 assert_array_eq(&expected.array(), &current_array, i).unwrap();
             }

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

@@ -10,7 +10,7 @@ use crate::register_kernel;
 impl MinMaxKernel for ConstantVTable {
     fn min_max(&self, array: &ConstantArray) -> VortexResult<Option<MinMaxResult>> {
         let scalar = array.scalar();
-        if scalar.is_null() {
+        if scalar.is_null() || scalar.as_primitive_opt().is_some_and(|p| p.is_nan()) {
             return Ok(None);
         }
         let non_nullable_dtype = scalar.dtype().as_nonnullable();
@@ -22,3 +22,21 @@ impl MinMaxKernel for ConstantVTable {
 }
 
 register_kernel!(MinMaxKernelAdapter(ConstantVTable).lift());
+
+#[cfg(test)]
+mod test {
+    use vortex_dtype::Nullability;
+    use vortex_dtype::half::f16;
+    use vortex_scalar::Scalar;
+
+    use crate::arrays::ConstantArray;
+    use crate::compute::min_max;
+
+    #[test]
+    fn test_min_max_nan() {
+        let scalar = Scalar::primitive(f16::NAN, Nullability::NonNullable);
+        let array = ConstantArray::new(scalar, 2).to_array();
+        let result = min_max(&array).unwrap();
+        assert_eq!(result, None);
+    }
+}

vortex-array/src/compute/min_max.rs

@@ -133,13 +133,8 @@ fn min_max_impl(
         return Ok(None);
     }
 
-    if let Some(array) = array.as_opt::<ConstantVTable>()
-        && !array.scalar().is_null()
-    {
-        return Ok(Some(MinMaxResult {
-            min: array.scalar().clone(),
-            max: array.scalar().clone(),
-        }));
+    if let Some(array) = array.as_opt::<ConstantVTable>() {
+        return ConstantVTable.min_max(array);
     }
 
     let min = array
@@ -176,7 +171,8 @@ fn min_max_impl(
     vortex_bail!(NotImplemented: "min_max", array.encoding_id());
 }
 
-/// The minimum and maximum non-null values of an array, or None if there are no non-null values.
+/// The minimum and maximum non-null values of an array, or None if there are no non-null/or non-nan
+/// values.
 pub trait MinMaxKernel: VTable {
     fn min_max(&self, array: &Self::Array) -> VortexResult<Option<MinMaxResult>>;
 }

vortex-scalar/src/primitive.rs

@@ -133,6 +133,11 @@ impl<'a> PrimitiveScalar<'a> {
         }))
     }
 
+    /// Returns true if the scalar is nan.
+    pub fn is_nan(&self) -> bool {
+        self.pvalue.as_ref().is_some_and(|p| p.is_nan())
+    }
+
     /// Attempts to extract the primitive value as the given type.
     ///
     /// # Errors

vortex-scalar/src/pvalue.rs

@@ -196,6 +196,16 @@ impl PValue {
         }
     }
 
+    /// Returns true if the value of float type and is NaN.
+    pub fn is_nan(&self) -> bool {
+        match self {
+            PValue::F16(f) => f.is_nan(),
+            PValue::F32(f) => f.is_nan(),
+            PValue::F64(f) => f.is_nan(),
+            _ => false,
+        }
+    }
+
     /// Reinterprets the bits of this value as a different primitive type.
     ///
     /// This performs a bitwise cast between types of the same width.