From b274b38a4094d10bb0b17f4e04e4a3fbc8380b0c Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Thu, 25 Jun 2026 12:16:05 +0100 Subject: [PATCH 1/3] Guard calling is_nan on scalar value on dtype being a float Signed-off-by: Robert Kruszewski --- vortex-array/src/aggregate_fn/fns/min_max/mod.rs | 4 ++++ vortex-array/src/dtype/ptype.rs | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs index 097d3137f42..b89f42e4ecb 100644 --- a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs @@ -149,6 +149,10 @@ pub(crate) fn nan_scalar(dtype: &DType) -> Scalar { /// Whether a scalar holds a primitive float NaN value. pub(crate) fn scalar_is_nan(scalar: &Scalar) -> bool { + if !scalar.dtype().is_float() { + return false; + } + scalar.as_primitive_opt().is_some_and(|p| p.is_nan()) } diff --git a/vortex-array/src/dtype/ptype.rs b/vortex-array/src/dtype/ptype.rs index ab443618d66..a038eb328c2 100644 --- a/vortex-array/src/dtype/ptype.rs +++ b/vortex-array/src/dtype/ptype.rs @@ -21,6 +21,7 @@ use num_traits::Unsigned; use num_traits::bounds::UpperBounded; use vortex_error::VortexError; use vortex_error::VortexResult; +use vortex_error::vortex_bail; use vortex_error::vortex_err; use crate::dtype::DType; @@ -845,7 +846,7 @@ impl TryFrom<&DType> for PType { if let DType::Primitive(p, _) = value { Ok(*p) } else { - Err(vortex_err!("Cannot convert DType {} into PType", value)) + vortex_bail!("Cannot convert DType {value} into PType") } } } From af30c93eb95f9456defbc531044c915f64bed687 Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Thu, 25 Jun 2026 12:20:20 +0100 Subject: [PATCH 2/3] more Signed-off-by: Robert Kruszewski --- vortex-array/src/aggregate_fn/fns/max/mod.rs | 4 ++-- vortex-array/src/aggregate_fn/fns/min/mod.rs | 4 ++-- vortex-array/src/aggregate_fn/fns/min_max/mod.rs | 6 ++++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/vortex-array/src/aggregate_fn/fns/max/mod.rs b/vortex-array/src/aggregate_fn/fns/max/mod.rs index 79194f56941..df4ab1adc5a 100644 --- a/vortex-array/src/aggregate_fn/fns/max/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/max/mod.rs @@ -49,7 +49,7 @@ impl MaxPartial { // NaN scalars are incomparable under `partial_max`; they poison the maximum when NaNs // participate, and are dropped when they are skipped. - if scalar_is_nan(&max) || self.is_poisoned() { + if self.element_dtype.is_float() && (scalar_is_nan(&max) || self.is_poisoned()) { if !self.skip_nans { self.poison(); } @@ -67,7 +67,7 @@ impl MaxPartial { } fn is_poisoned(&self) -> bool { - self.max.as_ref().is_some_and(scalar_is_nan) + self.element_dtype.is_float() && self.max.as_ref().is_some_and(scalar_is_nan) } } diff --git a/vortex-array/src/aggregate_fn/fns/min/mod.rs b/vortex-array/src/aggregate_fn/fns/min/mod.rs index 8a0fbd889ed..c997e156933 100644 --- a/vortex-array/src/aggregate_fn/fns/min/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/min/mod.rs @@ -49,7 +49,7 @@ impl MinPartial { // NaN scalars are incomparable under `partial_min`; they poison the minimum when NaNs // participate, and are dropped when they are skipped. - if scalar_is_nan(&min) || self.is_poisoned() { + if self.element_dtype.is_float() && (scalar_is_nan(&min) || self.is_poisoned()) { if !self.skip_nans { self.poison(); } @@ -67,7 +67,7 @@ impl MinPartial { } fn is_poisoned(&self) -> bool { - self.min.as_ref().is_some_and(scalar_is_nan) + self.element_dtype.is_float() && self.min.as_ref().is_some_and(scalar_is_nan) } } diff --git a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs index b89f42e4ecb..4995753ceeb 100644 --- a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs @@ -210,7 +210,9 @@ impl MinMaxPartial { // NaN scalars are incomparable under `partial_min`/`partial_max`, so they are handled // explicitly: a NaN extremum poisons the partial state when NaNs participate, and is // dropped when they are skipped. - if scalar_is_nan(&min) || scalar_is_nan(&max) || self.is_poisoned() { + if self.element_dtype.is_float() + && (scalar_is_nan(&min) || scalar_is_nan(&max) || self.is_poisoned()) + { if !self.skip_nans { self.poison(); } @@ -237,7 +239,7 @@ impl MinMaxPartial { /// Whether the partial state is poisoned to NaN. fn is_poisoned(&self) -> bool { - self.min.as_ref().is_some_and(scalar_is_nan) + self.element_dtype.is_float() && self.min.as_ref().is_some_and(scalar_is_nan) } } From ef24d8f4b0fb2a90b92c2dd08b164c65a8f13e73 Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Thu, 25 Jun 2026 12:26:01 +0100 Subject: [PATCH 3/3] simpler Signed-off-by: Robert Kruszewski --- vortex-array/src/aggregate_fn/fns/max/mod.rs | 2 +- vortex-array/src/aggregate_fn/fns/min/mod.rs | 2 +- vortex-array/src/aggregate_fn/fns/min_max/mod.rs | 4 +--- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/vortex-array/src/aggregate_fn/fns/max/mod.rs b/vortex-array/src/aggregate_fn/fns/max/mod.rs index df4ab1adc5a..dcec83c2b1f 100644 --- a/vortex-array/src/aggregate_fn/fns/max/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/max/mod.rs @@ -49,7 +49,7 @@ impl MaxPartial { // NaN scalars are incomparable under `partial_max`; they poison the maximum when NaNs // participate, and are dropped when they are skipped. - if self.element_dtype.is_float() && (scalar_is_nan(&max) || self.is_poisoned()) { + if scalar_is_nan(&max) || self.is_poisoned() { if !self.skip_nans { self.poison(); } diff --git a/vortex-array/src/aggregate_fn/fns/min/mod.rs b/vortex-array/src/aggregate_fn/fns/min/mod.rs index c997e156933..6f557c2158f 100644 --- a/vortex-array/src/aggregate_fn/fns/min/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/min/mod.rs @@ -49,7 +49,7 @@ impl MinPartial { // NaN scalars are incomparable under `partial_min`; they poison the minimum when NaNs // participate, and are dropped when they are skipped. - if self.element_dtype.is_float() && (scalar_is_nan(&min) || self.is_poisoned()) { + if scalar_is_nan(&min) || self.is_poisoned() { if !self.skip_nans { self.poison(); } diff --git a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs index 4995753ceeb..312ed1e1408 100644 --- a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs @@ -210,9 +210,7 @@ impl MinMaxPartial { // NaN scalars are incomparable under `partial_min`/`partial_max`, so they are handled // explicitly: a NaN extremum poisons the partial state when NaNs participate, and is // dropped when they are skipped. - if self.element_dtype.is_float() - && (scalar_is_nan(&min) || scalar_is_nan(&max) || self.is_poisoned()) - { + if scalar_is_nan(&min) || scalar_is_nan(&max) || self.is_poisoned() { if !self.skip_nans { self.poison(); }