Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions arrow-array/src/array/byte_view_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,14 +238,11 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
/// # Safety
///
/// Safe if [`Self::try_new`] would not error
pub unsafe fn new_unchecked<U>(
pub unsafe fn new_unchecked(
views: ScalarBuffer<u128>,
buffers: U,
buffers: Arc<[Buffer]>,
nulls: Option<NullBuffer>,
) -> Self
where
U: Into<Arc<[Buffer]>>,
{
) -> Self {
if cfg!(feature = "force_validate") {
return Self::new(views, buffers, nulls);
}
Expand All @@ -254,7 +251,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
data_type: T::DATA_TYPE,
phantom: Default::default(),
views,
buffers: buffers.into(),
buffers,
nulls,
}
}
Expand Down Expand Up @@ -300,9 +297,13 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
&self.views
}

/// Returns the buffers storing string data
/// Returns the shared collection of buffers storing non-inline string or binary data.
///
/// 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`].

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

#[inline]
pub fn data_buffers(&self) -> &[Buffer] {
pub fn data_buffers(&self) -> &Arc<[Buffer]> {
&self.buffers
}

Expand Down Expand Up @@ -538,7 +539,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
return unsafe {
GenericByteViewArray::new_unchecked(
self.views().clone(),
vec![], // empty data blocks
Arc::from([]), // empty data blocks
nulls,
)
};
Expand All @@ -553,7 +554,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
return unsafe {
GenericByteViewArray::new_unchecked(
self.views().clone(),
vec![], // empty data blocks
Arc::from([]), // empty data blocks
nulls,
)
};
Expand Down Expand Up @@ -652,7 +653,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
let views_scalar = ScalarBuffer::from(views_buf);

// SAFETY: views_scalar, data_blocks, and nulls are correctly aligned and sized
unsafe { GenericByteViewArray::new_unchecked(views_scalar, data_blocks, nulls) }
unsafe { GenericByteViewArray::new_unchecked(views_scalar, data_blocks.into(), nulls) }
}

/// Copy the i‑th view into `data_buf` if it refers to an out‑of‑line buffer.
Expand Down Expand Up @@ -1631,7 +1632,7 @@ mod tests {
gced.data_buffers().len()
);
// No output buffer may exceed the cap.
for buf in gced.data_buffers() {
for buf in gced.data_buffers().iter() {
assert!(buf.len() <= max_buffer_size, "buffer exceeded max size");
}
// Every value (inline, large, and null) is unchanged and in order.
Expand Down
4 changes: 2 additions & 2 deletions arrow-array/src/builder/generic_bytes_view_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewBuilder<T> {
}
let views = std::mem::take(&mut self.views_buffer);
// SAFETY: valid by construction
unsafe { GenericByteViewArray::new_unchecked(views.into(), completed, nulls) }
unsafe { GenericByteViewArray::new_unchecked(views.into(), completed.into(), nulls) }
}

/// Builds the [`GenericByteViewArray`] without resetting the builder
Expand All @@ -514,7 +514,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewBuilder<T> {
let views = ScalarBuffer::new(views, 0, len);
let nulls = self.null_buffer_builder.finish_cloned();
// SAFETY: valid by construction
unsafe { GenericByteViewArray::new_unchecked(views, completed, nulls) }
unsafe { GenericByteViewArray::new_unchecked(views, completed.into(), nulls) }
}

/// Returns the current null buffer as a slice
Expand Down
2 changes: 1 addition & 1 deletion arrow-array/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is only a test, but this is a nice improvement

let nulls = empty.nulls().cloned();

