Merge branch 'main' into avro-row-encoder

Author: jecsand838

arrow-arith/src/numeric.rs

@@ -389,24 +389,21 @@ impl From<Vec<Option<bool>>> for BooleanArray {
 
 impl From<ArrayData> for BooleanArray {
     fn from(data: ArrayData) -> Self {
+        let (data_type, len, nulls, offset, mut buffers, _child_data) = data.into_parts();
         assert_eq!(
-            data.data_type(),
-            &DataType::Boolean,
-            "BooleanArray expected ArrayData with type {} got {}",
+            data_type,
             DataType::Boolean,
-            data.data_type()
+            "BooleanArray expected ArrayData with type Boolean got {data_type:?}",
         );
         assert_eq!(
-            data.buffers().len(),
+            buffers.len(),
             1,
             "BooleanArray data should contain a single buffer only (values buffer)"
         );
-        let values = BooleanBuffer::new(data.buffers()[0].clone(), data.offset(), data.len());
+        let buffer = buffers.pop().expect("checked above");
+        let values = BooleanBuffer::new(buffer, offset, len);
 
-        Self {
-            values,
-            nulls: data.nulls().cloned(),
-        }
+        Self { values, nulls }
     }
 }
 

arrow-array/src/array/boolean_array.rs

@@ -15,7 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use crate::array::{get_offsets, print_long_array};
+use crate::array::{get_offsets_from_buffer, print_long_array};
 use crate::builder::GenericByteBuilder;
 use crate::iterator::ArrayIter;
 use crate::types::ByteArrayType;
@@ -542,30 +542,34 @@ impl<'a, T: ByteArrayType> ArrayAccessor for &'a GenericByteArray<T> {
 
 impl<T: ByteArrayType> From<ArrayData> for GenericByteArray<T> {
     fn from(data: ArrayData) -> Self {
+        let (data_type, len, nulls, offset, mut buffers, _child_data) = data.into_parts();
         assert_eq!(
-            data.data_type(),
-            &Self::DATA_TYPE,
+            data_type,
+            Self::DATA_TYPE,
             "{}{}Array expects DataType::{}",
             T::Offset::PREFIX,
             T::PREFIX,
             Self::DATA_TYPE
         );
         assert_eq!(
-            data.buffers().len(),
+            buffers.len(),
             2,
             "{}{}Array data should contain 2 buffers only (offsets and values)",
             T::Offset::PREFIX,
             T::PREFIX,
         );
+        // buffers are offset then value, so pop in reverse
+        let value_data = buffers.pop().expect("checked above");
+        let offset_buffer = buffers.pop().expect("checked above");
+
         // SAFETY:
         // ArrayData is valid, and verified type above
-        let value_offsets = unsafe { get_offsets(&data) };
-        let value_data = data.buffers()[1].clone();
+        let value_offsets = unsafe { get_offsets_from_buffer(offset_buffer, offset, len) };
         Self {
             value_offsets,
             value_data,
-            data_type: T::DATA_TYPE,
-            nulls: data.nulls().cloned(),
+            data_type,
+            nulls,
         }
     }
 }

arrow-array/src/array/byte_array.rs

@@ -988,14 +988,20 @@ impl<'a, T: ByteViewType + ?Sized> IntoIterator for &'a GenericByteViewArray<T>
 
 impl<T: ByteViewType + ?Sized> From<ArrayData> for GenericByteViewArray<T> {
     fn from(data: ArrayData) -> Self {
-        let (_data_type, len, nulls, offset, mut buffers, _child_data) = data.into_parts();
-        let views = buffers.remove(0); // need to maintain order of remaining buffers
-        let buffers = Arc::from(buffers);
-        let views = ScalarBuffer::new(views, offset, len);
+        let (data_type, len, nulls, offset, buffers, _child_data) = data.into_parts();
+        assert_eq!(
+            data_type,
+            T::DATA_TYPE,
+            "Mismatched data type, expected {}, got {data_type}",
+            T::DATA_TYPE
+        );
+        let mut buffers = buffers.into_iter();
+        // first buffer is views, remaining are data buffers
+        let views = ScalarBuffer::new(buffers.next().unwrap(), offset, len);
         Self {
-            data_type: T::DATA_TYPE,
+            data_type,
             views,
-            buffers,
+            buffers: Arc::from_iter(buffers),
             nulls,
             phantom: Default::default(),
         }
@@ -1205,9 +1211,11 @@ mod tests {
         Array, BinaryViewArray, GenericBinaryArray, GenericByteViewArray, StringViewArray,
     };
     use arrow_buffer::{Buffer, NullBuffer, ScalarBuffer};
-    use arrow_data::{ByteView, MAX_INLINE_VIEW_LEN};
+    use arrow_data::{ArrayDataBuilder, ByteView, MAX_INLINE_VIEW_LEN};
+    use arrow_schema::DataType;
     use rand::prelude::StdRng;
     use rand::{Rng, SeedableRng};
+    use std::str::from_utf8;
 
     const BLOCK_SIZE: u32 = 8;
 
@@ -1814,4 +1822,46 @@ mod tests {
 
         assert_eq!(lengths_iter.next(), None, "Should not have more lengths");
     }
+
+    #[should_panic(expected = "Mismatched data type, expected Utf8View, got BinaryView")]
+    #[test]
+    fn invalid_casting_from_array_data() {
+        // Should not be able to cast to StringViewArray due to invalid UTF-8
+        let array_data = binary_view_array_with_invalid_utf8_data().into_data();
+        let _ = StringViewArray::from(array_data);
+    }
+
+    #[should_panic(expected = "invalid utf-8 sequence")]
+    #[test]
+    fn invalid_array_data() {
+        let (views, buffers, nulls) = binary_view_array_with_invalid_utf8_data().into_parts();
+
+        // manually try and add invalid array data with Utf8View data type
+        let mut builder = ArrayDataBuilder::new(DataType::Utf8View)
+            .add_buffer(views.into_inner())
+            .len(3);
+        for buffer in buffers.iter() {
+            builder = builder.add_buffer(buffer.clone())
+        }
+        builder = builder.nulls(nulls);
+
+        let data = builder.build().unwrap(); // should fail validation
+        let _arr = StringViewArray::from(data);
+    }
+
+    /// Returns a BinaryViewArray with one invalid UTF-8 value
+    fn binary_view_array_with_invalid_utf8_data() -> BinaryViewArray {
+        let array = GenericByteViewArray::<BinaryViewType>::from(vec![
+            b"aaaaaaaaaaaaaaaaaaaaaaaaaaa" as &[u8],
+            &[
+                0xf0, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00,
+            ],
+            b"good",
+        ]);
+        assert!(from_utf8(array.value(0)).is_ok());
+        assert!(from_utf8(array.value(1)).is_err()); // value 1 is invalid utf8
+        assert!(from_utf8(array.value(2)).is_ok());
+        array
+    }
 }

arrow-array/src/array/byte_view_array.rs

@@ -25,7 +25,7 @@ use crate::{
 };
 use arrow_buffer::bit_util::set_bit;
 use arrow_buffer::buffer::NullBuffer;
-use arrow_buffer::{ArrowNativeType, BooleanBuffer, BooleanBufferBuilder};
+use arrow_buffer::{ArrowNativeType, BooleanBuffer, BooleanBufferBuilder, ScalarBuffer};
 use arrow_data::ArrayData;
 use arrow_schema::{ArrowError, DataType};
 use std::any::Any;
@@ -580,21 +580,25 @@ impl<K: ArrowDictionaryKeyType> DictionaryArray<K> {
     }
 }
 
-/// Constructs a `DictionaryArray` from an array data reference.
+/// Constructs a `DictionaryArray` from an `ArrayData`
 impl<T: ArrowDictionaryKeyType> From<ArrayData> for DictionaryArray<T> {
     fn from(data: ArrayData) -> Self {
+        let (data_type, len, nulls, offset, mut buffers, mut child_data) = data.into_parts();
+
         assert_eq!(
-            data.buffers().len(),
+            buffers.len(),
             1,
             "DictionaryArray data should contain a single buffer only (keys)."
         );
+        let buffer = buffers.pop().expect("checked above");
         assert_eq!(
-            data.child_data().len(),
+            child_data.len(),
             1,
             "DictionaryArray should contain a single child array (values)."
         );
+        let cd = child_data.pop().expect("checked above");
 
-        if let DataType::Dictionary(key_data_type, _) = data.data_type() {
+        if let DataType::Dictionary(key_data_type, _) = &data_type {
             assert_eq!(
                 &T::DATA_TYPE,
                 key_data_type.as_ref(),
@@ -603,19 +607,10 @@ impl<T: ArrowDictionaryKeyType> From<ArrayData> for DictionaryArray<T> {
                 key_data_type
             );
 
-            let values = make_array(data.child_data()[0].clone());
-            let data_type = data.data_type().clone();
+            let values = make_array(cd);
 
             // create a zero-copy of the keys' data
-            // SAFETY:
-            // ArrayData is valid and verified type above
-
-            let keys = PrimitiveArray::<T>::from(unsafe {
-                data.into_builder()
-                    .data_type(T::DATA_TYPE)
-                    .child_data(vec![])
-                    .build_unchecked()
-            });
+            let keys = PrimitiveArray::<T>::new(ScalarBuffer::new(buffer, offset, len), nulls);
 
             Self {
                 data_type,

arrow-array/src/array/dictionary_array.rs

@@ -20,7 +20,7 @@
 mod binary_array;
 
 use crate::types::*;
-use arrow_buffer::{ArrowNativeType, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow_buffer::{ArrowNativeType, Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
 use arrow_data::ArrayData;
 use arrow_schema::{DataType, IntervalUnit, TimeUnit};
 use std::any::Any;
@@ -939,6 +939,27 @@ unsafe fn get_offsets<O: ArrowNativeType>(data: &ArrayData) -> OffsetBuffer<O> {
     }
 }
 
+/// Helper function that creates an [`OffsetBuffer`] from a buffer and array offset/ length
+///
+/// # Safety
+///
+/// - buffer must contain valid arrow offsets ( [`OffsetBuffer`] ) for the
+///   given length and offset.
+unsafe fn get_offsets_from_buffer<O: ArrowNativeType>(
+    buffer: Buffer,
+    offset: usize,
+    len: usize,
+) -> OffsetBuffer<O> {
+    if len == 0 && buffer.is_empty() {
+        return OffsetBuffer::new_empty();
+    }
+
+    let scalar_buffer = ScalarBuffer::new(buffer, offset, len + 1);
+    // Safety:
+    // Arguments were valid
+    unsafe { OffsetBuffer::new_unchecked(scalar_buffer) }
+}
+
 /// Helper function for printing potentially long arrays.
 fn print_long_array<A, F>(array: &A, f: &mut std::fmt::Formatter, print_item: F) -> std::fmt::Result
 where

arrow-array/src/array/mod.rs

@@ -1597,18 +1597,21 @@ impl<T: ArrowTimestampType> PrimitiveArray<T> {
 /// Constructs a `PrimitiveArray` from an array data reference.
 impl<T: ArrowPrimitiveType> From<ArrayData> for PrimitiveArray<T> {
     fn from(data: ArrayData) -> Self {
-        Self::assert_compatible(data.data_type());
+        let (data_type, len, nulls, offset, mut buffers, _child_data) = data.into_parts();
+
+        Self::assert_compatible(&data_type);
         assert_eq!(
-            data.buffers().len(),
+            buffers.len(),
             1,
             "PrimitiveArray data should contain a single buffer only (values buffer)"
         );
+        let buffer = buffers.pop().expect("checked above");
 
-        let values = ScalarBuffer::new(data.buffers()[0].clone(), data.offset(), data.len());
+        let values = ScalarBuffer::new(buffer, offset, len);
         Self {
-            data_type: data.data_type().clone(),
+            data_type,
             values,
-            nulls: data.nulls().cloned(),
+            nulls,
         }
     }
 }