From 27ca289c565d1ad4531309a629ba83700d699778 Mon Sep 17 00:00:00 2001 From: psvri Date: Thu, 25 Aug 2022 00:31:17 +0530 Subject: [PATCH] Code cleanup of array value functions --- arrow/src/array/array_binary.rs | 13 +++++++++++-- arrow/src/array/array_boolean.rs | 22 +++++++++++++++++++--- arrow/src/array/array_decimal.rs | 19 ++++++++++++++++++- arrow/src/array/array_fixed_size_binary.rs | 17 ++++++++++++++++- arrow/src/array/array_primitive.rs | 21 ++++++++++++++++++--- arrow/src/array/array_string.rs | 13 +++++++++++-- arrow/src/array/array_union.rs | 12 +++++------- 7 files changed, 98 insertions(+), 19 deletions(-) diff --git a/arrow/src/array/array_binary.rs b/arrow/src/array/array_binary.rs index 8cc5be8efc6b..1c63e8e24b29 100644 --- a/arrow/src/array/array_binary.rs +++ b/arrow/src/array/array_binary.rs @@ -99,8 +99,15 @@ impl GenericBinaryArray { } /// Returns the element at index `i` as bytes slice + /// # Panics + /// Panics if index `i` is out of bounds. pub fn value(&self, i: usize) -> &[u8] { - assert!(i < self.data.len(), "BinaryArray out of bounds access"); + assert!( + i < self.data.len(), + "Trying to access an element at index {} from a BinaryArray of length {}", + i, + self.len() + ); //Soundness: length checked above, offset buffer length is 1 larger than logical array length let end = unsafe { self.value_offsets().get_unchecked(i + 1) }; let start = unsafe { self.value_offsets().get_unchecked(i) }; @@ -806,7 +813,9 @@ mod tests { } #[test] - #[should_panic(expected = "BinaryArray out of bounds access")] + #[should_panic( + expected = "Trying to access an element at index 4 from a BinaryArray of length 3" + )] fn test_binary_array_get_value_index_out_of_bound() { let values: [u8; 12] = [104, 101, 108, 108, 111, 112, 97, 114, 113, 117, 101, 116]; diff --git a/arrow/src/array/array_boolean.rs b/arrow/src/array/array_boolean.rs index cb1cd11db12c..7ea18ea62036 100644 --- a/arrow/src/array/array_boolean.rs +++ b/arrow/src/array/array_boolean.rs @@ -115,10 +115,15 @@ impl BooleanArray { } /// Returns the boolean value at index `i`. - /// - /// Panics of offset `i` is out of bounds + /// # Panics + /// Panics if index `i` is out of bounds pub fn value(&self, i: usize) -> bool { - assert!(i < self.len()); + assert!( + i < self.len(), + "Trying to access an element at index {} from a BooleanArray of length {}", + i, + self.len() + ); // Safety: // `i < self.len() unsafe { self.value_unchecked(i) } @@ -389,6 +394,17 @@ mod tests { } } + #[test] + #[should_panic( + expected = "Trying to access an element at index 4 from a BooleanArray of length 3" + )] + fn test_fixed_size_binary_array_get_value_index_out_of_bound() { + let v = vec![Some(true), None, Some(false)]; + let array = v.into_iter().collect::(); + + array.value(4); + } + #[test] #[should_panic(expected = "BooleanArray data should contain a single buffer only \ (values buffer)")] diff --git a/arrow/src/array/array_decimal.rs b/arrow/src/array/array_decimal.rs index b8b468164187..543fda1b1a8a 100644 --- a/arrow/src/array/array_decimal.rs +++ b/arrow/src/array/array_decimal.rs @@ -108,8 +108,15 @@ impl DecimalArray { } /// Returns the element at index `i`. + /// # Panics + /// Panics if index `i` is out of bounds. pub fn value(&self, i: usize) -> Decimal { - assert!(i < self.data().len(), "Out of bounds access"); + assert!( + i < self.data().len(), + "Trying to access an element at index {} from a DecimalArray of length {}", + i, + self.len() + ); unsafe { self.value_unchecked(i) } } @@ -952,4 +959,14 @@ mod tests { assert_eq!(101_i128, array.value(2).into()); assert!(!array.is_null(2)); } + + #[test] + #[should_panic( + expected = "Trying to access an element at index 4 from a DecimalArray of length 3" + )] + fn test_fixed_size_binary_array_get_value_index_out_of_bound() { + let array = Decimal128Array::from_iter_values(vec![-100, 0, 101].into_iter()); + + array.value(4); + } } diff --git a/arrow/src/array/array_fixed_size_binary.rs b/arrow/src/array/array_fixed_size_binary.rs index fcf8eaa76e9e..22eac1435a8d 100644 --- a/arrow/src/array/array_fixed_size_binary.rs +++ b/arrow/src/array/array_fixed_size_binary.rs @@ -60,10 +60,14 @@ pub struct FixedSizeBinaryArray { impl FixedSizeBinaryArray { /// Returns the element at index `i` as a byte slice. + /// # Panics + /// Panics if index `i` is out of bounds. pub fn value(&self, i: usize) -> &[u8] { assert!( i < self.data.len(), - "FixedSizeBinaryArray out of bounds access" + "Trying to access an element at index {} from a FixedSizeBinaryArray of length {}", + i, + self.len() ); let offset = i + self.data.offset(); unsafe { @@ -672,4 +676,15 @@ mod tests { // Should not panic RecordBatch::try_new(Arc::new(schema), vec![Arc::new(item)]).unwrap(); } + + #[test] + #[should_panic( + expected = "Trying to access an element at index 4 from a FixedSizeBinaryArray of length 3" + )] + fn test_fixed_size_binary_array_get_value_index_out_of_bound() { + let values = vec![Some("one".as_bytes()), Some(b"two"), None]; + let array = FixedSizeBinaryArray::from(values); + + array.value(4); + } } diff --git a/arrow/src/array/array_primitive.rs b/arrow/src/array/array_primitive.rs index cc85d2a7c923..7818e6ff01d5 100644 --- a/arrow/src/array/array_primitive.rs +++ b/arrow/src/array/array_primitive.rs @@ -106,11 +106,16 @@ impl PrimitiveArray { } /// Returns the primitive value at index `i`. - /// - /// Panics of offset `i` is out of bounds + /// # Panics + /// Panics if index `i` is out of bounds #[inline] pub fn value(&self, i: usize) -> T::Native { - assert!(i < self.len()); + assert!( + i < self.len(), + "Trying to access an element at index {} from a PrimitiveArray of length {}", + i, + self.len() + ); unsafe { self.value_unchecked(i) } } @@ -1128,4 +1133,14 @@ mod tests { assert_eq!(2, b.value(0)); assert_eq!(15, b.value(1)); } + + #[test] + #[should_panic( + expected = "Trying to access an element at index 4 from a PrimitiveArray of length 3" + )] + fn test_string_array_get_value_index_out_of_bound() { + let array: Int8Array = [10_i8, 11, 12].into_iter().collect(); + + array.value(4); + } } diff --git a/arrow/src/array/array_string.rs b/arrow/src/array/array_string.rs index b72152cc4acd..5dde2ea648d8 100644 --- a/arrow/src/array/array_string.rs +++ b/arrow/src/array/array_string.rs @@ -113,9 +113,16 @@ impl GenericStringArray { } /// Returns the element at index `i` as &str + /// # Panics + /// Panics if index `i` is out of bounds. #[inline] pub fn value(&self, i: usize) -> &str { - assert!(i < self.data.len(), "StringArray out of bounds access"); + assert!( + i < self.data.len(), + "Trying to access an element at index {} from a StringArray of length {}", + i, + self.len() + ); // Safety: // `i < self.data.len() unsafe { self.value_unchecked(i) } @@ -521,7 +528,9 @@ mod tests { } #[test] - #[should_panic(expected = "StringArray out of bounds access")] + #[should_panic( + expected = "Trying to access an element at index 4 from a StringArray of length 3" + )] fn test_string_array_get_value_index_out_of_bound() { let values: [u8; 12] = [ b'h', b'e', b'l', b'l', b'o', b'p', b'a', b'r', b'q', b'u', b'e', b't', diff --git a/arrow/src/array/array_union.rs b/arrow/src/array/array_union.rs index 58444b9a8fd1..b221239b2dbe 100644 --- a/arrow/src/array/array_union.rs +++ b/arrow/src/array/array_union.rs @@ -261,14 +261,12 @@ impl UnionArray { } } - /// Returns the array's value at `index`. - /// + /// Returns the array's value at index `i`. /// # Panics - /// - /// Panics if `index` is greater than the length of the array. - pub fn value(&self, index: usize) -> ArrayRef { - let type_id = self.type_id(index); - let value_offset = self.value_offset(index) as usize; + /// Panics if index `i` is out of bounds + pub fn value(&self, i: usize) -> ArrayRef { + let type_id = self.type_id(i); + let value_offset = self.value_offset(i) as usize; let child_data = self.boxed_fields[type_id as usize].clone(); child_data.slice(value_offset, 1) }