alamb commented on code in PR #9016:
URL: https://github.com/apache/arrow-rs/pull/9016#discussion_r2635943763
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -885,8 +896,11 @@ impl<T: ByteViewType + ?Sized> Array for
GenericByteViewArray<T> {
fn shrink_to_fit(&mut self) {
self.views.shrink_to_fit();
- self.buffers.iter_mut().for_each(|b| b.shrink_to_fit());
- self.buffers.shrink_to_fit();
+
+ if let Some(buffers) = Arc::get_mut(&mut self.buffers) {
Review Comment:
I think this will only shrink the buffers when there are no other
outstanding references to this code. I think it would be better to call
`Arc::make_mut` here to ensure that the buffers get shrunken
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -279,7 +290,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
}
/// Deconstruct this array into its constituent parts
- pub fn into_parts(self) -> (ScalarBuffer<u128>, Vec<Buffer>,
Option<NullBuffer>) {
+ pub fn into_parts(self) -> (ScalarBuffer<u128>, Arc<[Buffer]>,
Option<NullBuffer>) {
Review Comment:
this I think is a breaking API change
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -188,7 +188,10 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
/// # Panics
///
/// Panics if [`GenericByteViewArray::try_new`] returns an error
- pub fn new(views: ScalarBuffer<u128>, buffers: Vec<Buffer>, nulls:
Option<NullBuffer>) -> Self {
+ pub fn new<U>(views: ScalarBuffer<u128>, buffers: U, nulls:
Option<NullBuffer>) -> Self
Review Comment:
This is very elegant. It is nice to keep this API backwards compatible
(anything that used to compile still compiles).
--
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]
