Merge branch 'develop' into os/validate

onursatici · web-flow · commit 4904c9d09736 · 2025-11-06T13:46:28.000Z
diff --git a/encodings/sequence/src/compute/min_max.rs b/encodings/sequence/src/compute/min_max.rs
@@ -3,6 +3,7 @@
 
 use vortex_array::compute::{MinMaxKernel, MinMaxKernelAdapter, MinMaxResult};
 use vortex_array::register_kernel;
+use vortex_dtype::Nullability::NonNullable;
 use vortex_error::VortexResult;
 use vortex_scalar::Scalar;
 
@@ -19,8 +20,8 @@ impl MinMaxKernel for SequenceVTable {
             (last, base)
         };
         Ok(Some(MinMaxResult {
-            min: Scalar::new(array.dtype().clone(), min.into()),
-            max: Scalar::new(array.dtype().clone(), max.into()),
+            min: Scalar::primitive_value(min, array.ptype(), NonNullable),
+            max: Scalar::primitive_value(max, array.ptype(), NonNullable),
         }))
     }
 }
diff --git a/fuzz/fuzz_targets/array_ops.rs b/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();
             }
diff --git a/vortex-array/src/arrays/bool/compute/min_max.rs b/vortex-array/src/arrays/bool/compute/min_max.rs
@@ -3,6 +3,8 @@
 
 use std::ops::BitAnd;
 
+use Nullability::NonNullable;
+use vortex_dtype::Nullability;
 use vortex_error::VortexResult;
 use vortex_mask::Mask;
 use vortex_scalar::Scalar;
@@ -30,15 +32,15 @@ impl MinMaxKernel for BoolVTable {
         let Some(slice) = true_slices.next() else {
             // all false
             return Ok(Some(MinMaxResult {
-                min: Scalar::bool(false, array.dtype().nullability()),
-                max: Scalar::bool(false, array.dtype().nullability()),
+                min: Scalar::bool(false, NonNullable),
+                max: Scalar::bool(false, NonNullable),
             }));
         };
         if slice.0 == 0 && slice.1 == array.len() {
             // all true
             return Ok(Some(MinMaxResult {
-                min: Scalar::bool(true, array.dtype().nullability()),
-                max: Scalar::bool(true, array.dtype().nullability()),
+                min: Scalar::bool(true, NonNullable),
+                max: Scalar::bool(true, NonNullable),
             }));
         };
 
@@ -53,16 +55,16 @@ impl MinMaxKernel for BoolVTable {
                 let Some(_) = false_slices.next() else {
                     // In this case we don't have any false values which means we are all true and null
                     return Ok(Some(MinMaxResult {
-                        min: Scalar::bool(true, array.dtype().nullability()),
-                        max: Scalar::bool(true, array.dtype().nullability()),
+                        min: Scalar::bool(true, NonNullable),
+                        max: Scalar::bool(true, NonNullable),
                     }));
                 };
             }
         }
 
         Ok(Some(MinMaxResult {
-            min: Scalar::bool(false, array.dtype().nullability()),
-            max: Scalar::bool(true, array.dtype().nullability()),
+            min: Scalar::bool(false, NonNullable),
+            max: Scalar::bool(true, NonNullable),
         }))
     }
 }
