From 3c1d820f3b69d296a9a2f10abec4ce4349faed8b Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Fri, 7 Aug 2026 20:52:23 -0400 Subject: [PATCH] Share primitive binary operand decoding Move primitive array and constant decoding out of numeric execution so comparison does not depend on a numeric implementation detail. Numeric and comparison keep the same input classification and execution behavior. Signed-off-by: "Connor Tsui" --- .../scalar_fn/fns/binary/compare/primitive.rs | 2 +- vortex-array/src/scalar_fn/fns/binary/mod.rs | 1 + .../src/scalar_fn/fns/binary/numeric/mod.rs | 1 - .../scalar_fn/fns/binary/numeric/primitive.rs | 58 +--------------- .../scalar_fn/fns/binary/primitive_operand.rs | 69 +++++++++++++++++++ 5 files changed, 72 insertions(+), 59 deletions(-) create mode 100644 vortex-array/src/scalar_fn/fns/binary/primitive_operand.rs diff --git a/vortex-array/src/scalar_fn/fns/binary/compare/primitive.rs b/vortex-array/src/scalar_fn/fns/binary/compare/primitive.rs index 3bfb11a266e..1247358dce1 100644 --- a/vortex-array/src/scalar_fn/fns/binary/compare/primitive.rs +++ b/vortex-array/src/scalar_fn/fns/binary/compare/primitive.rs @@ -18,10 +18,10 @@ use crate::dtype::Nullability; use crate::dtype::PType; use crate::match_each_native_ptype; use crate::scalar::Scalar; -use crate::scalar_fn::fns::binary::PrimitiveOperand; use crate::scalar_fn::fns::binary::compare::collect_bits; use crate::scalar_fn::fns::binary::compare::collect_zip_bits; use crate::scalar_fn::fns::binary::compare::compare_validity; +use crate::scalar_fn::fns::binary::primitive_operand::PrimitiveOperand; use crate::scalar_fn::fns::operators::CompareOperator; /// Compare two primitive arrays of the same [`PType`]. diff --git a/vortex-array/src/scalar_fn/fns/binary/mod.rs b/vortex-array/src/scalar_fn/fns/binary/mod.rs index a5b9fe70539..80faff20e0a 100644 --- a/vortex-array/src/scalar_fn/fns/binary/mod.rs +++ b/vortex-array/src/scalar_fn/fns/binary/mod.rs @@ -43,6 +43,7 @@ mod compare; pub use compare::*; mod numeric; pub(crate) use numeric::*; +mod primitive_operand; use crate::scalar::NumericOperator; use crate::scalar::Scalar; diff --git a/vortex-array/src/scalar_fn/fns/binary/numeric/mod.rs b/vortex-array/src/scalar_fn/fns/binary/numeric/mod.rs index 6622e08f82b..6dc0de0fbea 100644 --- a/vortex-array/src/scalar_fn/fns/binary/numeric/mod.rs +++ b/vortex-array/src/scalar_fn/fns/binary/numeric/mod.rs @@ -13,7 +13,6 @@ mod primitive; mod tests; use decimal::execute_numeric_decimal; -pub(crate) use primitive::PrimitiveOperand; use primitive::execute_numeric_primitive; use vortex_error::VortexResult; use vortex_error::vortex_ensure; diff --git a/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs b/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs index 357547f25b8..8fd53d15216 100644 --- a/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs +++ b/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs @@ -14,7 +14,6 @@ use super::checked::checked_lanes; use crate::ArrayRef; use crate::ExecutionCtx; use crate::IntoArray; -use crate::arrays::Constant; use crate::arrays::ConstantArray; use crate::arrays::PrimitiveArray; use crate::builtins::ArrayBuiltins; @@ -25,6 +24,7 @@ use crate::dtype::half::f16; use crate::match_each_native_ptype; use crate::scalar::NumericOperator; use crate::scalar::Scalar; +use crate::scalar_fn::fns::binary::primitive_operand::PrimitiveOperand; use crate::validity::Validity; struct CheckedAdd; @@ -255,62 +255,6 @@ where } } -/// A primitive binary-operator operand: a materialized buffer, a non-null constant, or an -/// all-null constant. -pub(crate) enum PrimitiveOperand { - Array { - values: Buffer, - validity: Validity, - }, - Constant { - value: T, - len: usize, - validity: Validity, - }, - Null(usize), -} - -impl PrimitiveOperand { - pub(crate) fn try_new(array: &ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { - if let Some(constant) = array.as_opt::() { - return Ok( - match constant.scalar().as_primitive().try_typed_value::()? { - Some(value) => Self::Constant { - value, - len: array.len(), - validity: if constant.scalar().dtype().is_nullable() { - Validity::AllValid - } else { - Validity::NonNullable - }, - }, - None => Self::Null(array.len()), - }, - ); - } - - let array = array.clone().execute::(ctx)?; - let validity = array.validity()?; - let values = array.into_buffer::(); - Ok(Self::Array { values, validity }) - } - - pub(crate) fn len(&self) -> usize { - match self { - Self::Array { values, .. } => values.len(), - Self::Constant { len, .. } | Self::Null(len) => *len, - } - } - - pub(crate) fn validity(&self) -> Validity { - match self { - Self::Array { validity, .. } => validity.clone(), - Self::Constant { validity, .. } => validity.clone(), - Self::Null(_) => Validity::AllInvalid, - } - } -} - trait CheckedArithmetic: NativePType { const DIV_CHECKS_IN_VALUE_LOOP: bool; diff --git a/vortex-array/src/scalar_fn/fns/binary/primitive_operand.rs b/vortex-array/src/scalar_fn/fns/binary/primitive_operand.rs new file mode 100644 index 00000000000..71d1122fc79 --- /dev/null +++ b/vortex-array/src/scalar_fn/fns/binary/primitive_operand.rs @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +//! Decoding shared by primitive binary operators. + +use vortex_buffer::Buffer; +use vortex_error::VortexResult; + +use crate::ArrayRef; +use crate::ExecutionCtx; +use crate::arrays::Constant; +use crate::arrays::PrimitiveArray; +use crate::dtype::NativePType; +use crate::validity::Validity; + +/// A materialized primitive column, a non-null constant, or an all-null constant. +pub(super) enum PrimitiveOperand { + Array { + values: Buffer, + validity: Validity, + }, + Constant { + value: T, + len: usize, + validity: Validity, + }, + Null(usize), +} + +impl PrimitiveOperand { + pub(super) fn try_new(array: &ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { + if let Some(constant) = array.as_opt::() { + return Ok( + match constant.scalar().as_primitive().try_typed_value::()? { + Some(value) => Self::Constant { + value, + len: array.len(), + validity: if constant.scalar().dtype().is_nullable() { + Validity::AllValid + } else { + Validity::NonNullable + }, + }, + None => Self::Null(array.len()), + }, + ); + } + + let array = array.clone().execute::(ctx)?; + let validity = array.validity()?; + let values = array.into_buffer::(); + Ok(Self::Array { values, validity }) + } + + pub(super) fn len(&self) -> usize { + match self { + Self::Array { values, .. } => values.len(), + Self::Constant { len, .. } | Self::Null(len) => *len, + } + } + + pub(super) fn validity(&self) -> Validity { + match self { + Self::Array { validity, .. } => validity.clone(), + Self::Constant { validity, .. } => validity.clone(), + Self::Null(_) => Validity::AllInvalid, + } + } +}