This is an automated email from the ASF dual-hosted git repository.
kszucs pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 3eaf46e ARROW-3090: [Rust] Accompany error messages with assertions
3eaf46e is described below
commit 3eaf46ec804236c4bf95f33ab86d59994b2a9f43
Author: Paddy Horan <[email protected]>
AuthorDate: Wed Sep 5 15:48:01 2018 +0200
ARROW-3090: [Rust] Accompany error messages with assertions
Author: Paddy Horan <[email protected]>
Closes #2514 from paddyhoran/ARROW-3090 and squashes the following commits:
709b2138 <Paddy Horan> Fixed lints.
2e8da0ee <Paddy Horan> Updating assertions with error messages
---
rust/src/array.rs | 55 ++++++++++++++++++++++++++++++++++--------------
rust/src/buffer.rs | 15 +++++++------
rust/src/builder.rs | 23 ++++++++++++++------
rust/src/record_batch.rs | 11 ++++++++--
4 files changed, 73 insertions(+), 31 deletions(-)
diff --git a/rust/src/array.rs b/rust/src/array.rs
index 0e10736..b21e604 100644
--- a/rust/src/array.rs
+++ b/rust/src/array.rs
@@ -240,9 +240,16 @@ macro_rules! def_primitive_array {
/// Constructs a `PrimitiveArray` from an array data reference.
impl<T: ArrowPrimitiveType> From<ArrayDataRef> for PrimitiveArray<T> {
fn from(data: ArrayDataRef) -> Self {
- assert_eq!(data.buffers().len(), 1);
+ assert_eq!(
+ data.buffers().len(),
+ 1,
+ "PrimitiveArray data should contain a single buffer only (values
buffer)"
+ );
let raw_values = data.buffers()[0].raw_data();
- assert!(memory::is_aligned::<u8>(raw_values, mem::align_of::<T>()));
+ assert!(
+ memory::is_aligned::<u8>(raw_values, mem::align_of::<T>()),
+ "memory is not aligned"
+ );
Self {
data,
raw_values: RawPtrBox::new(raw_values as *const T),
@@ -321,20 +328,29 @@ impl ListArray {
/// Constructs a `ListArray` from an array data reference.
impl From<ArrayDataRef> for ListArray {
fn from(data: ArrayDataRef) -> Self {
- assert_eq!(data.buffers().len(), 1);
- assert_eq!(data.child_data().len(), 1);
+ assert_eq!(
+ data.buffers().len(),
+ 1,
+ "ListArray data should contain a single buffer only (value
offsets)"
+ );
+ assert_eq!(
+ data.child_data().len(),
+ 1,
+ "ListArray should contain a single child array (values array)"
+ );
let values = make_array(data.child_data()[0].clone());
let raw_value_offsets = data.buffers()[0].raw_data();
- assert!(memory::is_aligned(
- raw_value_offsets,
- mem::align_of::<i32>()
- ));
+ assert!(
+ memory::is_aligned(raw_value_offsets, mem::align_of::<i32>()),
+ "memory is not aligned"
+ );
let value_offsets = raw_value_offsets as *const i32;
unsafe {
- assert_eq!(*value_offsets.offset(0), 0);
+ assert_eq!(*value_offsets.offset(0), 0, "offsets do not start at
zero");
assert_eq!(
*value_offsets.offset(data.len() as isize),
- values.data().len() as i32
+ values.data().len() as i32,
+ "inconsistent offsets buffer and values array"
);
}
Self {
@@ -369,7 +385,10 @@ pub struct BinaryArray {
impl BinaryArray {
/// Returns the element at index `i` as a byte slice.
pub fn get_value(&self, i: i64) -> &[u8] {
- assert!(i >= 0 && i < self.data.len());
+ assert!(
+ i >= 0 && i < self.data.len(),
+ "BinaryArray out of bounds access"
+ );
let offset = i.checked_add(self.data.offset()).unwrap();
unsafe {
let pos = self.value_offset_at(offset);
@@ -413,12 +432,16 @@ impl BinaryArray {
impl From<ArrayDataRef> for BinaryArray {
fn from(data: ArrayDataRef) -> Self {
- assert_eq!(data.buffers().len(), 2);
+ assert_eq!(
+ data.buffers().len(),
+ 2,
+ "BinaryArray data should contain 2 buffers only (offsets and
values)"
+ );
let raw_value_offsets = data.buffers()[0].raw_data();
- assert!(memory::is_aligned(
- raw_value_offsets,
- mem::align_of::<i32>()
- ));
+ assert!(
+ memory::is_aligned(raw_value_offsets, mem::align_of::<i32>()),
+ "memory not aligned"
+ );
let value_data = data.buffers()[1].raw_data();
Self {
data: data.clone(),
diff --git a/rust/src/buffer.rs b/rust/src/buffer.rs
index d05848a..f50b19c 100644
--- a/rust/src/buffer.rs
+++ b/rust/src/buffer.rs
@@ -59,7 +59,7 @@ impl Drop for BufferData {
impl Buffer {
/// Creates a buffer from an existing memory region (must already be
byte-aligned)
pub fn from_raw_parts(ptr: *const u8, len: usize) -> Self {
- assert!(memory::is_aligned(ptr, 64));
+ assert!(memory::is_aligned(ptr, 64), "memory not aligned");
let buf_data = BufferData { ptr, len };
Buffer {
data: Arc::new(buf_data),
@@ -83,9 +83,12 @@ impl Buffer {
}
/// Returns a slice of this buffer, starting from `offset`.
- pub fn slice(&self, offset: usize) -> Buffer {
- assert!(self.offset + offset <= self.len());
- Buffer {
+ pub fn slice(&self, offset: usize) -> Self {
+ assert!(
+ self.offset + offset <= self.len(),
+ "the offset of the new Buffer cannot exceed the existing length"
+ );
+ Self {
data: self.data.clone(),
offset: self.offset + offset,
}
@@ -100,8 +103,8 @@ impl Buffer {
}
/// Returns an empty buffer.
- pub fn empty() -> Buffer {
- Buffer::from_raw_parts(::std::ptr::null(), 0)
+ pub fn empty() -> Self {
+ Self::from_raw_parts(::std::ptr::null(), 0)
}
}
diff --git a/rust/src/builder.rs b/rust/src/builder.rs
index ba9422d..58eac4a 100644
--- a/rust/src/builder.rs
+++ b/rust/src/builder.rs
@@ -67,8 +67,14 @@ where
/// Get the internal byte-aligned memory buffer as a mutable slice
pub fn slice_mut(&mut self, start: usize, end: usize) -> &mut [T] {
- assert!(end <= self.capacity as usize);
- assert!(start <= end);
+ assert!(
+ end <= self.capacity as usize,
+ "the end of the slice must be within the capacity"
+ );
+ assert!(
+ start <= end,
+ "the start of the slice cannot exceed the end of the slice"
+ );
unsafe {
slice::from_raw_parts_mut(self.data.offset(start as isize), (end -
start) as usize)
}
@@ -81,13 +87,13 @@ where
/// Push a value into the builder, growing the internal buffer as needed
pub fn push(&mut self, v: T) {
- assert!(!self.data.is_null());
+ assert!(!self.data.is_null(), "cannot push onto uninitialized data");
if self.len == self.capacity {
// grow capacity by 64 bytes or double the current capacity,
whichever is greater
let new_capacity = cmp::max(64, self.capacity * 2);
self.grow(new_capacity);
}
- assert!(self.len < self.capacity);
+ assert!(self.len < self.capacity, "new length exceeds capacity");
unsafe {
*self.data.offset(self.len as isize) = v;
}
@@ -96,8 +102,11 @@ where
/// Set a value at a slot in the allocated memory without adjusting the
length
pub fn set(&mut self, i: usize, v: T) {
- assert!(!self.data.is_null());
- assert!(i < self.capacity);
+ assert!(
+ !self.data.is_null(),
+ "cannot set value if data is uninitialized"
+ );
+ assert!(i < self.capacity, "index exceeds capacity");
unsafe {
*self.data.offset(i as isize) = v;
}
@@ -144,7 +153,7 @@ where
/// Build a Buffer from the existing memory
pub fn finish(&mut self) -> Buffer {
- assert!(!self.data.is_null());
+ assert!(!self.data.is_null(), "data has not been initialized");
let p = self.data;
self.data = ptr::null_mut(); // ensure builder cannot be re-used
Buffer::from_raw_parts(p as *mut u8, self.len)
diff --git a/rust/src/record_batch.rs b/rust/src/record_batch.rs
index 4f8e82b..e3c4138 100644
--- a/rust/src/record_batch.rs
+++ b/rust/src/record_batch.rs
@@ -28,11 +28,18 @@ pub struct RecordBatch {
impl RecordBatch {
pub fn new(schema: Arc<Schema>, columns: Vec<ArrayRef>) -> Self {
// assert that there are some columns
- assert!(columns.len() > 0);
+ assert!(
+ columns.len() > 0,
+ "at least one column must be defined to create a record batch"
+ );
// assert that all columns have the same row count
let len = columns[0].data().len();
for i in 1..columns.len() {
- assert_eq!(len, columns[i].data().len());
+ assert_eq!(
+ len,
+ columns[i].data().len(),
+ "all columns in a record batch must have the same length"
+ );
}
RecordBatch { schema, columns }
}