@@ -71,47 +73,47 @@ register_kernel!(MinMaxKernelAdapter(BoolVTable).lift());
 
 #[cfg(test)]
 mod tests {
-    use vortex_dtype::{DType, Nullability};
+    use Nullability::NonNullable;
+    use vortex_dtype::Nullability;
     use vortex_scalar::Scalar;
 
     use crate::arrays::BoolArray;
     use crate::compute::{MinMaxResult, min_max};
 
     #[test]
     fn test_min_max_nulls() {
-        let dtype = DType::Bool(Nullability::Nullable);
         assert_eq!(
             min_max(BoolArray::from_iter(vec![Some(true), Some(true), None, None]).as_ref())
                 .unwrap(),
             Some(MinMaxResult {
-                min: Scalar::new(dtype.clone(), true.into()),
-                max: Scalar::new(dtype.clone(), true.into()),
+                min: Scalar::bool(true, NonNullable),
+                max: Scalar::bool(true, NonNullable),
             })
         );
 
         assert_eq!(
             min_max(BoolArray::from_iter(vec![None, Some(true), Some(true)]).as_ref()).unwrap(),
             Some(MinMaxResult {
-                min: Scalar::new(dtype.clone(), true.into()),
-                max: Scalar::new(dtype.clone(), true.into()),
+                min: Scalar::bool(true, NonNullable),
+                max: Scalar::bool(true, NonNullable),
             })
         );
 
         assert_eq!(
             min_max(BoolArray::from_iter(vec![None, Some(true), Some(true), None]).as_ref())
                 .unwrap(),
             Some(MinMaxResult {
-                min: Scalar::new(dtype.clone(), true.into()),
-                max: Scalar::new(dtype.clone(), true.into()),
+                min: Scalar::bool(true, NonNullable),
+                max: Scalar::bool(true, NonNullable),
             })
         );
 
         assert_eq!(
             min_max(BoolArray::from_iter(vec![Some(false), Some(false), None, None]).as_ref())
                 .unwrap(),
             Some(MinMaxResult {
-                min: Scalar::new(dtype.clone(), false.into()),
-                max: Scalar::new(dtype, false.into()),
+                min: Scalar::bool(false, NonNullable),
+                max: Scalar::bool(false, NonNullable),
             })
         );
     }
diff --git a/vortex-array/src/arrays/constant/compute/min_max.rs b/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);
+    }
+}
diff --git a/vortex-array/src/arrays/decimal/compute/min_max.rs b/vortex-array/src/arrays/decimal/compute/min_max.rs
@@ -2,10 +2,11 @@
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
 use itertools::Itertools;
-use vortex_dtype::{DType, NativeDecimalType, match_each_decimal_value_type};
+use vortex_dtype::Nullability::NonNullable;
+use vortex_dtype::{DecimalDType, NativeDecimalType, match_each_decimal_value_type};
 use vortex_error::VortexResult;
 use vortex_mask::Mask;
-use vortex_scalar::{DecimalValue, Scalar, ScalarValue};
+use vortex_scalar::{DecimalValue, Scalar};
 
 use crate::arrays::{DecimalArray, DecimalVTable};
 use crate::compute::{MinMaxKernel, MinMaxKernelAdapter, MinMaxResult};
@@ -27,35 +28,38 @@ where
     D: Into<DecimalValue> + NativeDecimalType,
 {
     Ok(match array.validity_mask() {
-        Mask::AllTrue(_) => compute_min_max(array.buffer::<D>().iter(), array.dtype()),
+        Mask::AllTrue(_) => compute_min_max(array.buffer::<D>().iter(), array.decimal_dtype()),
         Mask::AllFalse(_) => None,
         Mask::Values(v) => compute_min_max(
             array
                 .buffer::<D>()
                 .iter()
                 .zip(v.bit_buffer().iter())
                 .filter_map(|(v, m)| m.then_some(v)),
-            array.dtype(),
+            array.decimal_dtype(),
         ),
     })
 }
 
-fn compute_min_max<'a, T>(iter: impl Iterator<Item = &'a T>, dtype: &DType) -> Option<MinMaxResult>
+fn compute_min_max<'a, T>(
+    iter: impl Iterator<Item = &'a T>,
+    decimal_dtype: DecimalDType,
+) -> Option<MinMaxResult>
 where
     T: Into<DecimalValue> + NativeDecimalType + Ord + Copy + 'a,
 {
     match iter.minmax_by(|a, b| a.cmp(b)) {
         itertools::MinMaxResult::NoElements => None,
         itertools::MinMaxResult::OneElement(&x) => {
-            let scalar = Scalar::new(dtype.clone(), ScalarValue::from(x.into()));
+            let scalar = Scalar::decimal(x.into(), decimal_dtype, NonNullable);
             Some(MinMaxResult {
                 min: scalar.clone(),
                 max: scalar,
             })
         }
         itertools::MinMaxResult::MinMax(&min, &max) => Some(MinMaxResult {
-            min: Scalar::new(dtype.clone(), ScalarValue::from(min.into())),
-            max: Scalar::new(dtype.clone(), ScalarValue::from(max.into())),
+            min: Scalar::decimal(min.into(), decimal_dtype, NonNullable),
+            max: Scalar::decimal(max.into(), decimal_dtype, NonNullable),
         }),
     }
 }
