Jefffrey commented on code in PR #6939:
URL: https://github.com/apache/arrow-rs/pull/6939#discussion_r1903194986
##########
arrow-buffer/src/buffer/immutable.rs:
##########
@@ -414,7 +454,29 @@ impl<T: ArrowNativeType> From<ScalarBuffer<T>> for Buffer {
}
}
-/// Creating a `Buffer` instance by storing the boolean values into the buffer
+/// Convert from internal `Bytes`, not [`bytes::Bytes`] to `Buffer`
Review Comment:
```suggestion
/// Convert from internal `Bytes` (not [`bytes::Bytes`]) to `Buffer`
```
##########
arrow-buffer/src/buffer/immutable.rs:
##########
@@ -414,7 +454,29 @@ impl<T: ArrowNativeType> From<ScalarBuffer<T>> for Buffer {
}
}
-/// Creating a `Buffer` instance by storing the boolean values into the buffer
+/// Convert from internal `Bytes`, not [`bytes::Bytes`] to `Buffer`
+impl From<Bytes> for Buffer {
+ #[inline]
+ fn from(bytes: Bytes) -> Self {
+ let length = bytes.len();
+ let ptr = bytes.as_ptr();
+ Self {
+ data: Arc::new(bytes),
+ ptr,
+ length,
+ }
+ }
+}
+
+/// Convert from [`bytes::Bytes`], not internal `Bytes` to `Buffer`
Review Comment:
```suggestion
/// Convert from [`bytes::Bytes`] (not internal `Bytes`) to `Buffer`
```
##########
arrow-buffer/src/buffer/immutable.rs:
##########
@@ -206,7 +235,9 @@ impl Buffer {
}
/// Returns a new [Buffer] that is a slice of this buffer starting at
`offset`.
- /// Doing so allows the same memory region to be shared between buffers.
+ ///
+ /// This function is `O(1)` and does not copy any data, and allows the
+ /// same memory region to be shared between buffers.
Review Comment:
```suggestion
/// This function is `O(1)` and does not copy any data, allowing the
/// same memory region to be shared between buffers.
```
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -316,9 +316,8 @@ impl ByteViewArrayDecoderPlain {
}
pub fn read(&mut self, output: &mut ViewBuffer, len: usize) ->
Result<usize> {
- // Here we convert `bytes::Bytes` into `arrow_buffer::Bytes`, which is
zero copy
- // Then we convert `arrow_buffer::Bytes` into `arrow_buffer:Buffer`,
which is also zero copy
- let buf = arrow_buffer::Buffer::from_bytes(self.buf.clone().into());
+ // Zero copy convert `bytes::Bytes` into `arrow_buffer:Buffer`
Review Comment:
```suggestion
// Zero copy convert `bytes::Bytes` into `arrow_buffer::Buffer`
```
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -549,9 +548,8 @@ impl ByteViewArrayDecoderDeltaLength {
let src_lengths = &self.lengths[self.length_offset..self.length_offset
+ to_read];
- // Here we convert `bytes::Bytes` into `arrow_buffer::Bytes`, which is
zero copy
- // Then we convert `arrow_buffer::Bytes` into `arrow_buffer:Buffer`,
which is also zero copy
- let bytes = arrow_buffer::Buffer::from_bytes(self.data.clone().into());
+ // Zero copy convert `bytes::Bytes` into `arrow_buffer:Buffer`
Review Comment:
```suggestion
// Zero copy convert `bytes::Bytes` into `arrow_buffer::Buffer`
```
##########
arrow-buffer/src/buffer/immutable.rs:
##########
@@ -240,7 +271,10 @@ impl Buffer {
/// Returns a new [Buffer] that is a slice of this buffer starting at
`offset`,
/// with `length` bytes.
- /// Doing so allows the same memory region to be shared between buffers.
+ ///
+ /// This function is `O(1)` and does not copy any data and allows the same
+ /// memory region to be shared between buffers.
Review Comment:
```suggestion
/// This function is `O(1)` and does not copy any data, allowing the same
/// memory region to be shared between buffers.
```
--
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]
