This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch 58_maintenance
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/58_maintenance by this push:
new 0d688a3f51 [58_maintenance] fix: `Buffer::into_mutable` return error
instead of panic for converting owned sliced when not start at 0 and fix
returned Mutable length (#11706)
0d688a3f51 is described below
commit 0d688a3f517dca80dbb161c7f3e7b252b3c6f29f
Author: Jeffrey Vo <[email protected]>
AuthorDate: Thu Oct 22 04:22:15 2026 +0900
[58_maintenance] fix: `Buffer::into_mutable` return error instead of panic
for converting owned sliced when not start at 0 and fix returned Mutable length
(#11706)
- Backport https://github.com/apache/arrow-rs/pull/10118
- For inclusion in https://github.com/apache/arrow-rs/issues/11665
Cherry-pick of
https://github.com/apache/arrow-rs/commit/4e5583273fb8f308eeb7d21dab979cd4268a502f
Co-authored-by: Raz Luvaton <[email protected]>
---
arrow-buffer/src/buffer/immutable.rs | 81 ++++++++++++++++++++++++++++++------
1 file changed, 68 insertions(+), 13 deletions(-)
diff --git a/arrow-buffer/src/buffer/immutable.rs
b/arrow-buffer/src/buffer/immutable.rs
index a73cc55086..74da6fb821 100644
--- a/arrow-buffer/src/buffer/immutable.rs
+++ b/arrow-buffer/src/buffer/immutable.rs
@@ -369,8 +369,8 @@ impl Buffer {
UnalignedBitChunk::new(self.as_slice(), offset, len).count_ones()
}
- /// Returns `MutableBuffer` for mutating the buffer if this buffer is not
shared.
- /// Returns `Err` if this is shared or its allocation is from an external
source or
+ /// Returns `MutableBuffer` for mutating the buffer if this buffer is not
shared or sliced.
+ /// Returns `Err` if this is shared or the [`Self::ptr_offset`] is greater
than 0 or its allocation is from an external source or
/// it is not allocated with alignment [`ALIGNMENT`]
///
/// # Example: Creating a [`MutableBuffer`] from a [`Buffer`]
@@ -394,17 +394,30 @@ impl Buffer {
pub fn into_mutable(self) -> Result<MutableBuffer, Self> {
let ptr = self.ptr;
let length = self.length;
- Arc::try_unwrap(self.data)
- .and_then(|bytes| {
- // The pointer of underlying buffer should not be offset.
- assert_eq!(ptr, bytes.ptr().as_ptr());
- MutableBuffer::from_bytes(bytes).map_err(Arc::new)
- })
- .map_err(|bytes| Buffer {
- data: bytes,
- ptr,
- length,
- })
+
+ // Disallow converting when the offset is not 0 because we can't start
a mutable from offset
+ let res = if self.ptr_offset() > 0 {
+ Err(self.data)
+ } else {
+ Arc::try_unwrap(self.data)
+ };
+
+ res.and_then(|bytes| {
+ // The pointer of underlying buffer should not be offset.
+ assert_eq!(ptr, bytes.ptr().as_ptr());
+ MutableBuffer::from_bytes(bytes).map_err(Arc::new)
+ })
+ .map_err(|bytes| Buffer {
+ data: bytes,
+ ptr,
+ length,
+ })
+ .map(|mut mutable| {
+ // We need to update the length since in case we are converting
sliced buffer we need to return MutableBuffer with the same length
+ // SAFETY: this is safe as the length is coming from valid Buffer
(this) and it is guaranteed to be less than the underlying data buffer
+ unsafe { mutable.set_len(length) };
+ mutable
+ })
}
/// Converts self into a `Vec`, if possible.
@@ -1091,4 +1104,46 @@ mod tests {
drop(capture);
assert_eq!(buffer2.strong_count(), 1);
}
+
+ #[test]
+ fn
into_mutable_should_return_error_for_sliced_owned_buffer_when_ptr_offset_is_not_0()
{
+ let original_buffer_data = [1_u8, 2, 3, 4, 5, 6, 7, 8];
+ for (slice_from, slice_length) in [
+ (original_buffer_data.len(), 0),
+ (2, 4),
+ (2, original_buffer_data.len() - 2),
+ ] {
+ let buffer = Buffer::from(original_buffer_data);
+ let sliced = buffer.slice_with_length(slice_from, slice_length);
+ drop(buffer); // Keep only 1 owner
+ assert_ne!(sliced.ptr_offset(), 0);
+
+ sliced
+ .into_mutable()
+ .expect_err("should not convert sliced buffer when ptr_offset
is not 0");
+ }
+ }
+
+ #[test]
+ fn
into_mutable_should_allow_converting_sliced_owned_buffer_when_ptr_offset_is_0()
{
+ let original_buffer_data = [1_u8, 2, 3, 4, 5, 6, 7, 8];
+ for slice_length in [0, original_buffer_data.len() - 2] {
+ let buffer = Buffer::from(original_buffer_data);
+ let sliced = buffer.slice_with_length(0, slice_length);
+ drop(buffer); // Keep only 1 owner
+ assert_eq!(sliced.ptr_offset(), 0);
+
+ let original_ptr = sliced.data_ptr();
+
+ let mutable = sliced
+ .into_mutable()
+ .unwrap_or_else(|_| panic!("should allow converting to
mutable when not sliced from end when slice_length is {slice_length}"));
+
+ let buffer_back: Buffer = mutable.into();
+ assert_eq!(buffer_back.data_ptr(), original_ptr);
+
+ let expected =
Buffer::from(original_buffer_data).slice_with_length(0, slice_length);
+ assert_eq!(buffer_back.as_slice(), expected.as_slice());
+ }
+ }
}