@@ -80,13 +84,14 @@ mod tests {
 
         let min_max = min_max(decimal.as_ref()).unwrap();
 
+        let non_nullable_dtype = decimal.dtype().as_nonnullable();
         let expected = MinMaxResult {
             min: Scalar::new(
-                decimal.dtype().clone(),
+                non_nullable_dtype.clone(),
                 ScalarValue::from(DecimalValue::from(100i32)),
             ),
             max: Scalar::new(
-                decimal.dtype().clone(),
+                non_nullable_dtype,
                 ScalarValue::from(DecimalValue::from(200i32)),
             ),
         };
diff --git a/vortex-array/src/arrays/extension/compute/min_max.rs b/vortex-array/src/arrays/extension/compute/min_max.rs
@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
+use std::sync::Arc;
+
+use vortex_dtype::Nullability;
 use vortex_error::VortexResult;
 use vortex_scalar::Scalar;
 
@@ -10,10 +13,12 @@ use crate::register_kernel;
 
 impl MinMaxKernel for ExtensionVTable {
     fn min_max(&self, array: &ExtensionArray) -> VortexResult<Option<MinMaxResult>> {
+        let non_nullable_ext_dtype =
+            Arc::new(array.ext_dtype().with_nullability(Nullability::NonNullable));
         Ok(
             compute::min_max(array.storage())?.map(|MinMaxResult { min, max }| MinMaxResult {
-                min: Scalar::extension(array.ext_dtype().clone(), min),
-                max: Scalar::extension(array.ext_dtype().clone(), max),
+                min: Scalar::extension(non_nullable_ext_dtype.clone(), min),
+                max: Scalar::extension(non_nullable_ext_dtype, max),
             }),
         )
     }
diff --git a/vortex-array/src/arrays/primitive/compute/min_max.rs b/vortex-array/src/arrays/primitive/compute/min_max.rs
@@ -2,10 +2,11 @@
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
 use itertools::Itertools;
-use vortex_dtype::{DType, NativePType, match_each_native_ptype};
+use vortex_dtype::Nullability::NonNullable;
+use vortex_dtype::{NativePType, match_each_native_ptype};
 use vortex_error::VortexResult;
 use vortex_mask::Mask;
-use vortex_scalar::{Scalar, ScalarValue};
+use vortex_scalar::{PValue, Scalar};
 
 use crate::arrays::{PrimitiveArray, PrimitiveVTable};
 use crate::compute::{MinMaxKernel, MinMaxKernelAdapter, MinMaxResult};
@@ -24,25 +25,26 @@ register_kernel!(MinMaxKernelAdapter(PrimitiveVTable).lift());
 #[inline]
 fn compute_min_max_with_validity<T>(array: &PrimitiveArray) -> VortexResult<Option<MinMaxResult>>
 where
-    T: Into<ScalarValue> + NativePType,
+    T: NativePType,
+    PValue: From<T>,
 {
     Ok(match array.validity_mask() {
-        Mask::AllTrue(_) => compute_min_max(array.as_slice::<T>().iter(), array.dtype()),
+        Mask::AllTrue(_) => compute_min_max(array.as_slice::<T>().iter()),
         Mask::AllFalse(_) => None,
         Mask::Values(v) => compute_min_max(
             array
                 .as_slice::<T>()
                 .iter()
                 .zip(v.bit_buffer().iter())
                 .filter_map(|(v, m)| m.then_some(v)),
-            array.dtype(),
         ),
     })
 }
 
-fn compute_min_max<'a, T>(iter: impl Iterator<Item = &'a T>, dtype: &DType) -> Option<MinMaxResult>
+fn compute_min_max<'a, T>(iter: impl Iterator<Item = &'a T>) -> Option<MinMaxResult>
 where
