jhorstmann commented on code in PR #9058:
URL: https://github.com/apache/arrow-rs/pull/9058#discussion_r2683034659


##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -943,15 +943,15 @@ impl<'a, T: ByteViewType + ?Sized> IntoIterator for &'a 
GenericByteViewArray<T>
 }
 
 impl<T: ByteViewType + ?Sized> From<ArrayData> for GenericByteViewArray<T> {
-    fn from(value: ArrayData) -> Self {
-        let views = value.buffers()[0].clone();
-        let views = ScalarBuffer::new(views, value.offset(), value.len());
-        let buffers = value.buffers()[1..].to_vec();
+    fn from(data: ArrayData) -> Self {
+        let (_data_type, len, nulls, offset, mut buffers, _child_data) = 
data.into_parts();

Review Comment:
   Unrelated to the performance improvement: I think this also needs to assert 
that `data_type` equals `T::DATA_TYPE`, otherwise it allows unchecked casting 
from binary to string without utf8 validation.
   
   From a performance perspective, not sure if it makes any measurable 
difference, but after that assert, you could use `data_type` instead of 
`T::DATA_TYPE` below. That might avoid a call to `DataType::drop` .



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to