Skip to content

[arrow-array] rename FixedSizeBinaryArary parameters for consistency #9873

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.

Sign up for GitHub

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

Closed
alamb wants to merge 1 commit into from
Closed

[arrow-array] rename FixedSizeBinaryArary parameters for consistency #9873

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
100 changes: 52 additions & 48 deletions arrow-array/src/array/fixed_size_binary_array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,16 @@ impl FixedSizeBinaryArray {
/// # Panics
///
/// Panics if [`Self::try_new`] returns an error
pub fn new(size: i32, values: Buffer, nulls: Option<NullBuffer>) -> Self {
Self::try_new(size, values, nulls).unwrap()
pub fn new(value_length: i32, values: Buffer, nulls: Option<NullBuffer>) -> Self {
Self::try_new(value_length, values, nulls).unwrap()
}

/// Create a new [`Scalar`] from `value`
pub fn new_scalar(value: impl AsRef<[u8]>) -> Scalar<Self> {
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
Expand All @@ -87,16 +88,16 @@ impl FixedSizeBinaryArray {
/// * `size == 0 && values.len() != 0`
/// * `len * size > i32::MAX`
pub fn try_new(
size: i32,
value_length: i32,
values: Buffer,
nulls: Option<NullBuffer>,
) -> Result<Self, ArrowError> {
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!("Size 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 {
Expand All @@ -122,12 +123,12 @@ impl FixedSizeBinaryArray {
}
};

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,
})
Expand Down Expand Up @@ -166,21 +167,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,
}
}
Expand Down Expand Up @@ -378,18 +379,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 = 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,
})
}
Expand Down Expand Up @@ -418,22 +419,25 @@ 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<T, U>(mut iter: T, size: i32) -> Result<Self, ArrowError>
pub fn try_from_sparse_iter_with_size<T, U>(
mut iter: T,
value_length: i32,
) -> Result<Self, ArrowError>
where
T: Iterator<Item = Option<U>>,
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!("Size 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);
Expand All @@ -448,18 +452,18 @@ 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()
)));
}

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;
Expand All @@ -468,14 +472,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,
})
}

Expand Down Expand Up @@ -537,18 +541,18 @@ impl FixedSizeBinaryArray {
));
}

let size = size.unwrap_or(0);
Self::validate_lengths(size, len)?;
let size = size.try_into().map_err(|_| {
let value_size = 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,
})
}
Expand Down Expand Up @@ -578,14 +582,14 @@ impl From<ArrayData> 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 {
Expand Down
Loading