-    T: Into<ScalarValue> + NativePType,
+    T: NativePType,
+    PValue: From<T>,
 {
     // `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
@@ -52,15 +54,15 @@ where
     {
         itertools::MinMaxResult::NoElements => None,
         itertools::MinMaxResult::OneElement(&x) => {
-            let scalar = Scalar::new(dtype.clone(), x.into());
+            let scalar = Scalar::primitive(x, NonNullable);
             Some(MinMaxResult {
                 min: scalar.clone(),
                 max: scalar,
             })
         }
         itertools::MinMaxResult::MinMax(&min, &max) => Some(MinMaxResult {
-            min: Scalar::new(dtype.clone(), min.into()),
-            max: Scalar::new(dtype.clone(), max.into()),
+            min: Scalar::primitive(min, NonNullable),
+            max: Scalar::primitive(max, NonNullable),
         }),
     }
 }
diff --git a/vortex-array/src/arrays/varbin/compute/min_max.rs b/vortex-array/src/arrays/varbin/compute/min_max.rs
@@ -3,6 +3,7 @@
 
 use itertools::Itertools;
 use vortex_dtype::DType;
+use vortex_dtype::Nullability::NonNullable;
 use vortex_error::{VortexResult, vortex_panic};
 use vortex_scalar::Scalar;
 
@@ -45,12 +46,12 @@ pub(crate) fn varbin_compute_min_max<T: ArrayAccessor<[u8]>>(
 /// Helper function to make sure that min/max has the right [`Scalar`] type.
 fn make_scalar(dtype: &DType, value: &[u8]) -> Scalar {
     match dtype {
-        DType::Binary(_) => Scalar::new(dtype.clone(), value.into()),
+        DType::Binary(_) => Scalar::binary(value.to_vec(), NonNullable),
         DType::Utf8(_) => {
             // SAFETY: We only call `compute_min_max` within `varbin/`, in which we always validate
             // the arrays, and we always pass `array.dtype()` in as the `dtype` argument.
             let value = unsafe { str::from_utf8_unchecked(value) };
-            Scalar::new(dtype.clone(), value.into())
+            Scalar::utf8(value, NonNullable)
         }
         _ => vortex_panic!("cannot make Scalar from bytes with dtype {dtype}"),
     }
@@ -60,7 +61,7 @@ fn make_scalar(dtype: &DType, value: &[u8]) -> Scalar {
 mod tests {
     use vortex_buffer::BufferString;
     use vortex_dtype::DType::Utf8;
-    use vortex_dtype::Nullability::Nullable;
+    use vortex_dtype::Nullability::{NonNullable, Nullable};
     use vortex_scalar::Scalar;
 
     use crate::arrays::VarBinArray;
@@ -83,14 +84,14 @@ mod tests {
         assert_eq!(
             min,
             Scalar::new(
-                Utf8(Nullable),
+                Utf8(NonNullable),
                 BufferString::from("hello world".to_string()).into(),
             )
         );
         assert_eq!(
             max,
             Scalar::new(
-                Utf8(Nullable),
+                Utf8(NonNullable),
                 BufferString::from("hello world this is a long string".to_string()).into()
             )
         );
diff --git a/vortex-array/src/compute/min_max.rs b/vortex-array/src/compute/min_max.rs
diff --git a/vortex-scalar/src/primitive.rs b/vortex-scalar/src/primitive.rs
diff --git a/vortex-scalar/src/pvalue.rs b/vortex-scalar/src/pvalue.rs
