Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code cleanup of array value functions #2583

Merged
merged 1 commit into from
Aug 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions arrow/src/array/array_binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) };
Expand Down Expand Up @@ -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];
Expand Down
22 changes: 19 additions & 3 deletions arrow/src/array/array_boolean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down Expand Up @@ -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)")]
Expand Down
19 changes: 18 additions & 1 deletion arrow/src/array/array_decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
}
Expand Down Expand Up @@ -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);
}
}
17 changes: 16 additions & 1 deletion arrow/src/array/array_fixed_size_binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}
21 changes: 18 additions & 3 deletions arrow/src/array/array_primitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
}

Expand Down Expand Up @@ -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);
}
}
13 changes: 11 additions & 2 deletions arrow/src/array/array_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down Expand Up @@ -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',
Expand Down
12 changes: 5 additions & 7 deletions arrow/src/array/array_union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down