// Create a dangling pointer to a view buffer with zero length.
Expand Down
2 changes: 1 addition & 1 deletion arrow-ipc/src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3567,7 +3567,7 @@ mod tests {
let array = unsafe {
StringViewArray::new_unchecked(
binary_view_array.views().clone(),
binary_view_array.data_buffers().to_vec(),
Arc::clone(binary_view_array.data_buffers()),
binary_view_array.nulls().cloned(),
)
};
Expand Down
2 changes: 1 addition & 1 deletion arrow-row/src/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ fn decode_binary_view_inner<const VALIDATE_UTF8: bool>(

// SAFETY:
// Valid by construction above
unsafe { BinaryViewArray::new_unchecked(views.into(), [values.into()], nulls) }
unsafe { BinaryViewArray::new_unchecked(views.into(), [values.into()].into(), nulls) }
}

/// Decodes a binary view array from `rows` with the provided `options`
Expand Down
5 changes: 3 additions & 2 deletions arrow-select/src/coalesce/byte_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,8 +501,9 @@ impl<B: ByteViewType> InProgressArray for InProgressByteViewArray<B> {

// Safety: we created valid views and buffers above and the
// input arrays had value data and nulls
let new_array =
unsafe { GenericByteViewArray::<B>::new_unchecked(views.into(), buffers, nulls) };
let new_array = unsafe {
GenericByteViewArray::<B>::new_unchecked(views.into(), buffers.into(), nulls)
};
Ok(Arc::new(new_array))
}

Expand Down
5 changes: 4 additions & 1 deletion arrow-select/src/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ fn filter_byte_view<T: ByteViewType>(
) -> GenericByteViewArray<T> {
let new_view_buffer = filter_native(array.views(), predicate);
let views = ScalarBuffer::new(new_view_buffer, 0, predicate.count);
let buffers = array.data_buffers().to_vec();
let buffers = Arc::clone(array.data_buffers());
let nulls = predicate.filter_nulls(array.nulls());

// SAFETY: each view is copied unchanged from `array.views()` and `buffers`
Expand Down Expand Up @@ -1297,6 +1297,9 @@ mod tests {
let actual = filter(&array, &predicate).unwrap();

assert_eq!(actual.len(), 3);
let actual_buffers = actual.as_byte_view::<T>().data_buffers();
let input_buffers = array.data_buffers();
assert!(Arc::ptr_eq(actual_buffers, input_buffers));

let expected = {
// ["hello", null, "large payload over 12 bytes"]
Expand Down
2 changes: 1 addition & 1 deletion arrow-select/src/interleave.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ fn interleave_views<T: ByteViewType>(
.collect();

let array = unsafe {
GenericByteViewArray::<T>::new_unchecked(views.into(), buffers, interleaved.nulls)
GenericByteViewArray::<T>::new_unchecked(views.into(), buffers.into(), interleaved.nulls)
};
Ok(Arc::new(array))
}
Expand Down
8 changes: 5 additions & 3 deletions arrow-select/src/take.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,10 +636,9 @@ fn take_byte_view<T: ByteViewType, IndexType: ArrowPrimitiveType>(
) -> Result<GenericByteViewArray<T>, ArrowError> {
let new_views = take_native(array.views(), indices);
let new_nulls = take_nulls(array.nulls(), indices);
let buffers = Arc::clone(array.data_buffers());
// Safety: array.views was valid, and take_native copies only valid values, and verifies bounds
Ok(unsafe {
GenericByteViewArray::new_unchecked(new_views, array.data_buffers().to_vec(), new_nulls)
})
Ok(unsafe { GenericByteViewArray::new_unchecked(new_views, buffers, new_nulls) })
}

/// `take` implementation for list arrays
Expand Down Expand Up @@ -1806,6 +1805,9 @@ mod tests {
let actual = take(&array, &index, None).unwrap();

assert_eq!(actual.len(), index.len());
let actual_buffers = actual.as_byte_view::<T>().data_buffers();
let input_buffers = array.data_buffers();
assert!(Arc::ptr_eq(actual_buffers, input_buffers));

let expected = {
// ["large payload over 12 bytes", null, "world", "large payload over 12 bytes", "lulu", null]
Expand Down
5 changes: 3 additions & 2 deletions parquet/src/arrow/buffer/view_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,15 @@ impl ViewBuffer {
let len = self.views.len();
let views = ScalarBuffer::from(self.views);
let nulls = null_buffer.and_then(|b| NullBuffer::from_unsliced_buffer(b, len));
let buffers = self.buffers.into();
match data_type {
ArrowType::Utf8View => {
// Safety: views were created correctly, and checked that the data is utf8 when building the buffer
unsafe { Arc::new(StringViewArray::new_unchecked(views, self.buffers, nulls)) }
unsafe { Arc::new(StringViewArray::new_unchecked(views, buffers, nulls)) }
}
ArrowType::BinaryView => {
// Safety: views were created correctly
unsafe { Arc::new(BinaryViewArray::new_unchecked(views, self.buffers, nulls)) }
unsafe { Arc::new(BinaryViewArray::new_unchecked(views, buffers, nulls)) }
}
_ => panic!("Unsupported data type: {data_type}"),
}
Expand Down
2 changes: 1 addition & 1 deletion parquet/tests/arrow_reader/invalid_utf8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ fn test_invalid_utf8_string_view_array() {
let array = unsafe {
StringViewArray::new_unchecked(
array.views().clone(),
array.data_buffers().to_vec(),
Arc::clone(array.data_buffers()),
array.nulls().cloned(),
)
};
Expand Down
Loading