Code cleanup of array value functions (#2583)

Author: psvri

arrow/src/array/array_binary.rs

@@ -99,8 +99,15 @@ impl<OffsetSize: OffsetSizeTrait> GenericBinaryArray<OffsetSize> {
     }
 
     /// 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];

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::<BooleanArray>();
+
+        array.value(4);
+    }
+
     #[test]
     #[should_panic(expected = "BooleanArray data should contain a single buffer only \
                                (values buffer)")]

arrow/src/array/array_decimal.rs

@@ -108,8 +108,15 @@ impl<T: DecimalType> DecimalArray<T> {
     }
 
     /// Returns the element at index `i`.
+    /// # Panics
+    /// Panics if index `i` is out of bounds.
     pub fn value(&self, i: usize) -> Decimal<T> {
-        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);
+    }
 }

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);
+    }
 }

arrow/src/array/array_primitive.rs

@@ -106,11 +106,16 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
     }
 
     /// 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);
+    }
 }

arrow/src/array/array_string.rs

@@ -113,9 +113,16 @@ impl<OffsetSize: OffsetSizeTrait> GenericStringArray<OffsetSize> {
     }
 
     /// 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',

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)
     }