Skip to content

Commit

Permalink
Remove impl<T: AsRef<[u8]>> From<T> for Buffer that easily accident…
Browse files Browse the repository at this point in the history
…ally copies data (apache#6043)

* deprecate auto copy, ask explicit reference

* update comments

* make cargo doc happy
  • Loading branch information
XiangpengHao authored Jul 16, 2024
1 parent 741bbf6 commit 8f76248
Showing 1 changed file with 23 additions and 10 deletions.
33 changes: 23 additions & 10 deletions arrow-buffer/src/buffer/immutable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,16 +356,29 @@ impl Buffer {
}
}

/// Creating a `Buffer` instance by copying the memory from a `AsRef<[u8]>` into a newly
/// allocated memory region.
impl<T: AsRef<[u8]>> From<T> for Buffer {
fn from(p: T) -> Self {
// allocate aligned memory buffer
let slice = p.as_ref();
let len = slice.len();
let mut buffer = MutableBuffer::new(len);
buffer.extend_from_slice(slice);
buffer.into()
/// Note that here we deliberately do not implement
/// `impl<T: AsRef<[u8]>> From<T> for Buffer`
/// As it would accept `Buffer::from(vec![...])` that would cause an unexpected copy.
/// Instead, we ask user to be explicit when copying is occurring, e.g., `Buffer::from(vec![...].to_byte_slice())`.
/// For zero-copy conversion, user should use `Buffer::from_vec(vec![...])`.
///
/// Since we removed impl for `AsRef<u8>`, we added the following three specific implementations to reduce API breakage.
/// See <https://github.com/apache/arrow-rs/issues/6033> for more discussion on this.
impl From<&[u8]> for Buffer {
fn from(p: &[u8]) -> Self {
Self::from_slice_ref(p)
}
}

impl<const N: usize> From<[u8; N]> for Buffer {
fn from(p: [u8; N]) -> Self {
Self::from_slice_ref(p)
}
}

impl<const N: usize> From<&[u8; N]> for Buffer {
fn from(p: &[u8; N]) -> Self {
Self::from_slice_ref(p)
}
}

Expand Down

0 comments on commit 8f76248

Please sign in to comment.