Rich-T-kid commented on code in PR #10737:
URL: https://github.com/apache/arrow-rs/pull/10737#discussion_r4252872023
##########
arrow-array/src/array/list_array.rs:
##########
@@ -1162,7 +1162,7 @@ mod tests {
}
#[test]
- #[should_panic(expected = "Memory pointer is not aligned with the
specified scalar type")]
+ #[should_panic(expected = "buffer is not aligned for i32 (requires 4-byte
alignment)")]
Review Comment:
I like that these erros are more defined 👍
##########
arrow-buffer/src/buffer/immutable.rs:
##########
@@ -260,31 +261,60 @@ impl Buffer {
/// # Panics
///
/// Panics iff `offset` is larger than `len`.
+ /// Use [`Self::try_slice`] for a fallible version.
pub fn slice(&self, offset: usize) -> Self {
let mut s = self.clone();
s.advance(offset);
s
}
+ /// Returns a new [Buffer] that is a slice of this buffer starting at
`offset`.
+ ///
+ /// This function is `O(1)` and does not copy any data, allowing the
+ /// same memory region to be shared between buffers.
+ ///
+ /// # Errors
+ ///
+ /// Errors iff `offset` is larger than `len`.
+ pub fn try_slice(&self, offset: usize) -> Result<Self, OutOfBoundsError> {
+ let mut s = self.clone();
+ s.try_advance(offset)?;
+ Ok(s)
+ }
+
/// Increases the offset of this buffer by `offset`
///
/// # Panics
///
/// Panics iff `offset` is larger than `len`.
+ /// Use [`Self::try_advance`] for a fallible version.
#[inline]
pub fn advance(&mut self, offset: usize) {
- assert!(
- offset <= self.length,
- "the offset of the new Buffer cannot exceed the existing length:
offset={} length={}",
- offset,
- self.length
- );
+ let length = self.length;
+ self.try_advance(offset).unwrap_or_else(|_| {
+ panic!(
+ "the offset of the new Buffer cannot exceed the existing
length: offset={offset} length={length}"
+ )
+ });
+ }
+
+ /// Increases the offset of this buffer by `offset`
+ ///
+ /// # Errors
+ ///
+ /// Errors iff `offset` is larger than `len`. The buffer is left unchanged.
+ #[inline]
+ pub fn try_advance(&mut self, offset: usize) -> Result<(),
OutOfBoundsError> {
Review Comment:
```suggestion
pub fn try_advance(&mut self, additional_offset: usize) -> Result<(),
OutOfBoundsError> {
```
just so its reads a bit easier in my opinion.
##########
arrow-schema/src/error.rs:
##########
@@ -21,9 +21,15 @@ use std::io::Write;
use std::error::Error;
+use arrow_buffer::BufferError;
+
/// Many different operations in the `arrow` crate return this error type.
#[derive(Debug)]
pub enum ArrowError {
+ /// Error returned by a buffer operation.
+ ///
+ /// Boxed to keep the size of `ArrowError` down.
Review Comment:
should be fine to remove the extra comment.
```suggestion
/// Error returned by a buffer operation.
```
--
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]
