diff --git a/STYLE.md b/STYLE.md index b75876e0e6e..b65a1de164f 100644 --- a/STYLE.md +++ b/STYLE.md @@ -114,8 +114,8 @@ Watch for these accessors inside `for i in 0..n { ... }`: | Per-element accessor | Hidden cost | Bulk replacement | | --- | --- | --- | -| `Validity::is_valid(i)` / `is_null(i)` | Array-backed validity allocates an `ExecutionCtx` and runs a scalar lookup per call. | Call `validity.execute_mask(len, ctx)?` once, then read the materialized mask. | -| `array.scalar_at(i)` / `array.execute_scalar(i, ctx)` | Executes through the compute stack per element. | Canonicalize once with `execute::` or `as_slice`, then index. | +| `Validity::execute_is_valid(i, ctx)` / `execute_is_null(i, ctx)` | Array-backed validity runs a scalar lookup per call. | Call `validity.execute_mask(len, ctx)?` once, then read the materialized mask. | +| `array.execute_scalar(i, ctx)` | Executes through the compute stack per element. | Canonicalize once with `execute::` or `as_slice`, then index. | | `BitBuffer::value(i)` / `Mask::value(i)` accumulated into a count | Recomputes the byte address and defeats popcount. | Use `true_count()`, `BitBuffer::count_range(start, end)`, or `set_indices()`. | | `BitIterator::next()` accumulated into a rank or prefix count | Processes one bit at a time. | Use `count_range` over each gap. | | Re-deriving a value such as `self.validity()?` | Repeats the derivation for every element. | Hoist the derivation above the loop. | diff --git a/clippy.toml b/clippy.toml index 73f74ca35f5..9f6d5bd21b8 100644 --- a/clippy.toml +++ b/clippy.toml @@ -17,14 +17,4 @@ disallowed-methods = [ { path = "std::thread::available_parallelism", reason = "This function might do an unbounded amount of work, use `vortex_utils::parallelism::get_available_parallelism instead" }, { path = "vortex_session::registry::Id::new", reason = "Interning a static id on every call grabs the interner lock (#8380). Use a `CachedId` static for static ids; for a dynamic string, annotate the call with `#[expect(clippy::disallowed_methods)]`.", allow-invalid = true }, { path = "vortex_session::registry::Id::new_static", reason = "Interning a static id on every call grabs the interner lock; use a `CachedId` static instead (#8380).", allow-invalid = true }, - { path = "vortex_array::legacy_session", reason = "Relies on the hidden global session; thread an explicit `VortexSession`/`ExecutionCtx` through instead" }, - { path = "vortex_array::ToCanonical::to_null", reason = "This function doesn't take the context, use `array.execute::(ctx)` instead", allow-invalid = true }, - { path = "vortex_array::ToCanonical::to_bool", reason = "This function doesn't take the context, use `array.execute::(ctx)` instead", allow-invalid = true }, - { path = "vortex_array::ToCanonical::to_primitive", reason = "This function doesn't take the context, use `array.execute::(ctx)` instead", allow-invalid = true }, - { path = "vortex_array::ToCanonical::to_decimal", reason = "This function doesn't take the context, use `array.execute::(ctx)` instead", allow-invalid = true }, - { path = "vortex_array::ToCanonical::to_struct", reason = "This function doesn't take the context, use `array.execute::(ctx)` instead", allow-invalid = true }, - { path = "vortex_array::ToCanonical::to_listview", reason = "This function doesn't take the context, use `array.execute::(ctx)` instead", allow-invalid = true }, - { path = "vortex_array::ToCanonical::to_fixed_size_list", reason = "This function doesn't take the context, use `array.execute::(ctx)` instead", allow-invalid = true }, - { path = "vortex_array::ToCanonical::to_varbinview", reason = "This function doesn't take the context, use `array.execute::(ctx)` instead", allow-invalid = true }, - { path = "vortex_array::ToCanonical::to_extension", reason = "This function doesn't take the context, use `array.execute::(ctx)` instead", allow-invalid = true }, ] diff --git a/docs/developer-guide/benchmarking.md b/docs/developer-guide/benchmarking.md index 9c2f096f897..a747a29bf3b 100644 --- a/docs/developer-guide/benchmarking.md +++ b/docs/developer-guide/benchmarking.md @@ -92,11 +92,13 @@ Return values are automatically black-boxed. You only need explicit `black_box` for side-effect-free results inside manual loops: ```rust -bencher.with_inputs(|| &array).bench_refs(|array| { - for idx in 0..len { - divan::black_box(array.scalar_at(idx).unwrap()); - } -}); +bencher + .with_inputs(|| (&array, SESSION.create_execution_ctx())) + .bench_refs(|(array, ctx)| { + for idx in 0..len { + divan::black_box(array.execute_scalar(idx, ctx).unwrap()); + } + }); ``` ### Use deterministic, seeded RNG diff --git a/encodings/fastlanes/src/delta/compute/cast.rs b/encodings/fastlanes/src/delta/compute/cast.rs index d2beafe62ff..658d3ddb2e0 100644 --- a/encodings/fastlanes/src/delta/compute/cast.rs +++ b/encodings/fastlanes/src/delta/compute/cast.rs @@ -53,6 +53,7 @@ mod tests { use std::sync::LazyLock; use rstest::rstest; + use vortex_array::Canonical; use vortex_array::IntoArray; use vortex_array::VortexSessionExecute; use vortex_array::arrays::PrimitiveArray; @@ -173,11 +174,13 @@ mod tests { let array = Delta::try_from_primitive_array(&values, &mut SESSION.create_execution_ctx())?; // The cast is lazy; the nullability check fires when it is executed. - #[expect(deprecated)] let result = array .into_array() .cast(DType::Primitive(PType::U32, Nullability::NonNullable)) - .and_then(|a| a.to_canonical().map(|c| c.into_array())); + .and_then(|a| { + a.execute::(&mut SESSION.create_execution_ctx()) + .map(|c| c.into_array()) + }); assert!( result.is_err(), diff --git a/vortex-array/src/array/erased.rs b/vortex-array/src/array/erased.rs index f0559b576bb..7f804c6c865 100644 --- a/vortex-array/src/array/erased.rs +++ b/vortex-array/src/array/erased.rs @@ -29,7 +29,6 @@ use crate::ExecutionCtx; use crate::ExecutionResult; use crate::IntoArray; use crate::VTable; -use crate::VortexSessionExecute; use crate::aggregate_fn::fns::sum::sum; use crate::array::ArrayData; use crate::array::ArrayId; @@ -48,7 +47,6 @@ use crate::dtype::DType; use crate::expr::stats::Precision; use crate::expr::stats::Stat; use crate::expr::stats::StatsProviderExt; -use crate::legacy_session; use crate::matcher::Matcher; use crate::optimizer::ArrayOptimizer; use crate::scalar::Scalar; @@ -266,16 +264,6 @@ impl ArrayRef { .optimize() } - /// Fetch the scalar at the given index. - #[deprecated( - note = "Use `execute_scalar` instead, which allows passing an execution context for more \ - efficient execution when fetching multiple scalars from the same array." - )] - #[allow(clippy::disallowed_methods)] - pub fn scalar_at(&self, index: usize) -> VortexResult { - self.execute_scalar(index, &mut legacy_session().create_execution_ctx()) - } - /// Execute the array to extract a scalar at the given index. /// /// A one-off read; the same as `self.probe().execute_scalar(index, ctx)`. @@ -399,21 +387,6 @@ impl ArrayRef { self.0.data.validity(self) } - /// Returns the canonical representation of the array. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - #[allow(clippy::disallowed_methods)] - pub fn into_canonical(self) -> VortexResult { - self.execute(&mut legacy_session().create_execution_ctx()) - } - - /// Returns the canonical representation of the array. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - pub fn to_canonical(&self) -> VortexResult { - #[expect(deprecated)] - let result = self.clone().into_canonical(); - result - } - /// Writes the array into the canonical builder. pub fn append_to_builder( &self, diff --git a/vortex-array/src/array/typed.rs b/vortex-array/src/array/typed.rs index 90f07a0bd38..1446afc217c 100644 --- a/vortex-array/src/array/typed.rs +++ b/vortex-array/src/array/typed.rs @@ -24,12 +24,10 @@ use crate::ArrayRef; use crate::ArraySlots; use crate::ExecutionCtx; use crate::IntoArray; -use crate::VortexSessionExecute; use crate::array::ArrayId; use crate::array::ArrayView; use crate::array::VTable; use crate::dtype::DType; -use crate::legacy_session; use crate::stats::ArrayStats; use crate::stats::StatsSet; use crate::stats::StatsSetRef; @@ -413,16 +411,6 @@ impl Array { self.inner.slice(range) } - #[deprecated( - note = "Use `execute_scalar` instead, which allows passing an execution context for more \ - efficient execution when fetching multiple scalars from the same array." - )] - #[allow(clippy::disallowed_methods)] - pub fn scalar_at(&self, index: usize) -> VortexResult { - self.inner - .execute_scalar(index, &mut legacy_session().create_execution_ctx()) - } - /// Execute the array to extract a scalar at the given index. pub fn execute_scalar( &self, @@ -467,13 +455,6 @@ impl Array { self.inner.all_invalid(ctx) } - #[deprecated(note = "Use Array::::execute::() instead")] - pub fn to_canonical(&self) -> VortexResult { - #[expect(deprecated)] - let result = self.inner.to_canonical(); - result - } - /// Returns the estimated physical bytes owned or referenced by this array tree. pub fn nbytes(&self) -> u64 { self.inner.nbytes() diff --git a/vortex-array/src/arrays/primitive/compute/cast.rs b/vortex-array/src/arrays/primitive/compute/cast.rs index deb4915fb23..3eb19b4b6f9 100644 --- a/vortex-array/src/arrays/primitive/compute/cast.rs +++ b/vortex-array/src/arrays/primitive/compute/cast.rs @@ -667,6 +667,7 @@ mod test { use vortex_mask::Mask; use crate::ArrayRef; + use crate::Canonical; use crate::IntoArray; use crate::VortexSessionExecute; use crate::array_session; @@ -1081,10 +1082,12 @@ mod test { #[test] fn cast_i32_u32() { let arr = buffer![-1i32].into_array(); - #[expect(deprecated)] let error = arr .cast(PType::U32.into()) - .and_then(|a| a.to_canonical().map(|c| c.into_array())) + .and_then(|a| { + a.execute::(&mut array_session().create_execution_ctx()) + .map(|c| c.into_array()) + }) .unwrap_err(); assert!(matches!(error, VortexError::Compute(..))); assert!(error.to_string().contains("values exceed target range")); @@ -1093,11 +1096,13 @@ mod test { #[test] fn cast_array_with_nulls_to_nonnullable() { let arr = PrimitiveArray::from_option_iter([Some(-1i32), None, Some(10)]); - #[expect(deprecated)] let err = arr .into_array() .cast(PType::I32.into()) - .and_then(|a| a.to_canonical().map(|c| c.into_array())) + .and_then(|a| { + a.execute::(&mut array_session().create_execution_ctx()) + .map(|c| c.into_array()) + }) .unwrap_err(); assert!(matches!(err, VortexError::InvalidArgument(..))); @@ -1157,10 +1162,12 @@ mod test { #[test] fn cast_same_width_int_out_of_range_errors() { let arr = buffer![u32::MAX].into_array(); - #[expect(deprecated)] let err = arr .cast(PType::I32.into()) - .and_then(|a| a.to_canonical().map(|c| c.into_array())) + .and_then(|a| { + a.execute::(&mut array_session().create_execution_ctx()) + .map(|c| c.into_array()) + }) .unwrap_err(); assert!(matches!(err, VortexError::Compute(..))); } diff --git a/vortex-array/src/canonical.rs b/vortex-array/src/canonical.rs index 97e36001fd4..eb92d6300b8 100644 --- a/vortex-array/src/canonical.rs +++ b/vortex-array/src/canonical.rs @@ -493,123 +493,6 @@ impl IntoArray for Canonical { } } -/// Trait for types that can be converted from an owned type into an owned array variant. -/// -/// # Canonicalization -/// -/// This trait has a blanket implementation for all types implementing [ToCanonical]. -#[deprecated(note = "use `array.execute::(ctx)` instead")] -pub trait ToCanonical { - /// Canonicalize into a [`NullArray`] if the target is [`Null`](DType::Null) typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_null(&self) -> NullArray; - - /// Canonicalize into a [`BoolArray`] if the target is [`Bool`](DType::Bool) typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_bool(&self) -> BoolArray; - - /// Canonicalize into a [`PrimitiveArray`] if the target is [`Primitive`](DType::Primitive) - /// typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_primitive(&self) -> PrimitiveArray; - - /// Canonicalize into a [`DecimalArray`] if the target is [`Decimal`](DType::Decimal) - /// typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_decimal(&self) -> DecimalArray; - - /// Canonicalize into a [`StructArray`] if the target is [`Struct`](DType::Struct) typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_struct(&self) -> StructArray; - - /// Canonicalize into a [`ListViewArray`] if the target is [`List`](DType::List) typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_listview(&self) -> ListViewArray; - - /// Canonicalize into a [`MapArray`] if the target is [`Map`](DType::Map) typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_map(&self) -> MapArray; - - /// Canonicalize into a [`FixedSizeListArray`] if the target is [`List`](DType::FixedSizeList) - /// typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_fixed_size_list(&self) -> FixedSizeListArray; - - /// Canonicalize into a [`VarBinViewArray`] if the target is [`Utf8`](DType::Utf8) - /// or [`Binary`](DType::Binary) typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_varbinview(&self) -> VarBinViewArray; - - /// Canonicalize into an [`ExtensionArray`] if the array is [`Extension`](DType::Extension) - /// typed. - #[deprecated(note = "use `array.execute::(ctx)` instead")] - fn to_extension(&self) -> ExtensionArray; -} - -// Blanket impl for all Array encodings. -#[expect(deprecated)] -impl ToCanonical for ArrayRef { - fn to_null(&self) -> NullArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_null() - } - - fn to_bool(&self) -> BoolArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_bool() - } - - fn to_primitive(&self) -> PrimitiveArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_primitive() - } - - fn to_decimal(&self) -> DecimalArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_decimal() - } - - fn to_struct(&self) -> StructArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_struct() - } - - fn to_listview(&self) -> ListViewArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_listview() - } - - fn to_map(&self) -> MapArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_map() - } - - fn to_fixed_size_list(&self) -> FixedSizeListArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_fixed_size_list() - } - - fn to_varbinview(&self) -> VarBinViewArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_varbinview() - } - - fn to_extension(&self) -> ExtensionArray { - #[expect(deprecated)] - let result = self.to_canonical().vortex_expect("to_canonical failed"); - result.into_extension() - } -} - impl From for ArrayRef { fn from(value: Canonical) -> Self { match_each_canonical!(value, |arr| arr.into_array()) diff --git a/vortex-array/src/lib.rs b/vortex-array/src/lib.rs index fc64f01de52..4eb2620fe82 100644 --- a/vortex-array/src/lib.rs +++ b/vortex-array/src/lib.rs @@ -79,8 +79,6 @@ extern crate self as vortex_array; -use std::sync::LazyLock; - pub use array::*; pub use canonical::*; pub use columnar::*; @@ -177,15 +175,4 @@ pub fn array_session() -> VortexSession { .with::() } -// TODO(ngates): canonicalize doesn't currently take a session, therefore we cannot invoke execute -// from the new array encodings to support back-compat for legacy encodings. So we hold a session -// here... -/// Returns the hidden global [`VortexSession`] used as a back-compat shim for code paths that -/// cannot yet thread an explicit session through. -#[inline] -pub fn legacy_session() -> &'static VortexSession { - static LEGACY_SESSION: LazyLock = LazyLock::new(array_session); - &LEGACY_SESSION -} - pub type ArrayContext = Interner; diff --git a/vortex-array/src/normalize.rs b/vortex-array/src/normalize.rs index 76c3ba23742..33f4de740ad 100644 --- a/vortex-array/src/normalize.rs +++ b/vortex-array/src/normalize.rs @@ -105,6 +105,7 @@ mod tests { use super::NormalizeOptions; use super::Operation; use crate::ArrayRef; + use crate::Canonical; use crate::IntoArray; use crate::VortexSessionExecute; use crate::array::VTable; @@ -229,8 +230,7 @@ mod tests { assert_eq!(normalized.len(), 3); // Verify the data: codes [1,0,1] -> values [20, 10, 20] - #[expect(deprecated)] - let normalized_canonical = normalized.to_canonical()?; + let normalized_canonical = normalized.execute::(&mut ctx)?; assert_arrays_eq!( normalized_canonical, PrimitiveArray::from_iter(vec![20i32, 10, 20]), diff --git a/vortex-array/src/validity.rs b/vortex-array/src/validity.rs index 4442d378e03..d1acdf650df 100644 --- a/vortex-array/src/validity.rs +++ b/vortex-array/src/validity.rs @@ -24,14 +24,12 @@ use crate::ArrayRef; use crate::Canonical; use crate::ExecutionCtx; use crate::IntoArray; -use crate::VortexSessionExecute; use crate::arrays::BoolArray; use crate::arrays::ChunkedArray; use crate::arrays::ConstantArray; use crate::builtins::ArrayBuiltins; use crate::dtype::DType; use crate::dtype::Nullability; -use crate::legacy_session; use crate::optimizer::ArrayOptimizer; use crate::patches::Patches; use crate::scalar::Scalar; @@ -184,22 +182,6 @@ impl Validity { Ok(!self.execute_is_valid(index, ctx)?) } - /// Returns whether the `index` item is valid. - #[deprecated(note = "use `execute_is_valid` with an explicit `ExecutionCtx`")] - #[inline] - #[allow(clippy::disallowed_methods)] - pub fn is_valid(&self, index: usize) -> VortexResult { - self.execute_is_valid(index, &mut legacy_session().create_execution_ctx()) - } - - /// Returns whether the `index` item is null. - #[deprecated(note = "use `execute_is_null` with an explicit `ExecutionCtx`")] - #[inline] - #[allow(clippy::disallowed_methods)] - pub fn is_null(&self, index: usize) -> VortexResult { - self.execute_is_null(index, &mut legacy_session().create_execution_ctx()) - } - #[inline] pub fn slice(&self, range: Range) -> VortexResult { match self { diff --git a/vortex-arrow/benches/to_arrow.rs b/vortex-arrow/benches/to_arrow.rs index e0e037ae372..9833bcc64d0 100644 --- a/vortex-arrow/benches/to_arrow.rs +++ b/vortex-arrow/benches/to_arrow.rs @@ -40,8 +40,6 @@ use vortex_array::session::ArraySessionExt; )] use vortex_arrow::ArrowArrayExecutor; use vortex_arrow::ArrowSessionExt; -#[allow(deprecated)] -use vortex_arrow::dtype::ToArrowType as _; use vortex_fsst::fsst_compress; use vortex_fsst::fsst_train_compressor; use vortex_onpair::DEFAULT_CONFIG; @@ -110,14 +108,6 @@ fn array() -> ArrayRef { .into_array() } -#[divan::bench] -fn to_arrow_dtype(bencher: Bencher) { - bencher.with_inputs(schema).bench_values(|dtype| { - #[expect(deprecated, reason = "benchmarking deprecated code path")] - dtype.to_arrow_dtype().unwrap() - }); -} - #[allow(non_snake_case)] #[divan::bench] fn ArrowExportVTable_to_arrow_field(bencher: Bencher) { diff --git a/vortex-arrow/src/convert.rs b/vortex-arrow/src/convert.rs index 20976c71434..bc25b1a9406 100644 --- a/vortex-arrow/src/convert.rs +++ b/vortex-arrow/src/convert.rs @@ -6,9 +6,7 @@ //! Each `from_arrow_*` function converts one Arrow array shape; [`from_arrow_dyn`] dispatches on //! the Arrow [`DataType`]. These functions have no awareness of Arrow extension types — the //! plugin-aware entry points are the [`ArrowSession`](crate::ArrowSession) methods, which fall -//! back to these conversions for non-extension data. The deprecated [`FromArrowArray`] impls are -//! thin shims over these functions and will eventually be removed. -#![allow(deprecated)] +//! back to these conversions for non-extension data. use std::ops::Range; use std::sync::Arc; @@ -107,7 +105,6 @@ use vortex_error::vortex_ensure_eq; use vortex_error::vortex_err; use vortex_error::vortex_panic; -use crate::FromArrowArray; use crate::dtype::from_arrow_time_unit; /// Zero-copy conversion of Arrow buffers into non-nullable Vortex arrays. @@ -179,28 +176,6 @@ where Ok(PrimitiveArray::new(buffer, validity).into_array()) } -macro_rules! impl_from_arrow_primitive { - ($T:path) => { - impl FromArrowArray<&ArrowPrimitiveArray<$T>> for ArrayRef { - fn from_arrow(value: &ArrowPrimitiveArray<$T>, nullable: bool) -> VortexResult { - from_arrow_primitive(value, nullable) - } - } - }; -} - -impl_from_arrow_primitive!(Int8Type); -impl_from_arrow_primitive!(Int16Type); -impl_from_arrow_primitive!(Int32Type); -impl_from_arrow_primitive!(Int64Type); -impl_from_arrow_primitive!(UInt8Type); -impl_from_arrow_primitive!(UInt16Type); -impl_from_arrow_primitive!(UInt32Type); -impl_from_arrow_primitive!(UInt64Type); -impl_from_arrow_primitive!(Float16Type); -impl_from_arrow_primitive!(Float32Type); -impl_from_arrow_primitive!(Float64Type); - /// Zero-copy conversion of an Arrow `Decimal32` array into a Vortex decimal array. pub fn from_arrow_decimal32( array: &ArrowPrimitiveArray, @@ -212,15 +187,6 @@ pub fn from_arrow_decimal32( Ok(DecimalArray::new(buffer, decimal_type, validity).into_array()) } -impl FromArrowArray<&ArrowPrimitiveArray> for ArrayRef { - fn from_arrow( - array: &ArrowPrimitiveArray, - nullable: bool, - ) -> VortexResult { - from_arrow_decimal32(array, nullable) - } -} - /// Zero-copy conversion of an Arrow `Decimal64` array into a Vortex decimal array. pub fn from_arrow_decimal64( array: &ArrowPrimitiveArray, @@ -232,15 +198,6 @@ pub fn from_arrow_decimal64( Ok(DecimalArray::new(buffer, decimal_type, validity).into_array()) } -impl FromArrowArray<&ArrowPrimitiveArray> for ArrayRef { - fn from_arrow( - array: &ArrowPrimitiveArray, - nullable: bool, - ) -> VortexResult { - from_arrow_decimal64(array, nullable) - } -} - /// Zero-copy conversion of an Arrow `Decimal128` array into a Vortex decimal array. pub fn from_arrow_decimal128( array: &ArrowPrimitiveArray, @@ -252,15 +209,6 @@ pub fn from_arrow_decimal128( Ok(DecimalArray::new(buffer, decimal_type, validity).into_array()) } -impl FromArrowArray<&ArrowPrimitiveArray> for ArrayRef { - fn from_arrow( - array: &ArrowPrimitiveArray, - nullable: bool, - ) -> VortexResult { - from_arrow_decimal128(array, nullable) - } -} - /// Zero-copy conversion of an Arrow `Decimal256` array into a Vortex decimal array. pub fn from_arrow_decimal256( array: &ArrowPrimitiveArray, @@ -276,44 +224,6 @@ pub fn from_arrow_decimal256( Ok(DecimalArray::new(buffer, decimal_type, validity).into_array()) } -impl FromArrowArray<&ArrowPrimitiveArray> for ArrayRef { - fn from_arrow( - array: &ArrowPrimitiveArray, - nullable: bool, - ) -> VortexResult { - from_arrow_decimal256(array, nullable) - } -} - -macro_rules! impl_from_arrow_temporal { - ($T:path) => { - impl FromArrowArray<&ArrowPrimitiveArray<$T>> for ArrayRef { - fn from_arrow( - value: &ArrowPrimitiveArray<$T>, - nullable: bool, - ) -> vortex_error::VortexResult { - from_arrow_temporal(value, nullable) - } - } - }; -} - -// timestamp -impl_from_arrow_temporal!(TimestampSecondType); -impl_from_arrow_temporal!(TimestampMillisecondType); -impl_from_arrow_temporal!(TimestampMicrosecondType); -impl_from_arrow_temporal!(TimestampNanosecondType); - -// time -impl_from_arrow_temporal!(Time32SecondType); -impl_from_arrow_temporal!(Time32MillisecondType); -impl_from_arrow_temporal!(Time64MicrosecondType); -impl_from_arrow_temporal!(Time64NanosecondType); - -// date -impl_from_arrow_temporal!(Date32Type); -impl_from_arrow_temporal!(Date64Type); - /// Conversion of an Arrow temporal array (timestamp/date/time) into a Vortex temporal /// extension array. pub fn from_arrow_temporal( @@ -394,15 +304,6 @@ where .into_array()) } -impl FromArrowArray<&GenericByteArray> for ArrayRef -where - ::Offset: IntegerPType, -{ - fn from_arrow(value: &GenericByteArray, nullable: bool) -> VortexResult { - from_arrow_bytes(value, nullable) - } -} - /// Zero-copy conversion of an Arrow string/binary view array into a Vortex `VarBinView` array. pub fn from_arrow_byte_view( value: &GenericByteViewArray, @@ -437,12 +338,6 @@ pub fn from_arrow_byte_view( }) } -impl FromArrowArray<&GenericByteViewArray> for ArrayRef { - fn from_arrow(value: &GenericByteViewArray, nullable: bool) -> VortexResult { - from_arrow_byte_view(value, nullable) - } -} - /// Zero-copy conversion of an Arrow boolean array into a Vortex `Bool` array. pub fn from_arrow_boolean(value: &ArrowBooleanArray, nullable: bool) -> VortexResult { Ok(BoolArray::new( @@ -452,12 +347,6 @@ pub fn from_arrow_boolean(value: &ArrowBooleanArray, nullable: bool) -> VortexRe .into_array()) } -impl FromArrowArray<&ArrowBooleanArray> for ArrayRef { - fn from_arrow(value: &ArrowBooleanArray, nullable: bool) -> VortexResult { - from_arrow_boolean(value, nullable) - } -} - /// Strip out the nulls from this array and return a new array without nulls. pub(crate) fn remove_nulls(data: arrow_data::ArrayData) -> VortexResult { if data.null_count() == 0 { @@ -531,12 +420,6 @@ pub fn from_arrow_struct(value: &ArrowStructArray, nullable: bool) -> VortexResu .into_array()) } -impl FromArrowArray<&ArrowStructArray> for ArrayRef { - fn from_arrow(value: &ArrowStructArray, nullable: bool) -> VortexResult { - from_arrow_struct(value, nullable) - } -} - /// Conversion of an Arrow (large) list array into a Vortex `List` array. pub fn from_arrow_list( value: &GenericListArray, @@ -565,12 +448,6 @@ pub fn from_arrow_list( Ok(unsafe { ListArray::new_unchecked(elements, offsets.into_array(), nulls) }.into_array()) } -impl FromArrowArray<&GenericListArray> for ArrayRef { - fn from_arrow(value: &GenericListArray, nullable: bool) -> VortexResult { - from_arrow_list(value, nullable) - } -} - /// Conversion of an Arrow (large) list-view array into a Vortex `ListView` array. pub fn from_arrow_list_view( array: &GenericListViewArray, @@ -595,12 +472,6 @@ pub fn from_arrow_list_view( Ok(unsafe { ListViewArray::new_unchecked(elements, offsets, sizes, nulls) }.into_array()) } -impl FromArrowArray<&GenericListViewArray> for ArrayRef { - fn from_arrow(array: &GenericListViewArray, nullable: bool) -> VortexResult { - from_arrow_list_view(array, nullable) - } -} - /// Conversion of an Arrow fixed-size list array into a Vortex `FixedSizeList` array. pub fn from_arrow_fixed_size_list( array: &ArrowFixedSizeListArray, @@ -619,12 +490,6 @@ pub fn from_arrow_fixed_size_list( .into_array()) } -impl FromArrowArray<&ArrowFixedSizeListArray> for ArrayRef { - fn from_arrow(array: &ArrowFixedSizeListArray, nullable: bool) -> VortexResult { - from_arrow_fixed_size_list(array, nullable) - } -} - pub(crate) fn map_from_arrow_parts( entries: ArrayRef, offsets: &OffsetBuffer, @@ -698,12 +563,6 @@ pub fn from_arrow_map(array: &ArrowMapArray, nullable: bool) -> VortexResult for ArrayRef { - fn from_arrow(array: &ArrowMapArray, nullable: bool) -> VortexResult { - from_arrow_map(array, nullable) - } -} - /// Conversion of an Arrow null array into a Vortex `Null` array. pub fn from_arrow_null(value: &ArrowNullArray, nullable: bool) -> VortexResult { vortex_ensure!( @@ -713,12 +572,6 @@ pub fn from_arrow_null(value: &ArrowNullArray, nullable: bool) -> VortexResult for ArrayRef { - fn from_arrow(value: &ArrowNullArray, nullable: bool) -> VortexResult { - from_arrow_null(value, nullable) - } -} - /// Conversion of an Arrow dictionary array into a Vortex `Dict` array. pub fn from_arrow_dictionary( array: &DictionaryArray, @@ -731,12 +584,6 @@ pub fn from_arrow_dictionary( Ok(unsafe { DictArray::new_unchecked(keys, values) }) } -impl FromArrowArray<&DictionaryArray> for DictArray { - fn from_arrow(array: &DictionaryArray, nullable: bool) -> VortexResult { - from_arrow_dictionary(array, nullable) - } -} - /// Convert an Arrow [`NullBuffer`] into a Vortex [`Validity`] for an array of the given /// nullability. /// @@ -890,29 +737,11 @@ pub fn from_arrow_dyn(array: &dyn ArrowArray, nullable: bool) -> VortexResult for ArrayRef { - fn from_arrow(array: &dyn ArrowArray, nullable: bool) -> VortexResult { - from_arrow_dyn(array, nullable) - } -} - /// Canonical conversion of an Arrow [`RecordBatch`] into a Vortex struct array. pub fn from_arrow_batch(batch: &RecordBatch, nullable: bool) -> VortexResult { from_arrow_struct(&arrow_array::StructArray::from(batch.clone()), nullable) } -impl FromArrowArray for ArrayRef { - fn from_arrow(array: RecordBatch, nullable: bool) -> VortexResult { - from_arrow_batch(&array, nullable) - } -} - -impl FromArrowArray<&RecordBatch> for ArrayRef { - fn from_arrow(array: &RecordBatch, nullable: bool) -> VortexResult { - from_arrow_batch(array, nullable) - } -} - #[cfg(test)] mod tests { use std::ops::Range; @@ -971,7 +800,6 @@ mod tests { use arrow_schema::Fields; use arrow_schema::Schema; use rstest::rstest; - use vortex_array::ArrayRef; use vortex_array::IntoArray; use vortex_array::VortexSessionExecute as _; use vortex_array::array_session; @@ -1010,7 +838,6 @@ mod tests { use vortex_buffer::buffer; use vortex_error::VortexResult; - use crate::FromArrowArray as _; use crate::IntoVortexArray as _; use crate::convert::from_arrow_batch; use crate::convert::from_arrow_dyn; @@ -1071,8 +898,8 @@ mod tests { #[case] non_nullable: Arc, #[case] expected_ptype: PType, ) { - let v_null = ArrayRef::from_arrow(nullable.as_ref(), true).unwrap(); - let v_non_null = ArrayRef::from_arrow(non_nullable.as_ref(), false).unwrap(); + let v_null = from_arrow_dyn(nullable.as_ref(), true).unwrap(); + let v_non_null = from_arrow_dyn(non_nullable.as_ref(), false).unwrap(); assert_eq!(v_null.len(), 4); assert_eq!(v_non_null.len(), 4); assert_eq!(v_null.as_::().ptype(), expected_ptype); @@ -1140,7 +967,7 @@ mod tests { Some(::Native::from_f32(3.5)), ]; let arrow_array = arrow_array::PrimitiveArray::::from(values); - let vortex_array = ArrayRef::from_arrow(&arrow_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, true).unwrap(); let non_null_values = vec![ ::Native::from_f32(1.5), @@ -1148,7 +975,7 @@ mod tests { ]; let arrow_array_non_null = arrow_array::PrimitiveArray::::from(non_null_values); - let vortex_array_non_null = ArrayRef::from_arrow(&arrow_array_non_null, false).unwrap(); + let vortex_array_non_null = from_arrow_dyn(&arrow_array_non_null, false).unwrap(); assert_eq!(vortex_array.len(), 3); assert_eq!(vortex_array_non_null.len(), 2); @@ -1188,7 +1015,7 @@ mod tests { builder.append_value(11111); let decimal_array = builder.finish().with_precision_and_scale(10, 2).unwrap(); - let vortex_array = ArrayRef::from_arrow(&decimal_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&decimal_array, true).unwrap(); assert_eq!(vortex_array.len(), 4); let mut builder_non_null = Decimal128Builder::with_capacity(3); @@ -1200,7 +1027,7 @@ mod tests { .with_precision_and_scale(10, 2) .unwrap(); - let vortex_array_non_null = ArrayRef::from_arrow(&decimal_array_non_null, false).unwrap(); + let vortex_array_non_null = from_arrow_dyn(&decimal_array_non_null, false).unwrap(); assert_eq!(vortex_array_non_null.len(), 3); // Verify metadata - should be DecimalArray with correct precision and scale @@ -1238,7 +1065,7 @@ mod tests { builder.append_value(arrow_buffer::i256::from_i128(11111)); let decimal_array = builder.finish().with_precision_and_scale(38, 10).unwrap(); - let vortex_array = ArrayRef::from_arrow(&decimal_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&decimal_array, true).unwrap(); assert_eq!(vortex_array.len(), 4); let mut builder_non_null = Decimal256Builder::with_capacity(3); @@ -1250,7 +1077,7 @@ mod tests { .with_precision_and_scale(38, 10) .unwrap(); - let vortex_array_non_null = ArrayRef::from_arrow(&decimal_array_non_null, false).unwrap(); + let vortex_array_non_null = from_arrow_dyn(&decimal_array_non_null, false).unwrap(); assert_eq!(vortex_array_non_null.len(), 3); // Verify metadata - should be DecimalArray with correct precision and scale @@ -1339,8 +1166,8 @@ mod tests { #[case] nullable: Arc, #[case] non_nullable: Arc, ) { - let v_null = ArrayRef::from_arrow(nullable.as_ref(), true).unwrap(); - let v_non_null = ArrayRef::from_arrow(non_nullable.as_ref(), false).unwrap(); + let v_null = from_arrow_dyn(nullable.as_ref(), true).unwrap(); + let v_non_null = from_arrow_dyn(non_nullable.as_ref(), false).unwrap(); assert_eq!(v_null.len(), 4); assert_eq!(v_non_null.len(), 4); let expected = |nullable: bool| { @@ -1433,11 +1260,11 @@ mod tests { let arrow_array = TimestampMicrosecondArray::from(vec![Some(1000), None, Some(3000), Some(4000)]) .with_timezone("UTC"); - let vortex_array = ArrayRef::from_arrow(&arrow_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, true).unwrap(); let arrow_array_non_null = TimestampMicrosecondArray::from(vec![1000_i64, 2000, 3000, 4000]).with_timezone("UTC"); - let vortex_array_non_null = ArrayRef::from_arrow(&arrow_array_non_null, false).unwrap(); + let vortex_array_non_null = from_arrow_dyn(&arrow_array_non_null, false).unwrap(); assert_eq!(vortex_array.len(), 4); assert_eq!( @@ -1519,8 +1346,8 @@ mod tests { #[case] non_nullable: Arc, #[case] expected_non_nullable_dtype: DType, ) { - let v_null = ArrayRef::from_arrow(nullable.as_ref(), true).unwrap(); - let v_non_null = ArrayRef::from_arrow(non_nullable.as_ref(), false).unwrap(); + let v_null = from_arrow_dyn(nullable.as_ref(), true).unwrap(); + let v_non_null = from_arrow_dyn(non_nullable.as_ref(), false).unwrap(); assert_eq!(v_null.len(), 4); assert_eq!(v_non_null.len(), 4); assert_eq!(v_null.dtype(), &expected_non_nullable_dtype.as_nullable()); @@ -1551,7 +1378,7 @@ mod tests { builder.append_value("world"); builder.append_value("test"); let arrow_array = builder.finish(); - let vortex_array = ArrayRef::from_arrow(&arrow_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, true).unwrap(); let mut builder_non_null = StringViewBuilder::new(); builder_non_null.append_value("hello"); @@ -1559,7 +1386,7 @@ mod tests { builder_non_null.append_value("test"); builder_non_null.append_value("vortex"); let arrow_array_non_null = builder_non_null.finish(); - let vortex_array_non_null = ArrayRef::from_arrow(&arrow_array_non_null, false).unwrap(); + let vortex_array_non_null = from_arrow_dyn(&arrow_array_non_null, false).unwrap(); assert_eq!(vortex_array.len(), 4); assert_eq!(vortex_array_non_null.len(), 4); @@ -1607,7 +1434,7 @@ mod tests { builder.append_value(b"world"); builder.append_value(b"test"); let arrow_array = builder.finish(); - let vortex_array = ArrayRef::from_arrow(&arrow_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, true).unwrap(); let mut builder_non_null = BinaryViewBuilder::new(); builder_non_null.append_value(b"hello"); @@ -1615,7 +1442,7 @@ mod tests { builder_non_null.append_value(b"test"); builder_non_null.append_value(b"vortex"); let arrow_array_non_null = builder_non_null.finish(); - let vortex_array_non_null = ArrayRef::from_arrow(&arrow_array_non_null, false).unwrap(); + let vortex_array_non_null = from_arrow_dyn(&arrow_array_non_null, false).unwrap(); assert_eq!(vortex_array.len(), 4); assert_eq!(vortex_array_non_null.len(), 4); @@ -1659,10 +1486,10 @@ mod tests { #[test] fn test_boolean_array_conversion() { let arrow_array = BooleanArray::from(vec![Some(true), None, Some(false), Some(true)]); - let vortex_array = ArrayRef::from_arrow(&arrow_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, true).unwrap(); let arrow_array_non_null = BooleanArray::from(vec![true, false, true, false]); - let vortex_array_non_null = ArrayRef::from_arrow(&arrow_array_non_null, false).unwrap(); + let vortex_array_non_null = from_arrow_dyn(&arrow_array_non_null, false).unwrap(); assert_eq!(vortex_array.len(), 4); assert_eq!(vortex_array_non_null.len(), 4); @@ -1696,7 +1523,7 @@ mod tests { None, ); - let vortex_array = ArrayRef::from_arrow(&arrow_array, false).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, false).unwrap(); assert_eq!(vortex_array.len(), 3); // Verify metadata - should be StructArray with correct field names @@ -1717,7 +1544,7 @@ mod tests { ]))), ); - let vortex_nullable_array = ArrayRef::from_arrow(&nullable_array, true).unwrap(); + let vortex_nullable_array = from_arrow_dyn(&nullable_array, true).unwrap(); assert_eq!(vortex_nullable_array.len(), 3); // Verify metadata for nullable struct @@ -1762,7 +1589,7 @@ mod tests { builder.append_value([Some(4), Some(5)]); let arrow_array = builder.finish(); - let vortex_array = ArrayRef::from_arrow(&arrow_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, true).unwrap(); assert_eq!(vortex_array.len(), 3); // Verify metadata - should be ListArray with correct offsets @@ -1777,7 +1604,7 @@ mod tests { builder_non_null.append_value([Some(4), Some(5)]); let arrow_array_non_null = builder_non_null.finish(); - let vortex_array_non_null = ArrayRef::from_arrow(&arrow_array_non_null, false).unwrap(); + let vortex_array_non_null = from_arrow_dyn(&arrow_array_non_null, false).unwrap(); assert_eq!(vortex_array_non_null.len(), 2); // Verify metadata for non-nullable list @@ -1828,8 +1655,8 @@ mod tests { let mut ctx = array_session().create_execution_ctx(); // A slice keeps its parent's whole child; the import must not. - let sliced = ArrayRef::from_arrow(&strings(0..1000).slice(500, 2), false)?; - let fresh = ArrayRef::from_arrow(&strings(500..502), false)?; + let sliced = from_arrow_dyn(&strings(0..1000).slice(500, 2), false)?; + let fresh = from_arrow_dyn(&strings(500..502), false)?; assert_eq!(sliced.nbytes(), fresh.nbytes()); let expected = VarBinArray::from_iter_nonnull( ["value-500", "value-501"], @@ -1838,8 +1665,8 @@ mod tests { assert_arrays_eq!(sliced, expected, &mut ctx); assert_arrays_eq!(fresh, expected, &mut ctx); - let sliced = ArrayRef::from_arrow(&lists(0..1000).slice(500, 2), false)?; - let fresh = ArrayRef::from_arrow(&lists(500..502), false)?; + let sliced = from_arrow_dyn(&lists(0..1000).slice(500, 2), false)?; + let fresh = from_arrow_dyn(&lists(500..502), false)?; assert_eq!(sliced.nbytes(), fresh.nbytes()); let expected = ListArray::new( PrimitiveArray::from_option_iter([Some(500i64), Some(501), Some(501), Some(502)]) @@ -1862,7 +1689,7 @@ mod tests { builder.append_value([Some(4), Some(5)]); let arrow_array = builder.finish(); - let vortex_array = ArrayRef::from_arrow(&arrow_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, true).unwrap(); assert_eq!(vortex_array.len(), 3); // Verify metadata - should be ListArray with correct offsets (I64 for large lists) @@ -1877,7 +1704,7 @@ mod tests { builder_non_null.append_value([Some(4), Some(5)]); let arrow_array_non_null = builder_non_null.finish(); - let vortex_array_non_null = ArrayRef::from_arrow(&arrow_array_non_null, false).unwrap(); + let vortex_array_non_null = from_arrow_dyn(&arrow_array_non_null, false).unwrap(); assert_eq!(vortex_array_non_null.len(), 2); // Verify metadata for non-nullable large list @@ -1936,7 +1763,7 @@ mod tests { let arrow_array = ArrowFixedSizeListArray::try_new(Arc::clone(&field), 3, Arc::new(values), None) .unwrap(); - let vortex_array = ArrayRef::from_arrow(&arrow_array, false).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, false).unwrap(); assert_eq!(vortex_array.len(), 4); @@ -1969,7 +1796,7 @@ mod tests { Some(null_buffer), ) .unwrap(); - let vortex_array_nullable = ArrayRef::from_arrow(&arrow_array_nullable, true).unwrap(); + let vortex_array_nullable = from_arrow_dyn(&arrow_array_nullable, true).unwrap(); assert_eq!(vortex_array_nullable.len(), 3); @@ -2055,7 +1882,7 @@ mod tests { ) .unwrap(); - let vortex_array = ArrayRef::from_arrow(&arrow_array, false).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, false).unwrap(); assert_eq!(vortex_array.len(), 4); // Verify metadata - should be ListViewArray with correct offsets and sizes @@ -2081,7 +1908,7 @@ mod tests { ) .unwrap(); - let vortex_array_nullable = ArrayRef::from_arrow(&arrow_array_nullable, true).unwrap(); + let vortex_array_nullable = from_arrow_dyn(&arrow_array_nullable, true).unwrap(); assert_eq!(vortex_array_nullable.len(), 4); // Test LargeListView (i64 offsets and sizes) @@ -2097,7 +1924,7 @@ mod tests { ) .unwrap(); - let large_vortex_array = ArrayRef::from_arrow(&large_arrow_array, false).unwrap(); + let large_vortex_array = from_arrow_dyn(&large_arrow_array, false).unwrap(); assert_eq!(large_vortex_array.len(), 4); // Verify metadata for large ListView @@ -2139,7 +1966,7 @@ mod tests { #[test] fn test_null_array_conversion() { let arrow_array = NullArray::new(5); - let vortex_array = ArrayRef::from_arrow(&arrow_array, true).unwrap(); + let vortex_array = from_arrow_dyn(&arrow_array, true).unwrap(); assert_eq!(vortex_array.len(), 5); assert_arrays_eq!( vortex_array, @@ -2214,7 +2041,7 @@ mod tests { let record_batch = RecordBatch::try_new(schema, vec![field1_data, field2_data]).unwrap(); - let vortex_array = ArrayRef::from_arrow(record_batch, false).unwrap(); + let vortex_array = from_arrow_batch(&record_batch, false).unwrap(); let expected = VortexStructArray::new( ["field1", "field2"].into(), vec![ @@ -2245,7 +2072,7 @@ mod tests { let record_batch = RecordBatch::try_new(schema, vec![field1_data, field2_data]).unwrap(); - let vortex_array = ArrayRef::from_arrow(&record_batch, false).unwrap(); + let vortex_array = from_arrow_batch(&record_batch, false).unwrap(); let expected = VortexStructArray::new( ["field1", "field2"].into(), vec![ @@ -2271,7 +2098,7 @@ mod tests { fn test_dyn_array_conversion() { let int_array = Int32Array::from(vec![1, 2, 3, 4]); let dyn_array: &dyn ArrowArray = &int_array; - let vortex_array = ArrayRef::from_arrow(dyn_array, false).unwrap(); + let vortex_array = from_arrow_dyn(dyn_array, false).unwrap(); assert_arrays_eq!( vortex_array, buffer![1i32, 2, 3, 4].into_array(), @@ -2280,7 +2107,7 @@ mod tests { let string_array = StringArray::from(vec!["a", "b", "c"]); let dyn_array: &dyn ArrowArray = &string_array; - let vortex_array = ArrayRef::from_arrow(dyn_array, false).unwrap(); + let vortex_array = from_arrow_dyn(dyn_array, false).unwrap(); assert_arrays_eq!( vortex_array, VarBinArray::from_iter_nonnull(["a", "b", "c"], DType::Utf8(Nullability::NonNullable)), @@ -2289,7 +2116,7 @@ mod tests { let bool_array = BooleanArray::from(vec![true, false, true]); let dyn_array: &dyn ArrowArray = &bool_array; - let vortex_array = ArrayRef::from_arrow(dyn_array, false).unwrap(); + let vortex_array = from_arrow_dyn(dyn_array, false).unwrap(); assert_arrays_eq!( vortex_array, BoolArray::from_iter([true, false, true]), @@ -2309,7 +2136,7 @@ mod tests { 1, ); let actual = - ArrayRef::from_arrow(null_struct_array_with_non_nullable_field.as_ref(), true).unwrap(); + from_arrow_dyn(null_struct_array_with_non_nullable_field.as_ref(), true).unwrap(); assert_arrays_eq!( actual, VortexStructArray::new( @@ -2337,7 +2164,7 @@ mod tests { 1, ); let actual = - ArrayRef::from_arrow(null_struct_array_with_non_nullable_field.as_ref(), true).unwrap(); + from_arrow_dyn(null_struct_array_with_non_nullable_field.as_ref(), true).unwrap(); assert_arrays_eq!( actual, VortexStructArray::new( @@ -2414,7 +2241,7 @@ mod tests { // Requesting `nullable = false` on an Arrow array that physically contains nulls is a // contradiction and must surface as an error, not a panic. let arrow_array = Int32Array::from(vec![Some(1), None, Some(3)]); - assert!(ArrayRef::from_arrow(&arrow_array, false).is_err()); + assert!(from_arrow_dyn(&arrow_array, false).is_err()); } #[test] @@ -2422,7 +2249,7 @@ mod tests { // An Arrow NullArray is entirely null, so it cannot be converted to a non-nullable // Vortex array. let arrow_array = NullArray::new(5); - assert!(ArrayRef::from_arrow(&arrow_array, false).is_err()); + assert!(from_arrow_dyn(&arrow_array, false).is_err()); } #[test] @@ -2433,7 +2260,7 @@ mod tests { &DataType::Struct(Fields::from(vec![Field::new("a", DataType::Int32, true)])), 3, ); - assert!(ArrayRef::from_arrow(struct_array.as_ref(), false).is_err()); + assert!(from_arrow_dyn(struct_array.as_ref(), false).is_err()); } #[test] @@ -2444,7 +2271,7 @@ mod tests { builder.append_value([Some(1), Some(2)]); builder.append_null(); let list = builder.finish(); - assert!(ArrayRef::from_arrow(&list, false).is_err()); + assert!(from_arrow_dyn(&list, false).is_err()); } #[test] @@ -2461,8 +2288,6 @@ mod tests { 1, ); - assert!( - ArrayRef::from_arrow(null_struct_array_with_non_nullable_field.as_ref(), true).is_err() - ); + assert!(from_arrow_dyn(null_struct_array_with_non_nullable_field.as_ref(), true).is_err()); } } diff --git a/vortex-arrow/src/datum.rs b/vortex-arrow/src/datum.rs index 953a9d8d714..d501ff1d920 100644 --- a/vortex-arrow/src/datum.rs +++ b/vortex-arrow/src/datum.rs @@ -9,17 +9,12 @@ use arrow_schema::Field; use vortex_array::ArrayRef; use vortex_array::ExecutionCtx; use vortex_array::IntoArray; -use vortex_array::VortexSessionExecute; use vortex_array::arrays::Constant; use vortex_array::arrays::ConstantArray; -use vortex_array::legacy_session; -use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_panic; use crate::ArrowSessionExt; -#[allow(deprecated)] -use crate::FromArrowArray; /// A wrapper around a generic Arrow array that can be used as a Datum in Arrow compute. #[derive(Debug)] @@ -99,45 +94,6 @@ impl ArrowDatum for Datum { } } -/// Convert an Arrow array to an Array with a specific length. -/// This is useful for compute functions that delegate to Arrow using [Datum], -/// which will return a scalar (length 1 Arrow array) if the input array is constant. -/// -/// # Error -/// -/// The provided array must have length -#[deprecated( - note = "Relies on the hidden global `legacy_session()`; use `from_arrow_columnar` with an explicit `ExecutionCtx` instead" -)] -#[allow(clippy::disallowed_methods)] -#[allow(deprecated)] -pub fn from_arrow_array_with_len(array: A, len: usize, nullable: bool) -> VortexResult -where - ArrayRef: FromArrowArray, -{ - let array = ArrayRef::from_arrow(array, nullable)?; - if array.len() == len { - return Ok(array); - } - - if array.len() != 1 { - vortex_panic!( - "Array length mismatch, expected {} got {} for encoding {}", - len, - array.len(), - array.encoding_id() - ); - } - - Ok(ConstantArray::new( - array - .execute_scalar(0, &mut legacy_session().create_execution_ctx()) - .vortex_expect("array of length 1 must support execute_scalar(0)"), - len, - ) - .into_array()) -} - /// Convert an Arrow array to an Array with a specific length, using the provided /// [`ExecutionCtx`]. /// diff --git a/vortex-arrow/src/dtype.rs b/vortex-arrow/src/dtype.rs index 860d56113da..9fb33b8ebba 100644 --- a/vortex-arrow/src/dtype.rs +++ b/vortex-arrow/src/dtype.rs @@ -9,9 +9,8 @@ //! The conversions in this module are "naive": every logical type is encoded in its simplest //! corresponding Arrow type, and Arrow extension types (other than the builtin temporal types and //! Parquet Variant) are not understood. The authoritative, plugin-aware conversion entry point is -//! [`ArrowSession`](crate::ArrowSession) — prefer its `to_arrow_schema` / `to_arrow_datatype` / -//! `from_arrow_schema` / `from_arrow_field` / `from_arrow_datatype` methods over the deprecated -//! traits in this module. +//! [`ArrowSession`](crate::ArrowSession), through its `to_arrow_schema` / `to_arrow_datatype` / +//! `from_arrow_schema` / `from_arrow_field` / `from_arrow_datatype` methods. //! //! For this reason, it's recommended to do as much computation as possible within Vortex, and then //! materialize an Arrow ArrayRef at the very end of the processing chain. @@ -22,9 +21,6 @@ use arrow_schema::DataType; use arrow_schema::Field; use arrow_schema::FieldRef; use arrow_schema::Fields; -use arrow_schema::Schema; -use arrow_schema::SchemaBuilder; -use arrow_schema::SchemaRef; use arrow_schema::TimeUnit as ArrowTimeUnit; use vortex_array::dtype::DType; use vortex_array::dtype::DecimalDType; @@ -38,7 +34,6 @@ use vortex_array::extension::datetime::TemporalMetadata; use vortex_array::extension::datetime::Time; use vortex_array::extension::datetime::TimeUnit; use vortex_array::extension::datetime::Timestamp; -use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; use vortex_error::vortex_ensure; @@ -46,54 +41,6 @@ use vortex_error::vortex_ensure_eq; use vortex_error::vortex_err; use vortex_error::vortex_panic; -/// Trait for converting Arrow types to Vortex types. -#[deprecated( - note = "Use `ArrowSession` (`from_arrow_schema`, `from_arrow_field`, `from_arrow_datatype`) instead" -)] -pub trait FromArrowType: Sized { - /// Convert the Arrow type to a Vortex type. - #[deprecated( - note = "Use `ArrowSession` (`from_arrow_schema`, `from_arrow_field`, `from_arrow_datatype`) instead" - )] - fn from_arrow(value: T) -> Self; -} - -/// Trait for converting Arrow types to Vortex types. -#[deprecated( - note = "Use `ArrowSession` (`from_arrow_schema`, `from_arrow_field`, `from_arrow_datatype`) instead" -)] -pub trait TryFromArrowType: Sized { - /// Convert the Arrow type to a Vortex type. - #[deprecated( - note = "Use `ArrowSession` (`from_arrow_schema`, `from_arrow_field`, `from_arrow_datatype`) instead" - )] - fn try_from_arrow(value: T) -> VortexResult; -} - -/// Extension trait converting Vortex [`DType`]s into Arrow schemas and data types. -/// -/// This mirrors inherent methods that lived on [`DType`] before Arrow interoperability moved -/// into this crate. -#[deprecated(note = "Use `ArrowSession::to_arrow_schema` / `to_arrow_datatype` instead")] -pub trait ToArrowType { - /// Convert a Vortex [`DType`] into an Arrow [`Schema`]. - /// - /// This method is not plugin-aware and strips any `ARROW:extension:name` metadata for - /// non-builtin extensions (only `arrow.parquet.variant` is special-cased here). Use the - /// session method when you need round-trippable extension metadata. - #[deprecated(note = "Use `ArrowSession::to_arrow_schema` instead")] - fn to_arrow_schema(&self) -> VortexResult; - - /// Returns the Arrow [`DataType`] that best corresponds to this Vortex [`DType`]. - /// - /// This method has no awareness of registered Arrow extension plugins, so any - /// [`DType::Extension`] outside the builtin temporal set will fail or silently lose its - /// `ARROW:extension:name` metadata. The session methods recurse through containers - /// and dispatch plugins at every extension node. - #[deprecated(note = "Use `ArrowSession::to_arrow_datatype` instead")] - fn to_arrow_dtype(&self) -> VortexResult; -} - /// Convert an Arrow [`ArrowTimeUnit`] to a Vortex [`TimeUnit`]. pub(crate) fn from_arrow_time_unit(value: ArrowTimeUnit) -> TimeUnit { match value { @@ -140,21 +87,6 @@ pub(crate) fn ptype_from_arrow(value: &DataType) -> VortexResult { } } -/// Naive conversion of an Arrow decimal [`DataType`] to a [`DecimalDType`]. -pub(crate) fn decimal_dtype_from_arrow(value: &DataType) -> VortexResult { - match value { - DataType::Decimal32(precision, scale) - | DataType::Decimal64(precision, scale) - | DataType::Decimal128(precision, scale) - | DataType::Decimal256(precision, scale) => DecimalDType::try_new(*precision, *scale), - - _ => Err(vortex_err!( - "Arrow datatype {:?} cannot be converted to DecimalDType", - value - )), - } -} - /// Naive conversion of an Arrow [`DataType`] to a Vortex [`DType`], with no awareness of Arrow /// extension plugins. pub(crate) fn from_arrow_data_type( @@ -269,49 +201,6 @@ pub(crate) fn from_arrow_fields_naive(fields: &Fields) -> VortexResult>() } -/// Naive conversion of an Arrow [`Schema`] to a Vortex top-level non-nullable struct [`DType`]. -pub(crate) fn from_arrow_schema_naive(schema: &Schema) -> VortexResult { - Ok(DType::Struct( - from_arrow_fields_naive(schema.fields())?, - Nullability::NonNullable, // Must match From for Array - )) -} - -/// Naive conversion of a Vortex top-level struct [`DType`] to an Arrow [`Schema`], with the -/// builtin `arrow.parquet.variant` special-case. -fn to_arrow_schema_naive(dtype: &DType) -> VortexResult { - let DType::Struct(struct_dtype, nullable) = dtype else { - vortex_bail!("only DType::Struct can be converted to arrow schema"); - }; - - if *nullable != Nullability::NonNullable { - vortex_bail!("top-level struct in Schema must be NonNullable"); - } - - let mut builder = SchemaBuilder::with_capacity(struct_dtype.names().len()); - for (field_name, field_dtype) in struct_dtype.names().iter().zip(struct_dtype.fields()) { - let field = if field_dtype.is_variant() { - let storage = DataType::Struct(variant_storage_fields_minimal()); - Field::new(field_name.as_ref(), storage, field_dtype.is_nullable()).with_metadata( - [( - "ARROW:extension:name".to_owned(), - "arrow.parquet.variant".to_owned(), - )] - .into(), - ) - } else { - Field::new( - field_name.as_ref(), - to_data_type_naive(&field_dtype)?, - field_dtype.is_nullable(), - ) - }; - builder.push(field); - } - - Ok(builder.finish()) -} - /// Naive conversion from a Vortex `DType` to the nearest Arrow physical data type. pub(crate) fn to_data_type_naive(dtype: &DType) -> VortexResult { Ok(match dtype { @@ -423,118 +312,8 @@ pub(crate) fn to_data_type_naive(dtype: &DType) -> VortexResult { }) } -fn variant_storage_fields_minimal() -> Fields { - Fields::from(vec![ - Field::new("metadata", DataType::Binary, false), - Field::new("value", DataType::Binary, true), - ]) -} - -/// Impls of the deprecated conversion traits, delegating to the naive free functions above. -#[allow(deprecated)] -mod deprecated_impls { - use super::*; - - impl TryFromArrowType<&DataType> for PType { - fn try_from_arrow(value: &DataType) -> VortexResult { - ptype_from_arrow(value) - } - } - - impl TryFromArrowType<&DataType> for DecimalDType { - fn try_from_arrow(value: &DataType) -> VortexResult { - decimal_dtype_from_arrow(value) - } - } - - impl FromArrowType<&ArrowTimeUnit> for TimeUnit { - fn from_arrow(value: &ArrowTimeUnit) -> Self { - from_arrow_time_unit(*value) - } - } - - impl FromArrowType for TimeUnit { - fn from_arrow(value: ArrowTimeUnit) -> Self { - from_arrow_time_unit(value) - } - } - - impl FromArrowType for DType { - fn from_arrow(value: SchemaRef) -> Self { - Self::from_arrow(value.as_ref()) - } - } - - impl TryFromArrowType for DType { - fn try_from_arrow(value: SchemaRef) -> VortexResult { - from_arrow_schema_naive(value.as_ref()) - } - } - - impl FromArrowType<&Schema> for DType { - fn from_arrow(value: &Schema) -> Self { - from_arrow_schema_naive(value).vortex_expect("arrow schema to dtype") - } - } - - impl TryFromArrowType<&Schema> for DType { - fn try_from_arrow(value: &Schema) -> VortexResult { - from_arrow_schema_naive(value) - } - } - - impl FromArrowType<&Fields> for StructFields { - fn from_arrow(value: &Fields) -> Self { - from_arrow_fields_naive(value).vortex_expect("arrow fields to struct fields") - } - } - - impl TryFromArrowType<&Fields> for StructFields { - fn try_from_arrow(value: &Fields) -> VortexResult { - from_arrow_fields_naive(value) - } - } - - impl FromArrowType<(&DataType, Nullability)> for DType { - fn from_arrow(value: (&DataType, Nullability)) -> Self { - from_arrow_data_type(value.0, value.1).vortex_expect("arrow data type to dtype") - } - } - - impl TryFromArrowType<(&DataType, Nullability)> for DType { - fn try_from_arrow( - (data_type, nullability): (&DataType, Nullability), - ) -> VortexResult { - from_arrow_data_type(data_type, nullability) - } - } - - impl FromArrowType<&Field> for DType { - fn from_arrow(field: &Field) -> Self { - from_arrow_field_naive(field).vortex_expect("arrow field to dtype") - } - } - - impl TryFromArrowType<&Field> for DType { - fn try_from_arrow(field: &Field) -> VortexResult { - from_arrow_field_naive(field) - } - } - - impl ToArrowType for DType { - fn to_arrow_schema(&self) -> VortexResult { - to_arrow_schema_naive(self) - } - - fn to_arrow_dtype(&self) -> VortexResult { - to_data_type_naive(self) - } - } -} - #[cfg(test)] mod test { - #![expect(deprecated, reason = "tests for deprecated traits and methods")] use arrow_schema::DataType; use arrow_schema::Field; use arrow_schema::FieldRef; @@ -550,48 +329,40 @@ mod test { use vortex_array::dtype::StructFields; use super::*; + use crate::ArrowSession; #[test] fn test_dtype_conversion_success() { - assert_eq!(DType::Null.to_arrow_dtype().unwrap(), DataType::Null); + assert_eq!(to_data_type_naive(&DType::Null).unwrap(), DataType::Null); assert_eq!( - DType::Bool(Nullability::NonNullable) - .to_arrow_dtype() - .unwrap(), + to_data_type_naive(&DType::Bool(Nullability::NonNullable)).unwrap(), DataType::Boolean ); assert_eq!( - DType::Primitive(PType::U64, Nullability::NonNullable) - .to_arrow_dtype() - .unwrap(), + to_data_type_naive(&DType::Primitive(PType::U64, Nullability::NonNullable)).unwrap(), DataType::UInt64 ); assert_eq!( - DType::Utf8(Nullability::NonNullable) - .to_arrow_dtype() - .unwrap(), + to_data_type_naive(&DType::Utf8(Nullability::NonNullable)).unwrap(), DataType::Utf8View ); assert_eq!( - DType::Binary(Nullability::NonNullable) - .to_arrow_dtype() - .unwrap(), + to_data_type_naive(&DType::Binary(Nullability::NonNullable)).unwrap(), DataType::BinaryView ); assert_eq!( - DType::struct_( + to_data_type_naive(&DType::struct_( [ ("field_a", DType::Bool(false.into())), ("field_b", DType::Utf8(true.into())) ], Nullability::NonNullable, - ) - .to_arrow_dtype() + )) .unwrap(), DataType::Struct(Fields::from(vec![ FieldRef::from(Field::new("field_a", DataType::Boolean, false)), @@ -609,7 +380,7 @@ mod test { use vortex_array::dtype::DecimalDType; let dtype = DType::Decimal(DecimalDType::new(precision, 0), Nullability::NonNullable); - assert_eq!(dtype.to_arrow_dtype().unwrap(), expected); + assert_eq!(to_data_type_naive(&dtype).unwrap(), expected); } #[rstest] @@ -625,8 +396,7 @@ mod test { #[test] fn test_variant_dtype_to_arrow_dtype_errors() { - let err = DType::Variant(Nullability::NonNullable) - .to_arrow_dtype() + let err = to_data_type_naive(&DType::Variant(Nullability::NonNullable)) .unwrap_err() .to_string(); assert!(err.contains("Variant")); @@ -639,13 +409,13 @@ mod test { Nullability::Nullable, ); - let arrow_list_non_nullable = list_non_nullable.to_arrow_dtype().unwrap(); + let arrow_list_non_nullable = to_data_type_naive(&list_non_nullable).unwrap(); let list_nullable = DType::List( Arc::new(DType::Primitive(PType::I64, Nullability::Nullable)), Nullability::Nullable, ); - let arrow_list_nullable = list_nullable.to_arrow_dtype().unwrap(); + let arrow_list_nullable = to_data_type_naive(&list_nullable).unwrap(); assert_ne!(arrow_list_non_nullable, arrow_list_nullable); assert_eq!( @@ -679,7 +449,9 @@ mod test { let schema_nonnull = DType::Struct(the_struct, Nullability::NonNullable); assert_eq!( - schema_nonnull.to_arrow_schema().unwrap(), + ArrowSession::default() + .to_arrow_schema(&schema_nonnull) + .unwrap(), Schema::new(Fields::from(vec![ Field::new("field_a", DataType::Boolean, false), Field::new("field_b", DataType::Utf8View, false), @@ -688,43 +460,6 @@ mod test { ); } - #[test] - fn test_schema_variant_field_metadata() { - let dtype = DType::struct_( - [("v", DType::Variant(Nullability::NonNullable))], - Nullability::NonNullable, - ); - let schema = dtype.to_arrow_schema().unwrap(); - let field = schema.field(0); - assert_eq!( - field - .metadata() - .get("ARROW:extension:name") - .map(|s| s.as_str()), - Some("arrow.parquet.variant") - ); - assert_eq!(field.name(), "v"); - assert_eq!( - field.data_type(), - &DataType::Struct(Fields::from(vec![ - Field::new("metadata", DataType::Binary, false), - Field::new("value", DataType::Binary, true) - ])) - ); - assert!(!field.is_nullable()); - } - - #[rstest] - fn test_schema_conversion_rejects_nullable_struct(the_struct: StructFields) { - let schema_null = DType::Struct(the_struct, Nullability::Nullable); - let error = schema_null.to_arrow_schema().unwrap_err(); - assert!( - error - .to_string() - .contains("top-level struct in Schema must be NonNullable") - ); - } - #[test] fn test_unicode_field_names_roundtrip() { // Regression test for https://github.com/vortex-data/vortex/issues/5979. @@ -740,7 +475,7 @@ mod test { Nullability::NonNullable, ); - let arrow_dtype = original_dtype.to_arrow_dtype().unwrap(); + let arrow_dtype = to_data_type_naive(&original_dtype).unwrap(); assert_eq!( arrow_dtype, DataType::Struct(Fields::from(vec![Field::new( @@ -749,7 +484,8 @@ mod test { true )])) ); - let roundtripped_dtype = DType::from_arrow((&arrow_dtype, Nullability::NonNullable)); + let roundtripped_dtype = + from_arrow_data_type(&arrow_dtype, Nullability::NonNullable).unwrap(); assert_eq!(original_dtype, roundtripped_dtype); } @@ -769,7 +505,7 @@ mod test { let original_dtype = DType::struct_([("\u{7}=outer", inner_struct)], Nullability::NonNullable); - let arrow_dtype = original_dtype.to_arrow_dtype().unwrap(); + let arrow_dtype = to_data_type_naive(&original_dtype).unwrap(); assert_eq!( arrow_dtype, DataType::Struct(Fields::from(vec![Field::new( @@ -782,7 +518,8 @@ mod test { true )])) ); - let roundtripped_dtype = DType::from_arrow((&arrow_dtype, Nullability::NonNullable)); + let roundtripped_dtype = + from_arrow_data_type(&arrow_dtype, Nullability::NonNullable).unwrap(); assert_eq!(original_dtype, roundtripped_dtype); } @@ -797,7 +534,7 @@ mod test { Nullability::Nullable, )?; - let arrow = dtype.to_arrow_dtype()?; + let arrow = to_data_type_naive(&dtype)?; let DataType::Map(entries, keys_sorted) = &arrow else { panic!("expected Map, got {arrow:?}"); }; @@ -811,10 +548,7 @@ mod test { assert!(!fields[0].is_nullable()); assert_eq!(fields[1].name(), "value"); assert!(fields[1].is_nullable()); - assert_eq!( - DType::try_from_arrow((&arrow, Nullability::Nullable))?, - dtype - ); + assert_eq!(from_arrow_data_type(&arrow, Nullability::Nullable)?, dtype); let positional = DataType::Map( Arc::new(Field::new_struct( @@ -828,7 +562,7 @@ mod test { false, ); assert_eq!( - DType::try_from_arrow((&positional, Nullability::NonNullable))?, + from_arrow_data_type(&positional, Nullability::NonNullable)?, DType::map( DType::Primitive(PType::I32, Nullability::NonNullable), DType::Utf8(Nullability::Nullable), @@ -869,7 +603,7 @@ mod test { for entries in invalid_entries { let data_type = DataType::Map(Arc::new(entries), false); - assert!(DType::try_from_arrow((&data_type, Nullability::NonNullable)).is_err()); + assert!(from_arrow_data_type(&data_type, Nullability::NonNullable).is_err()); } } @@ -880,21 +614,21 @@ mod test { #[case::interval(DataType::Interval(arrow_schema::IntervalUnit::DayTime))] #[case::fixed_size_binary(DataType::FixedSizeBinary(3))] fn test_try_from_arrow_unsupported_type_errors(#[case] data_type: DataType) { - let err = DType::try_from_arrow((&data_type, Nullability::NonNullable)) + let err = from_arrow_data_type(&data_type, Nullability::NonNullable) .expect_err("unsupported Arrow type should not convert") .to_string(); assert!(err.contains("not supported"), "unexpected error: {err}"); // The same unsupported type nested in a field or schema must also error cleanly. let field = Field::new("c0", data_type, true); - assert!(DType::try_from_arrow(&field).is_err()); + assert!(from_arrow_field_naive(&field).is_err()); let schema = Schema::new(vec![field]); - assert!(DType::try_from_arrow(&schema).is_err()); + assert!(ArrowSession::default().from_arrow_schema(&schema).is_err()); } #[test] fn test_try_from_arrow_supported_type_succeeds() -> VortexResult<()> { - let dtype = DType::try_from_arrow((&DataType::Int32, Nullability::Nullable))?; + let dtype = from_arrow_data_type(&DataType::Int32, Nullability::Nullable)?; assert_eq!(dtype, DType::Primitive(PType::I32, Nullability::Nullable)); Ok(()) } diff --git a/vortex-arrow/src/executor/map.rs b/vortex-arrow/src/executor/map.rs index 810c74a6a1d..f93b5a34179 100644 --- a/vortex-arrow/src/executor/map.rs +++ b/vortex-arrow/src/executor/map.rs @@ -86,7 +86,6 @@ mod tests { use arrow_schema::DataType; use arrow_schema::Field; use arrow_schema::Fields; - use vortex_array::ArrayRef; use vortex_array::IntoArray; use vortex_array::VortexSessionExecute; use vortex_array::array_session; @@ -105,7 +104,7 @@ mod tests { use vortex_error::VortexResult; use vortex_mask::Mask; - use crate::FromArrowArray as _; + use crate::convert::from_arrow_map; use crate::session::ArrowSessionExt as _; fn i32_utf8_map_field(keys_sorted: bool, nullable: bool) -> Field { @@ -202,7 +201,7 @@ mod tests { } #[test] - fn legacy_from_arrow_map_imports_canonical_map() -> VortexResult<()> { + fn from_arrow_map_imports_canonical_map() -> VortexResult<()> { let arrow = i32_utf8_map_array( vec![0, 1, 2], vec![1, 2], @@ -210,7 +209,7 @@ mod tests { None, false, )?; - let vortex = ArrayRef::from_arrow(&arrow, false)?; + let vortex = from_arrow_map(&arrow, false)?; assert!(vortex.is::()); assert_eq!( @@ -274,13 +273,13 @@ mod tests { false, Nullable, )?; - let legacy = ArrayRef::from_arrow(&arrow, true)?; - assert_eq!(legacy.dtype(), &expected_dtype); + let canonical = from_arrow_map(&arrow, true)?; + assert_eq!(canonical.dtype(), &expected_dtype); let field = Field::new("maps", arrow.data_type().clone(), true); let vortex = session.from_arrow_array(Arc::new(arrow), &field)?; assert_eq!(vortex.dtype(), &expected_dtype); - for imported in [&legacy, &vortex] { + for imported in [&canonical, &vortex] { assert_eq!(imported.len(), 4); let expected_rows = [ Some(vec![("joe", 1i32)]), diff --git a/vortex-arrow/src/lib.rs b/vortex-arrow/src/lib.rs index 7b08d5e7c6f..b0388ff176f 100644 --- a/vortex-arrow/src/lib.rs +++ b/vortex-arrow/src/lib.rs @@ -16,12 +16,6 @@ //! The [`ArrowSession`] session variable is created lazily on first use of //! [`ArrowSessionExt::arrow`], so no explicit registration is required for plain conversions. -use arrow_array::ArrayRef as ArrowArrayRef; -use arrow_schema::DataType; -use vortex_array::ArrayRef; -use vortex_array::VortexSessionExecute; -use vortex_array::legacy_session; -use vortex_error::VortexResult; use vortex_session::VortexSession; pub mod convert; @@ -38,12 +32,6 @@ mod session; pub use convert::IntoVortexArray; pub use convert::nulls; pub use datum::*; -#[allow(deprecated)] -pub use dtype::FromArrowType; -#[allow(deprecated)] -pub use dtype::ToArrowType; -#[allow(deprecated)] -pub use dtype::TryFromArrowType; pub use executor::*; pub use iter::*; pub use null_buffer::to_arrow_null_buffer; @@ -58,61 +46,3 @@ pub use session::*; pub fn initialize(session: &VortexSession) { session.register(ArrowSession::default()); } - -/// Construct a Vortex array from an Arrow array (or other Arrow container) of type `A`. -/// -/// Implementations reuse the underlying Arrow buffers without copying wherever the Arrow and -/// Vortex memory layouts allow it. -#[deprecated(note = "Use `ArrowSession` (`from_arrow_array`, `from_arrow_record_batch`) instead")] -pub trait FromArrowArray { - /// Convert `array` into a Vortex array whose [`DType`](vortex_array::dtype::DType) has the requested - /// `nullable` [`Nullability`](vortex_array::dtype::Nullability). - /// - /// An Arrow array can carry a validity (null) buffer regardless of whether its schema declares - /// the field nullable, so the desired nullability is supplied separately by the caller - /// (typically from the corresponding Arrow `Field`'s `is_nullable`). This flag is reconciled - /// with the array's physical nulls as follows: - /// - /// - `nullable == true`: the resulting validity is derived from the array's null buffer, or - /// all-valid when the array has none. - /// - `nullable == false`: the array must contain no nulls, and the result is non-nullable. - /// - /// # Errors - /// - /// Returns an error if `nullable` is `false` but `array` physically contains one or more nulls - /// (including an Arrow `NullArray`, which is entirely null), or if the Arrow data type is not - /// supported. - #[deprecated( - note = "Use `ArrowSession` (`from_arrow_array`, `from_arrow_record_batch`) instead" - )] - fn from_arrow(array: A, nullable: bool) -> VortexResult - where - Self: Sized; -} - -#[deprecated(note = "Use `execute_arrow(None, ctx)` or `execute_arrow(Some(dt), ctx)` instead")] -pub trait IntoArrowArray { - #[deprecated(note = "Use `execute_arrow(None, ctx)` instead")] - fn into_arrow_preferred(self) -> VortexResult; - - #[deprecated(note = "Use `execute_arrow(Some(data_type), ctx)` instead")] - fn into_arrow(self, data_type: &DataType) -> VortexResult; -} - -#[allow(deprecated)] -impl IntoArrowArray for ArrayRef { - /// Convert this [`vortex_array::ArrayRef`] into an Arrow [`ArrowArrayRef`] by using the array's - /// preferred (cheapest) Arrow [`DataType`]. - #[allow(clippy::disallowed_methods)] - fn into_arrow_preferred(self) -> VortexResult { - self.execute_arrow(None, &mut legacy_session().create_execution_ctx()) - } - - #[allow(clippy::disallowed_methods)] - fn into_arrow(self, data_type: &DataType) -> VortexResult { - self.execute_arrow( - Some(data_type), - &mut legacy_session().create_execution_ctx(), - ) - } -} diff --git a/vortex-arrow/src/run_end_import.rs b/vortex-arrow/src/run_end_import.rs index d18b6bbecd0..d92c6b6c5a8 100644 --- a/vortex-arrow/src/run_end_import.rs +++ b/vortex-arrow/src/run_end_import.rs @@ -18,9 +18,9 @@ use vortex_runend::RunEnd; /// Build a Vortex run-end array from an Arrow [`RunArray`] and its already-converted `values`. /// -/// The caller converts `values` separately (rather than this function calling -/// [`crate::FromArrowArray`] on `array.values()`) so that extension metadata on the Arrow values -/// field dispatches through the session's registered import plugins. +/// The caller converts `values` separately (rather than this function converting +/// `array.values()` itself) so that extension metadata on the Arrow values field dispatches +/// through the session's registered import plugins. pub(crate) fn run_end_from_arrow( array: &RunArray, values: ArrayRef, diff --git a/vortex-arrow/src/session.rs b/vortex-arrow/src/session.rs index c830ef16bb0..9212def5b85 100644 --- a/vortex-arrow/src/session.rs +++ b/vortex-arrow/src/session.rs @@ -213,8 +213,7 @@ pub trait ArrowImportVTable: 'static + Send + Sync + Debug { /// /// `session` is provided so plugins can convert storage or nested arrays through the /// session (e.g. [`ArrowSession::from_arrow_array`], passing a nullability rather than a - /// [`Field`] so the plugin is not dispatched again) instead of the deprecated - /// `FromArrowArray` trait. + /// [`Field`] so the plugin is not dispatched again). #[allow(clippy::wrong_self_convention)] fn from_arrow_array( &self, diff --git a/vortex-duckdb/src/convert/expr.rs b/vortex-duckdb/src/convert/expr.rs index dff2b674d5b..7d7cf0e6aa8 100644 --- a/vortex-duckdb/src/convert/expr.rs +++ b/vortex-duckdb/src/convert/expr.rs @@ -17,7 +17,6 @@ use vortex::aggregate_fn::fns::max::Max; use vortex::aggregate_fn::fns::mean::Mean; use vortex::aggregate_fn::fns::min::Min; use vortex::aggregate_fn::fns::sum_v2::SumV2; -use vortex::arrow::ArrowSessionExt; use vortex::dtype::DType; use vortex::dtype::Nullability; use vortex::dtype::PType; @@ -158,7 +157,7 @@ fn spatial_operand(value: &ExpressionRef, ctx: ConvertCtx<'_>) -> VortexResult diff --git a/vortex-jni/src/expression.rs b/vortex-jni/src/expression.rs index a766bf3d6c3..afcefe990c4 100644 --- a/vortex-jni/src/expression.rs +++ b/vortex-jni/src/expression.rs @@ -24,6 +24,7 @@ use jni::sys::jfloat; use jni::sys::jint; use jni::sys::jlong; use jni::sys::jshort; +use vortex::VortexSessionDefault; use vortex::dtype::BigCast; use vortex::dtype::DType; use vortex::dtype::DecimalDType; @@ -71,7 +72,7 @@ use vortex::scalar_fn::fns::list_contains::ListContainsOptions; use vortex::scalar_fn::fns::literal::Literal; use vortex::scalar_fn::fns::merge::DuplicateHandling; use vortex::scalar_fn::fns::operators::Operator; -use vortex_arrow::ArrowSession; +use vortex::session::VortexSession; use vortex_spatial::extension::native_geometry_scalar_from_wkb; use vortex_spatial::scalar_fn::area::SpatialArea; use vortex_spatial::scalar_fn::collect::SpatialCollect; @@ -748,7 +749,7 @@ pub extern "system" fn Java_dev_vortex_jni_NativeExpression_literalGeometry( throw_runtime!("geometry literal WKB bytes must not be null"); } let bytes = env.convert_byte_array(&wkb)?; - let scalar = native_geometry_scalar_from_wkb(&bytes, &ArrowSession::default())? + let scalar = native_geometry_scalar_from_wkb(&bytes, &VortexSession::default())? .ok_or_else(|| vortex_err!("unsupported WKB geometry type for a geometry literal"))?; Ok(into_raw(lit(scalar))) }) diff --git a/vortex-spatial/src/extension/mod.rs b/vortex-spatial/src/extension/mod.rs index e662c479bb6..4da9cd74571 100644 --- a/vortex-spatial/src/extension/mod.rs +++ b/vortex-spatial/src/extension/mod.rs @@ -42,6 +42,7 @@ pub use rect::*; use vortex_array::ArrayRef; use vortex_array::ExecutionCtx; use vortex_array::IntoArray; +use vortex_array::VortexSessionExecute; use vortex_array::arrays::ConstantArray; use vortex_array::arrays::ExtensionArray; use vortex_array::arrays::ListViewArray; @@ -58,10 +59,12 @@ use vortex_array::dtype::extension::ExtDType; use vortex_array::dtype::extension::ExtVTable; use vortex_array::scalar::Scalar; use vortex_arrow::ArrowSession; +use vortex_arrow::ArrowSessionExt; use vortex_buffer::Buffer; use vortex_error::VortexResult; use vortex_error::vortex_bail; use vortex_error::vortex_err; +use vortex_session::VortexSession; pub use wkb::*; /// Whether `dtype` is one of the native geometry extension types the spatial kernels operate on. @@ -194,7 +197,7 @@ pub(crate) fn single_geometry( /// `Point`/`Polygon`/`MultiPolygon` scalar. `None` for unsupported types. Plan-time, one value only. pub fn native_geometry_scalar_from_wkb( bytes: &[u8], - session: &ArrowSession, + session: &VortexSession, ) -> VortexResult> { let metadata = geoarrow_metadata(&SpatialMetadata::default()); let binary = BinaryArray::from(vec![Some(bytes)]); @@ -208,7 +211,9 @@ pub fn native_geometry_scalar_from_wkb( let to_storage = |target: &GeoArrowType| -> VortexResult { let native = cast(&wkb, target).map_err(|e| vortex_err!("failed to cast WKB literal: {e}"))?; - session.from_arrow_array(native.to_array_ref(), false) + session + .arrow() + .from_arrow_array(native.to_array_ref(), false) }; let scalar = match Wkb::try_from_bytes(bytes)?.geometry_type() { @@ -216,39 +221,39 @@ pub fn native_geometry_scalar_from_wkb( let target = GeoArrowType::Point( PointType::new(Dimension::XY, metadata).with_coord_type(CoordType::Separated), ); - spatial_ext_scalar(Point, to_storage(&target)?)? + spatial_ext_scalar(Point, to_storage(&target)?, session)? } GeometryType::LineString => { let target = GeoArrowType::LineString( LineStringType::new(Dimension::XY, metadata).with_coord_type(CoordType::Separated), ); - spatial_ext_scalar(LineString, to_storage(&target)?)? + spatial_ext_scalar(LineString, to_storage(&target)?, session)? } GeometryType::Polygon => { let target = GeoArrowType::Polygon( PolygonType::new(Dimension::XY, metadata).with_coord_type(CoordType::Separated), ); - spatial_ext_scalar(Polygon, to_storage(&target)?)? + spatial_ext_scalar(Polygon, to_storage(&target)?, session)? } GeometryType::MultiPoint => { let target = GeoArrowType::MultiPoint( MultiPointType::new(Dimension::XY, metadata).with_coord_type(CoordType::Separated), ); - spatial_ext_scalar(MultiPoint, to_storage(&target)?)? + spatial_ext_scalar(MultiPoint, to_storage(&target)?, session)? } GeometryType::MultiLineString => { let target = GeoArrowType::MultiLineString( MultiLineStringType::new(Dimension::XY, metadata) .with_coord_type(CoordType::Separated), ); - spatial_ext_scalar(MultiLineString, to_storage(&target)?)? + spatial_ext_scalar(MultiLineString, to_storage(&target)?, session)? } GeometryType::MultiPolygon => { let target = GeoArrowType::MultiPolygon( MultiPolygonType::new(Dimension::XY, metadata) .with_coord_type(CoordType::Separated), ); - spatial_ext_scalar(MultiPolygon, to_storage(&target)?)? + spatial_ext_scalar(MultiPolygon, to_storage(&target)?, session)? } _ => return Ok(None), }; @@ -256,18 +261,17 @@ pub fn native_geometry_scalar_from_wkb( } /// Wrap cast-from-WKB `storage` in its `vtable` extension type and pull out the single scalar. -// `scalar_at` is deprecated for `execute_scalar`, but there is no execution context at plan time. -#[allow(deprecated)] fn spatial_ext_scalar>( vtable: V, storage: ArrayRef, + session: &VortexSession, ) -> VortexResult { let ext = ExtDType::try_with_vtable(vtable, SpatialMetadata::default(), storage.dtype().clone())? .erased(); ExtensionArray::try_new(ext, storage)? .into_array() - .scalar_at(0) + .execute_scalar(0, &mut session.create_execution_ctx()) } /// Extension metadata that is common to all the spatial extension types. @@ -334,7 +338,6 @@ mod tests { use prost::Message; use vortex_array::VortexSessionExecute; use vortex_array::dtype::DType; - use vortex_arrow::ArrowSessionExt; use vortex_error::VortexResult; use vortex_error::vortex_err; @@ -349,8 +352,7 @@ mod tests { fn native_geometry_scalar_from_wkb( bytes: &[u8], ) -> VortexResult> { - let session = vortex_array::array_session(); - super::native_geometry_scalar_from_wkb(bytes, &session.arrow()) + super::native_geometry_scalar_from_wkb(bytes, &vortex_array::array_session()) } #[test] diff --git a/vortex-spatial/src/scalar_fn/contains.rs b/vortex-spatial/src/scalar_fn/contains.rs index 796e283f63f..9093f7055c2 100644 --- a/vortex-spatial/src/scalar_fn/contains.rs +++ b/vortex-spatial/src/scalar_fn/contains.rs @@ -93,7 +93,6 @@ mod tests { use vortex_array::scalar_fn::EmptyOptions; use vortex_array::scalar_fn::ScalarFnVTable; use vortex_array::validity::Validity; - use vortex_arrow::ArrowSessionExt; use vortex_buffer::BitBuffer; use vortex_error::VortexResult; use vortex_error::vortex_err; @@ -118,7 +117,7 @@ mod tests { wkb::writer::write_geometry(&mut buf, geometry, &WriteOptions::default()) .map_err(|e| vortex_err!("writing WKB failed: {e}"))?; let session = vortex_array::array_session(); - let scalar = crate::extension::native_geometry_scalar_from_wkb(&buf, &session.arrow())? + let scalar = crate::extension::native_geometry_scalar_from_wkb(&buf, &session)? .ok_or_else(|| vortex_err!("unsupported geometry type"))?; Ok(ConstantArray::new(scalar, len).into_array()) } diff --git a/vortex-spatial/src/scalar_fn/intersects.rs b/vortex-spatial/src/scalar_fn/intersects.rs index bf913ec507c..3a6593b2ba2 100644 --- a/vortex-spatial/src/scalar_fn/intersects.rs +++ b/vortex-spatial/src/scalar_fn/intersects.rs @@ -92,7 +92,6 @@ mod tests { use vortex_array::scalar_fn::EmptyOptions; use vortex_array::scalar_fn::ScalarFnVTable; use vortex_array::validity::Validity; - use vortex_arrow::ArrowSessionExt; use vortex_buffer::BitBuffer; use vortex_error::VortexResult; use vortex_error::vortex_err; @@ -139,7 +138,7 @@ mod tests { wkb::writer::write_geometry(&mut buf, geometry, &WriteOptions::default()) .map_err(|e| vortex_err!("writing WKB failed: {e}"))?; let session = vortex_array::array_session(); - let scalar = crate::extension::native_geometry_scalar_from_wkb(&buf, &session.arrow())? + let scalar = crate::extension::native_geometry_scalar_from_wkb(&buf, &session)? .ok_or_else(|| vortex_err!("unsupported geometry type"))?; Ok(ConstantArray::new(scalar, len).into_array()) }