diff --git a/arrow-array/src/array/fixed_size_binary_array.rs b/arrow-array/src/array/fixed_size_binary_array.rs index 3c3dc8f58920..a54496addc5d 100644 --- a/arrow-array/src/array/fixed_size_binary_array.rs +++ b/arrow-array/src/array/fixed_size_binary_array.rs @@ -96,44 +96,48 @@ pub struct FixedSizeBinaryArray { } impl FixedSizeBinaryArray { - /// Create a new [`FixedSizeBinaryArray`] with `size` element size, panicking on failure + /// Create a new [`FixedSizeBinaryArray`] with `value_length` bytes per element, panicking on + /// failure /// /// # Panics /// /// Panics if [`Self::try_new`] returns an error - pub fn new(size: i32, values: Buffer, nulls: Option) -> Self { - Self::try_new(size, values, nulls).unwrap() + pub fn new(value_length: i32, values: Buffer, nulls: Option) -> Self { + Self::try_new(value_length, values, nulls).unwrap() } /// Create a new [`Scalar`] from `value` pub fn new_scalar(value: impl AsRef<[u8]>) -> Scalar { let v = value.as_ref(); - let size = i32::try_from(v.len()).expect("FixedSizeBinaryArray value length exceeds i32"); - Scalar::new(Self::new(size, Buffer::from(v), None)) + let value_length = + i32::try_from(v.len()).expect("FixedSizeBinaryArray value length exceeds i32"); + Scalar::new(Self::new(value_length, Buffer::from(v), None)) } /// Create a new [`FixedSizeBinaryArray`] from the provided parts, returning an error on failure /// - /// Creating an arrow with `size == 0` will try to get the length from the null buffer. If - /// no null buffer is provided, the resulting array will have length zero. + /// Creating an array with `value_length == 0` will try to get the length from the null + /// buffer. If no null buffer is provided, the resulting array will have length zero. /// /// # Errors /// - /// * `size < 0` - /// * `values.len() / size != nulls.len()` - /// * `size == 0 && values.len() != 0` - /// * `len * size > i32::MAX` + /// * `value_length < 0` + /// * `values.len() / value_length != nulls.len()` + /// * `value_length == 0 && values.len() != 0` + /// * `len * value_length > i32::MAX` pub fn try_new( - size: i32, + value_length: i32, values: Buffer, nulls: Option, ) -> Result { - let data_type = DataType::FixedSizeBinary(size); - let s = size.to_usize().ok_or_else(|| { - ArrowError::InvalidArgumentError(format!("Size cannot be negative, got {size}")) + let data_type = DataType::FixedSizeBinary(value_length); + let value_size = value_length.to_usize().ok_or_else(|| { + ArrowError::InvalidArgumentError(format!( + "Value length cannot be negative, got {value_length}" + )) })?; - let len = match values.len().checked_div(s) { + let len = match values.len().checked_div(value_size) { Some(len) => { if let Some(n) = nulls.as_ref() { if n.len() != len { @@ -150,21 +154,21 @@ impl FixedSizeBinaryArray { None => { if !values.is_empty() { return Err(ArrowError::InvalidArgumentError( - "Buffer cannot have non-zero length if the item size is zero".to_owned(), + "Buffer cannot have non-zero length if the value length is zero".to_owned(), )); } - // If the item size is zero, try to determine the length from the null buffer + // If the value length is zero, try to determine the length from the null buffer nulls.as_ref().map(|n| n.len()).unwrap_or(0) } }; - Self::validate_lengths(s, len)?; + Self::validate_lengths(value_size, len)?; Ok(Self { data_type, value_data: values, - value_length: size, + value_length, nulls, len, }) @@ -203,21 +207,21 @@ impl FixedSizeBinaryArray { /// /// Panics if /// - /// * `size < 0` - /// * `size * len` would overflow `usize` - /// * `size * len > i32::MAX` - /// * `size * len * 8` would overflow `usize` - pub fn new_null(size: i32, len: usize) -> Self { + /// * `value_length < 0` + /// * `value_length * len` would overflow `usize` + /// * `value_length * len > i32::MAX` + /// * `value_length * len * 8` would overflow `usize` + pub fn new_null(value_length: i32, len: usize) -> Self { const BITS_IN_A_BYTE: usize = 8; - let size_usize = size.to_usize().unwrap(); - Self::validate_lengths(size_usize, len).unwrap(); - let capacity_in_bytes = size_usize.checked_mul(len).unwrap(); + let value_size = value_length.to_usize().unwrap(); + Self::validate_lengths(value_size, len).unwrap(); + let capacity_in_bytes = value_size.checked_mul(len).unwrap(); let capacity_in_bits = capacity_in_bytes.checked_mul(BITS_IN_A_BYTE).unwrap(); Self { - data_type: DataType::FixedSizeBinary(size), + data_type: DataType::FixedSizeBinary(value_length), value_data: MutableBuffer::new_null(capacity_in_bits).into(), nulls: Some(NullBuffer::new_null(len)), - value_length: size, + value_length, len, } } @@ -351,7 +355,7 @@ impl FixedSizeBinaryArray { U: AsRef<[u8]>, { let mut len = 0; - let mut size = None; + let mut value_size = None; let mut byte = 0; let iter_size_hint = iter.size_hint().0; @@ -369,7 +373,7 @@ impl FixedSizeBinaryArray { if let Some(slice) = item { let slice = slice.as_ref(); - if let Some(size) = size { + if let Some(size) = value_size { if size != slice.len() { return Err(ArrowError::InvalidArgumentError(format!( "Nested array size mismatch: one is {}, and the other is {}", @@ -379,7 +383,7 @@ impl FixedSizeBinaryArray { } } else { let len = slice.len(); - size = Some(len); + value_size = Some(len); // Now that we know how large each element is we can reserve // sufficient capacity in the underlying mutable buffer for // the data. @@ -396,7 +400,7 @@ impl FixedSizeBinaryArray { } bit_util::set_bit(null_buf.as_slice_mut(), len); buffer.extend_from_slice(slice); - } else if let Some(size) = size { + } else if let Some(size) = value_size { buffer.extend_zeros(size); } else { prepend += 1; @@ -415,18 +419,18 @@ impl FixedSizeBinaryArray { let nulls = NullBuffer::from_unsliced_buffer(null_buf, len); - let size = size.unwrap_or(0); - Self::validate_lengths(size, len)?; - let size = size.try_into().map_err(|_| { + let value_size = value_size.unwrap_or(0); + Self::validate_lengths(value_size, len)?; + let value_length = value_size.try_into().map_err(|_| { ArrowError::InvalidArgumentError(format!( - "FixedSizeBinaryArray value length exceeds i32, got {size}" + "FixedSizeBinaryArray value length exceeds i32, got {value_size}" )) })?; Ok(Self { - data_type: DataType::FixedSizeBinary(size), + data_type: DataType::FixedSizeBinary(value_length), value_data: buffer.into(), nulls, - value_length: size, + value_length, len, }) } @@ -434,8 +438,8 @@ impl FixedSizeBinaryArray { /// Create an array from an iterable argument of sparse byte slices. /// Sparsity means that items returned by the iterator are optional, i.e input argument can /// contain `None` items. In cases where the iterator returns only `None` values, this - /// also takes a size parameter to ensure that the a valid FixedSizeBinaryArray is still - /// created. + /// also takes a `value_length` parameter to ensure that a valid + /// [`FixedSizeBinaryArray`] is still created. /// /// # Examples /// @@ -455,22 +459,27 @@ impl FixedSizeBinaryArray { /// # Errors /// /// Returns error if argument has length zero, or sizes of nested slices don't match. - pub fn try_from_sparse_iter_with_size(mut iter: T, size: i32) -> Result + pub fn try_from_sparse_iter_with_size( + mut iter: T, + value_length: i32, + ) -> Result where T: Iterator>, U: AsRef<[u8]>, { - let size_usize = size.to_usize().ok_or_else(|| { - ArrowError::InvalidArgumentError(format!("Size cannot be negative, got {size}")) + let value_size = value_length.to_usize().ok_or_else(|| { + ArrowError::InvalidArgumentError(format!( + "Value length cannot be negative, got {value_length}" + )) })?; let mut len = 0; let mut byte = 0; let iter_size_hint = iter.size_hint().0; let mut null_buf = MutableBuffer::new(bit_util::ceil(iter_size_hint, 8)); - let capacity = iter_size_hint.checked_mul(size_usize).ok_or_else(|| { + let capacity = iter_size_hint.checked_mul(value_size).ok_or_else(|| { ArrowError::InvalidArgumentError(format!( - "FixedSizeBinaryArray error: value size {size_usize} * len hint {iter_size_hint} exceeds usize" + "FixedSizeBinaryArray error: value size {value_size} * len hint {iter_size_hint} exceeds usize" )) })?; let mut buffer = MutableBuffer::new(capacity); @@ -485,10 +494,10 @@ impl FixedSizeBinaryArray { if let Some(slice) = item { let slice = slice.as_ref(); - if size_usize != slice.len() { + if value_size != slice.len() { return Err(ArrowError::InvalidArgumentError(format!( "Nested array size mismatch: one is {}, and the other is {}", - size, + value_length, slice.len() ))); } @@ -496,7 +505,7 @@ impl FixedSizeBinaryArray { bit_util::set_bit(null_buf.as_slice_mut(), len); buffer.extend_from_slice(slice); } else { - buffer.extend_zeros(size_usize); + buffer.extend_zeros(value_size); } len += 1; @@ -505,14 +514,14 @@ impl FixedSizeBinaryArray { })?; let nulls = NullBuffer::from_unsliced_buffer(null_buf, len); - Self::validate_lengths(size_usize, len)?; + Self::validate_lengths(value_size, len)?; Ok(Self { - data_type: DataType::FixedSizeBinary(size), + data_type: DataType::FixedSizeBinary(value_length), value_data: buffer.into(), nulls, len, - value_length: size, + value_length, }) } @@ -539,23 +548,22 @@ impl FixedSizeBinaryArray { U: AsRef<[u8]>, { let mut len = 0; - let mut size = None; + let mut value_size = None; let iter_size_hint = iter.size_hint().0; let mut buffer = MutableBuffer::new(0); iter.try_for_each(|item| -> Result<(), ArrowError> { let slice = item.as_ref(); - if let Some(size) = size { - if size != slice.len() { + if let Some(value_size) = value_size { + if value_size != slice.len() { return Err(ArrowError::InvalidArgumentError(format!( - "Nested array size mismatch: one is {}, and the other is {}", - size, + "Nested array size mismatch: one is {value_size}, and the other is {}", slice.len() ))); } } else { let len = slice.len(); - size = Some(len); + value_size = Some(len); if let Some(capacity) = iter_size_hint.checked_mul(len) { buffer.reserve(capacity); } @@ -574,18 +582,18 @@ impl FixedSizeBinaryArray { )); } - let size = size.unwrap_or(0); - Self::validate_lengths(size, len)?; - let size = size.try_into().map_err(|_| { + let value_size = value_size.unwrap_or(0); + Self::validate_lengths(value_size, len)?; + let value_length = value_size.try_into().map_err(|_| { ArrowError::InvalidArgumentError(format!( - "FixedSizeBinaryArray value length exceeds i32, got {size}" + "FixedSizeBinaryArray value length exceeds i32, got {value_size}" )) })?; Ok(Self { - data_type: DataType::FixedSizeBinary(size), + data_type: DataType::FixedSizeBinary(value_length), value_data: buffer.into(), nulls: None, - value_length: size, + value_length, len, }) } @@ -615,14 +623,14 @@ impl From for FixedSizeBinaryArray { _ => panic!("Expected data type to be FixedSizeBinary"), }; - let size = value_length + let value_size = value_length .to_usize() .expect("FixedSizeBinaryArray value length must be non-negative"); - Self::validate_lengths(size, len) + Self::validate_lengths(value_size, len) .expect("FixedSizeBinaryArray offsets must fit within i32"); let value_data = buffers[0].slice_with_length( - offset.checked_mul(size).expect("offset overflow"), - len.checked_mul(size).expect("length overflow"), + offset.checked_mul(value_size).expect("offset overflow"), + len.checked_mul(value_size).expect("length overflow"), ); Self { @@ -1157,7 +1165,7 @@ mod tests { assert_eq!( err.to_string(), - "Invalid argument error: Size cannot be negative, got -1" + "Invalid argument error: Value length cannot be negative, got -1" ); let nulls = NullBuffer::new_null(3); @@ -1182,7 +1190,7 @@ mod tests { FixedSizeBinaryArray::try_new(0, buffer, None).unwrap_err(); assert_eq!( zero_sized_with_non_empty_buffer_err.to_string(), - "Invalid argument error: Buffer cannot have non-zero length if the item size is zero" + "Invalid argument error: Buffer cannot have non-zero length if the value length is zero" ); } }