From 7c8bdfe1375380e21464c90b04ed4841b1bdb13e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 23:44:19 +0000 Subject: [PATCH] fix(arrow): return an error for incompatible Arrow export targets `execute_arrow_naive` dispatches purely on the requested Arrow `DataType`, but the bool, null, decimal, list, list-view, and fixed-size-list arms then call `execute::`, which is documented as panicking when the array's dtype does not match. Exporting an array to an incompatible Arrow type therefore panicked instead of returning an error. Guard each arm with `vortex_ensure!` on the source dtype, mirroring the existing check in `to_arrow_byte_array`. The guards are per-arm rather than centralised because compatibility is not symmetric: a Bool source exports to Int32 via a cast kernel, while a Primitive source cannot produce Boolean, so a single "type classes must match" rule would reject conversions that work. Signed-off-by: Robert Kruszewski --- vortex-arrow/src/executor/bool.rs | 7 ++ vortex-arrow/src/executor/decimal.rs | 9 ++ vortex-arrow/src/executor/fixed_size_list.rs | 7 ++ vortex-arrow/src/executor/list.rs | 6 ++ vortex-arrow/src/executor/list_view.rs | 6 ++ vortex-arrow/src/executor/mod.rs | 91 ++++++++++++++++++++ vortex-arrow/src/executor/null.rs | 7 ++ 7 files changed, 133 insertions(+) diff --git a/vortex-arrow/src/executor/bool.rs b/vortex-arrow/src/executor/bool.rs index 64bffa804f2..f6815884b33 100644 --- a/vortex-arrow/src/executor/bool.rs +++ b/vortex-arrow/src/executor/bool.rs @@ -9,7 +9,9 @@ use vortex_array::ArrayRef; use vortex_array::ExecutionCtx; use vortex_array::arrays::BoolArray; use vortex_array::arrays::bool::BoolArrayExt; +use vortex_array::dtype::DType; use vortex_error::VortexResult; +use vortex_error::vortex_ensure; use crate::null_buffer::to_null_buffer; @@ -33,6 +35,11 @@ pub(super) fn to_arrow_bool( array: ArrayRef, ctx: &mut ExecutionCtx, ) -> VortexResult { + vortex_ensure!( + matches!(array.dtype(), DType::Bool(_)), + "Cannot convert Vortex array with dtype {} to an Arrow Boolean array", + array.dtype() + ); let bool_array = array.execute::(ctx)?; canonical_bool_to_arrow(&bool_array, ctx) } diff --git a/vortex-arrow/src/executor/decimal.rs b/vortex-arrow/src/executor/decimal.rs index d5eabc77145..0acffa15361 100644 --- a/vortex-arrow/src/executor/decimal.rs +++ b/vortex-arrow/src/executor/decimal.rs @@ -16,9 +16,11 @@ use num_traits::ToPrimitive; use vortex_array::ArrayRef; use vortex_array::ExecutionCtx; use vortex_array::arrays::DecimalArray; +use vortex_array::dtype::DType; use vortex_array::dtype::DecimalType; use vortex_buffer::Buffer; use vortex_error::VortexResult; +use vortex_error::vortex_ensure; use vortex_error::vortex_err; use crate::null_buffer::to_null_buffer; @@ -28,6 +30,13 @@ pub(super) fn to_arrow_decimal( data_type: &DataType, ctx: &mut ExecutionCtx, ) -> VortexResult { + vortex_ensure!( + matches!(array.dtype(), DType::Decimal(..)), + "Cannot convert Vortex array with dtype {} to an Arrow {} array", + array.dtype(), + data_type + ); + // Execute the array as a DecimalArray. let decimal_array = array.execute::(ctx)?; diff --git a/vortex-arrow/src/executor/fixed_size_list.rs b/vortex-arrow/src/executor/fixed_size_list.rs index 2d556f38e73..7220bbe13f7 100644 --- a/vortex-arrow/src/executor/fixed_size_list.rs +++ b/vortex-arrow/src/executor/fixed_size_list.rs @@ -10,6 +10,7 @@ use vortex_array::arrays::FixedSizeList; use vortex_array::arrays::FixedSizeListArray; use vortex_array::arrays::fixed_size_list::FixedSizeListArrayExt; use vortex_array::arrays::fixed_size_list::FixedSizeListArraySlotsExt; +use vortex_array::dtype::DType; use vortex_error::VortexResult; use vortex_error::vortex_ensure; @@ -22,6 +23,12 @@ pub(super) fn to_arrow_fixed_list( elements_field: &FieldRef, ctx: &mut ExecutionCtx, ) -> VortexResult { + vortex_ensure!( + matches!(array.dtype(), DType::FixedSizeList(..)), + "Cannot convert Vortex array with dtype {} to an Arrow FixedSizeList array", + array.dtype() + ); + // Check for Vortex FixedSizeListArray and convert directly. if let Some(array) = array.as_opt::() { return list_to_list(&array.into_owned(), elements_field, list_size, ctx); diff --git a/vortex-arrow/src/executor/list.rs b/vortex-arrow/src/executor/list.rs index 271fedd5667..177f4e9cff6 100644 --- a/vortex-arrow/src/executor/list.rs +++ b/vortex-arrow/src/executor/list.rs @@ -52,6 +52,12 @@ pub(super) fn to_arrow_list( elements_field: &FieldRef, ctx: &mut ExecutionCtx, ) -> VortexResult { + vortex_ensure!( + matches!(array.dtype(), DType::List(..)), + "Cannot convert Vortex array with dtype {} to an Arrow list array", + array.dtype() + ); + let array = array.execute_until::(ctx)?; // If the Vortex array is already in List format, we can directly convert it. diff --git a/vortex-arrow/src/executor/list_view.rs b/vortex-arrow/src/executor/list_view.rs index ed1ba93e556..350e59dcf8b 100644 --- a/vortex-arrow/src/executor/list_view.rs +++ b/vortex-arrow/src/executor/list_view.rs @@ -31,6 +31,12 @@ pub(super) fn to_arrow_list_view( elements_field: &FieldRef, ctx: &mut ExecutionCtx, ) -> VortexResult { + vortex_ensure!( + matches!(array.dtype(), DType::List(..)), + "Cannot convert Vortex array with dtype {} to an Arrow list-view array", + array.dtype() + ); + let array = array.execute::(ctx)?; // Reclaim unreferenced elements before handing the array to Arrow. Otherwise downstream diff --git a/vortex-arrow/src/executor/mod.rs b/vortex-arrow/src/executor/mod.rs index 14d94ccc033..854a713f746 100644 --- a/vortex-arrow/src/executor/mod.rs +++ b/vortex-arrow/src/executor/mod.rs @@ -251,3 +251,94 @@ fn infer_nearest_arrow_type(array: &ArrayRef) -> VortexResult { // Everything else: use canonical dtype conversion to_data_type_naive(array.dtype()) } + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use arrow_schema::DataType; + use arrow_schema::Field; + use arrow_schema::TimeUnit; + use rstest::rstest; + use vortex_array::ArrayRef; + use vortex_array::IntoArray; + use vortex_array::VortexSessionExecute; + use vortex_array::array_session; + use vortex_array::arrays::BoolArray; + use vortex_array::arrays::PrimitiveArray; + use vortex_array::arrays::VarBinViewArray; + + use crate::ArrowSessionExt; + + fn utf8() -> ArrayRef { + VarBinViewArray::from_iter_str(["a", "bb"]).into_array() + } + + fn primitive() -> ArrayRef { + PrimitiveArray::from_iter([1i32, 2]).into_array() + } + + fn boolean() -> ArrayRef { + BoolArray::from_iter([true, false]).into_array() + } + + fn list_target() -> DataType { + DataType::List(Arc::new(Field::new("item", DataType::Int32, true))) + } + + /// Must error rather than panic inside `execute::`. + #[rstest] + #[case::bool_from_utf8(utf8(), DataType::Boolean)] + #[case::bool_from_primitive(primitive(), DataType::Boolean)] + #[case::null_from_utf8(utf8(), DataType::Null)] + #[case::null_from_primitive(primitive(), DataType::Null)] + #[case::decimal_from_utf8(utf8(), DataType::Decimal128(10, 2))] + #[case::decimal_from_primitive(primitive(), DataType::Decimal128(10, 2))] + #[case::list_from_utf8(utf8(), list_target())] + #[case::list_from_primitive(primitive(), list_target())] + #[case::list_view_from_primitive( + primitive(), + DataType::ListView(Arc::new(Field::new("item", DataType::Int32, true))) + )] + #[case::fixed_size_list_from_utf8( + utf8(), + DataType::FixedSizeList(Arc::new(Field::new("item", DataType::Int32, true)), 2) + )] + #[case::byte_from_primitive(primitive(), DataType::Utf8)] + #[case::byte_from_bool(boolean(), DataType::Binary)] + fn incompatible_target_returns_error(#[case] array: ArrayRef, #[case] target: DataType) { + let session = array_session(); + let mut ctx = session.create_execution_ctx(); + let field = Field::new("f", target.clone(), array.dtype().is_nullable()); + + let result = session.arrow().execute_arrow(array, Some(&field), &mut ctx); + + assert!( + result.is_err(), + "expected an error exporting to {target:?}, got Ok" + ); + } + + /// Cross-class conversions that are genuinely supported must keep working. + #[rstest] + #[case::bool_to_int32(boolean(), DataType::Int32)] + #[case::bool_to_float64(boolean(), DataType::Float64)] + #[case::primitive_to_int64(primitive(), DataType::Int64)] + #[case::primitive_to_date32(primitive(), DataType::Date32)] + #[case::primitive_to_timestamp(primitive(), DataType::Timestamp(TimeUnit::Microsecond, None))] + #[case::utf8_to_binary(utf8(), DataType::Binary)] + #[case::utf8_to_large_utf8(utf8(), DataType::LargeUtf8)] + fn supported_cross_class_target_still_works(#[case] array: ArrayRef, #[case] target: DataType) { + let session = array_session(); + let mut ctx = session.create_execution_ctx(); + let field = Field::new("f", target.clone(), array.dtype().is_nullable()); + + let result = session.arrow().execute_arrow(array, Some(&field), &mut ctx); + + assert!( + result.is_ok(), + "expected {target:?} export to succeed, got {:?}", + result.err() + ); + } +} diff --git a/vortex-arrow/src/executor/null.rs b/vortex-arrow/src/executor/null.rs index c5ffc991006..7e5dac3e9c4 100644 --- a/vortex-arrow/src/executor/null.rs +++ b/vortex-arrow/src/executor/null.rs @@ -8,7 +8,9 @@ use arrow_array::NullArray as ArrowNullArray; use vortex_array::ArrayRef; use vortex_array::ExecutionCtx; use vortex_array::arrays::NullArray; +use vortex_array::dtype::DType; use vortex_error::VortexResult; +use vortex_error::vortex_ensure; /// Convert a canonical NullArray directly to Arrow. pub fn canonical_null_to_arrow(array: &NullArray) -> ArrowArrayRef { @@ -19,6 +21,11 @@ pub(super) fn to_arrow_null( array: ArrayRef, ctx: &mut ExecutionCtx, ) -> VortexResult { + vortex_ensure!( + matches!(array.dtype(), DType::Null), + "Cannot convert Vortex array with dtype {} to an Arrow Null array", + array.dtype() + ); let null_array = array.execute::(ctx)?; Ok(canonical_null_to_arrow(&null_array)) }