alamb commented on code in PR #9016:
URL: https://github.com/apache/arrow-rs/pull/9016#discussion_r2635930709
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -1012,12 +1026,15 @@ where
}
impl<T: ByteViewType + ?Sized> From<GenericByteViewArray<T>> for ArrayData {
- fn from(mut array: GenericByteViewArray<T>) -> Self {
+ fn from(array: GenericByteViewArray<T>) -> Self {
let len = array.len();
- array.buffers.insert(0, array.views.into_inner());
+
+ let mut buffers = array.buffers.iter().cloned().collect::<Vec<_>>();
Review Comment:
I think you could call to_vec here
```suggestion
let mut buffers = array.buffers.into_vec();
```
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -884,12 +895,15 @@ impl<T: ByteViewType + ?Sized> Array for
GenericByteViewArray<T> {
}
fn shrink_to_fit(&mut self) {
+ /*
Review Comment:
I think you can use Arc::make_mut here to modify (or clone) the buffers here
as needed
```rust
fn shrink_to_fit(&mut self) {
self.views.shrink_to_fit();
Arc::make_mut(&mut self.buffers).iter_mut().for_each(|b|
b.shrink_to_fit());
if let Some(nulls) = &mut self.nulls {
nulls.shrink_to_fit();
}
}
```
I think the call to
```rust
self.buffers.shrink_to_fit();
```
which would shrink the Vec today doesn't really have an analog when the view
has an Arc of the slice -- we could potentially call `shrink_to_fit` that
prior to creating the Arc but I think that might be unecessary.
I would personally suggest not messing with the actual self.buffers call
--
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]
