FromArrowArray is implemented for &dyn Array instead of ArrayRef (#3378)

We never rely on the value being an owned Arc

---------

Signed-off-by: Robert Kruszewski <[email protected]>

Author: robert3005

vortex-array/src/arrays/varbin/canonical.rs

@@ -21,7 +21,7 @@ impl CanonicalVTable<VarBinVTable> for VarBinVTable {
             _ => unreachable!("VarBinArray must have Utf8 or Binary dtype"),
         };
         Ok(Canonical::VarBinView(
-            ArrayRef::from_arrow(array, nullable).to_varbinview()?,
+            ArrayRef::from_arrow(array.as_ref(), nullable).to_varbinview()?,
         ))
     }
 }

vortex-array/src/arrow/array.rs

@@ -84,7 +84,7 @@ impl ArrayVTable<ArrowVTable> for ArrowVTable {
 
 impl CanonicalVTable<ArrowVTable> for ArrowVTable {
     fn canonicalize(array: &ArrowArray) -> VortexResult<Canonical> {
-        ArrayRef::from_arrow(array.inner.clone(), array.dtype.is_nullable()).to_canonical()
+        ArrayRef::from_arrow(array.inner.as_ref(), array.dtype.is_nullable()).to_canonical()
     }
 }
 

vortex-array/src/arrow/convert.rs

@@ -1,7 +1,7 @@
 use arrow_array::array::{
-    Array as ArrowArray, ArrayRef as ArrowArrayRef, ArrowPrimitiveType,
-    BooleanArray as ArrowBooleanArray, GenericByteArray, NullArray as ArrowNullArray,
-    OffsetSizeTrait, PrimitiveArray as ArrowPrimitiveArray, StructArray as ArrowStructArray,
+    Array as ArrowArray, ArrowPrimitiveType, BooleanArray as ArrowBooleanArray, GenericByteArray,
+    NullArray as ArrowNullArray, OffsetSizeTrait, PrimitiveArray as ArrowPrimitiveArray,
+    StructArray as ArrowStructArray,
 };
 use arrow_array::cast::{AsArray, as_null_array};
 use arrow_array::types::{
@@ -233,7 +233,7 @@ impl FromArrowArray<&ArrowStructArray> for ArrayRef {
                 .columns()
                 .iter()
                 .zip(value.fields())
-                .map(|(c, field)| Self::from_arrow(c.clone(), field.is_nullable()))
+                .map(|(c, field)| Self::from_arrow(c.as_ref(), field.is_nullable()))
                 .collect(),
             value.len(),
             nulls(value.nulls(), nullable),
@@ -252,7 +252,7 @@ impl<O: OffsetSizeTrait + NativePType> FromArrowArray<&GenericListArray<O>> for
             dt => vortex_panic!("Invalid data type for ListArray: {dt}"),
         };
         ListArray::try_new(
-            Self::from_arrow(value.values().clone(), elem_nullable),
+            Self::from_arrow(value.values().as_ref(), elem_nullable),
             // offsets are always non-nullable
             value.offsets().clone().into_array(),
             nulls(value.nulls(), nullable),
@@ -286,8 +286,8 @@ fn nulls(nulls: Option<&NullBuffer>, nullable: bool) -> Validity {
     }
 }
 
-impl FromArrowArray<ArrowArrayRef> for ArrayRef {
-    fn from_arrow(array: ArrowArrayRef, nullable: bool) -> Self {
+impl FromArrowArray<&dyn ArrowArray> for ArrayRef {
+    fn from_arrow(array: &dyn ArrowArray, nullable: bool) -> Self {
         match array.data_type() {
             DataType::Boolean => Self::from_arrow(array.as_boolean(), nullable),
             DataType::UInt8 => Self::from_arrow(array.as_primitive::<UInt8Type>(), nullable),
@@ -322,7 +322,7 @@ impl FromArrowArray<ArrowArrayRef> for ArrayRef {
             DataType::Struct(_) => Self::from_arrow(array.as_struct(), nullable),
             DataType::List(_) => Self::from_arrow(array.as_list::<i32>(), nullable),
             DataType::LargeList(_) => Self::from_arrow(array.as_list::<i64>(), nullable),
-            DataType::Null => Self::from_arrow(as_null_array(&array), nullable),
+            DataType::Null => Self::from_arrow(as_null_array(array), nullable),
             DataType::Timestamp(u, _) => match u {
                 ArrowTimeUnit::Second => {
                     Self::from_arrow(array.as_primitive::<TimestampSecondType>(), nullable)

vortex-array/src/arrow/record_batch.rs

@@ -20,7 +20,7 @@ impl TryIntoArray for RecordBatch {
             self.columns()
                 .iter()
                 .zip(self.schema().fields())
-                .map(|(array, field)| ArrayRef::from_arrow(array.clone(), field.is_nullable()))
+                .map(|(array, field)| ArrayRef::from_arrow(array.as_ref(), field.is_nullable()))
                 .collect(),
             self.num_rows(),
             Validity::NonNullable, // Must match FromArrowType<SchemaRef> for DType

vortex-array/src/compute/boolean.rs

@@ -1,8 +1,7 @@
 use std::any::Any;
-use std::sync::{Arc, LazyLock};
+use std::sync::LazyLock;
 
 use arcref::ArcRef;
-use arrow_array::ArrayRef as ArrowArrayRef;
 use arrow_array::cast::AsArray;
 use arrow_schema::DataType;
 use vortex_dtype::DType;
@@ -253,10 +252,7 @@ pub(crate) fn arrow_boolean(
         BooleanOperator::OrKleene => arrow_arith::boolean::or_kleene(&lhs, &rhs)?,
     };
 
-    Ok(ArrayRef::from_arrow(
-        Arc::new(array) as ArrowArrayRef,
-        nullable,
-    ))
+    Ok(ArrayRef::from_arrow(&array, nullable))
 }
 
 #[cfg(test)]

vortex-array/src/compute/filter.rs

@@ -239,7 +239,10 @@ pub fn arrow_filter_fn(array: &dyn Array, mask: &Mask) -> VortexResult<ArrayRef>
     let mask_array = BooleanArray::new(values.boolean_buffer().clone(), None);
     let filtered = arrow_select::filter::filter(array_ref.as_ref(), &mask_array)?;
 
-    Ok(ArrayRef::from_arrow(filtered, array.dtype().is_nullable()))
+    Ok(ArrayRef::from_arrow(
+        filtered.as_ref(),
+        array.dtype().is_nullable(),
+    ))
 }
 
 #[cfg(test)]

vortex-array/src/compute/mask.rs

@@ -128,7 +128,7 @@ impl ComputeFnVTable for MaskFn {
 
         let masked = arrow_select::nullif::nullif(array_ref.as_ref(), &mask)?;
 
-        Ok(ArrayRef::from_arrow(masked, true).into())
+        Ok(ArrayRef::from_arrow(masked.as_ref(), true).into())
     }
 
     fn return_dtype(&self, args: &InvocationArgs) -> VortexResult<DType> {

vortex-array/src/compute/numeric.rs

@@ -253,7 +253,7 @@ fn arrow_numeric(
         NumericOperator::RDiv => arrow_arith::numeric::div(&right, &left)?,
     };
 
-    from_arrow_array_with_len(array, len, nullable)
+    from_arrow_array_with_len(array.as_ref(), len, nullable)
 }
 
 #[cfg(test)]

vortex-duckdb/src/convert/array/array_ref.rs

@@ -83,6 +83,9 @@ impl FromDuckDB<SizedFlatVector> for ArrayRef {
         let len = sized_vector.len;
         let arrow_arr = flat_vector_to_arrow_array(&mut sized_vector.vector, len)
             .map_err(|e| vortex_err!("Failed to convert duckdb array to vortex: {}", e))?;
-        Ok(ArrayRef::from_arrow(arrow_arr, sized_vector.nullable))
+        Ok(ArrayRef::from_arrow(
+            arrow_arr.as_ref(),
+            sized_vector.nullable,
+        ))
     }
 }

vortex-python/src/arrays/from_arrow.rs

@@ -24,7 +24,7 @@ pub(super) fn from_arrow(obj: &Bound<'_, PyAny>) -> PyResult<PyArrayRef> {
     if obj.is_instance(&pa_array)? {
         let arrow_array = ArrowArrayData::from_pyarrow_bound(obj).map(make_array)?;
         let is_nullable = arrow_array.is_nullable();
-        let enc_array = ArrayRef::from_arrow(arrow_array, is_nullable);
+        let enc_array = ArrayRef::from_arrow(arrow_array.as_ref(), is_nullable);
         Ok(PyArrayRef::from(enc_array))
     } else if obj.is_instance(&chunked_array)? {
         let chunks: Vec<Bound<PyAny>> = obj.getattr("chunks")?.extract()?;
@@ -33,7 +33,7 @@ pub(super) fn from_arrow(obj: &Bound<'_, PyAny>) -> PyResult<PyArrayRef> {
             .map(|a| {
                 ArrowArrayData::from_pyarrow_bound(a)
                     .map(make_array)
-                    .map(|a| ArrayRef::from_arrow(a, false))
+                    .map(|a| ArrayRef::from_arrow(a.as_ref(), false))
             })
             .collect::<PyResult<Vec<_>>>()?;
         let dtype: DType = obj