alamb commented on code in PR #10708:
URL: https://github.com/apache/arrow-rs/pull/10708#discussion_r3830556306
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -300,27 +297,14 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
&self.views
}
- /// Returns the buffers storing string data
- #[inline]
- pub fn data_buffers(&self) -> &[Buffer] {
- &self.buffers
- }
-
- /// Returns a cloned `Arc` of the buffers storing non-inline string or
binary data.
+ /// Returns the shared collection of buffers storing non-inline string or
binary data.
///
- /// This is useful when needing to construct a new byte view array from
this existing
- /// array, but [`into_parts`] is not feasible (e.g. need to keep both
arrays around),
- /// and trying to reconstruct the buffers from [`data_buffers`] would
require a
- /// `Vec` allocation and cloning of each buffer element, which can be
expensive
- /// if there is a large number of buffers.
- ///
- /// This operation is `O(1)` and clones only the collection's `Arc`.
- ///
- /// [`into_parts`]: Self::into_parts
- /// [`data_buffers`]: Self::data_buffers
+ /// The returned `Arc` can be cloned to share the buffers with another
array without
+ /// allocating a new collection or cloning the individual buffers. To
consume this
+ /// array and take ownership of its buffers, use [`Self::into_parts`].
Review Comment:
👍
##########
arrow-array/src/ffi.rs:
##########
@@ -1815,7 +1815,7 @@ mod tests_from_ffi {
#[cfg(not(feature = "force_validate"))]
fn test_utf8_view_ffi_from_dangling_pointer() {
let empty = GenericByteViewBuilder::<StringViewType>::new().finish();
- let buffers = empty.data_buffers().to_vec();
+ let buffers = Arc::clone(empty.data_buffers());
Review Comment:
it is only a test, but this is a nice improvement
--
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]
