This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new d2519a1b72 fix(arrow-row): allow to convert non empty fixed size
binary/list array with size length 0 and no nulls (#10271)
d2519a1b72 is described below
commit d2519a1b721a968301b8f7ebb02af46cfa64e644
Author: Raz Luvaton <[email protected]>
AuthorDate: Fri Jul 3 00:59:24 2026 +0300
fix(arrow-row): allow to convert non empty fixed size binary/list array
with size length 0 and no nulls (#10271)
# Which issue does this PR close?
- Closes #10270.
# Rationale for this change
for bug fix - because it should allow that case
for the `FixedSizeBinaryArray::try_new_with_length` - so we can create
fixed size binary array with size 0 and no nulls with non 0 length. this
is similar to `FixedSizeListArray::try_new_with_length` already exists
# What changes are included in this PR?
fixed the bug by using `FixedSizeBinaryArray::try_new_with_length` and
`FixedSizeListArray::try_new_with_length` with the expected row count
created `FixedSizeBinaryArray::try_new_with_length` that allow getting
length
# Are these changes tested?
yes
# Are there any user-facing changes?
yes, new function `FixedSizeBinaryArray::try_new_with_length`
---------
Co-authored-by: Andrew Lamb <[email protected]>
---
arrow-array/src/array/fixed_size_binary_array.rs | 146 +++++++++++++++++++----
arrow-row/src/fixed.rs | 6 +-
arrow-row/src/lib.rs | 81 +++++++++++++
arrow-row/src/list.rs | 6 +-
4 files changed, 213 insertions(+), 26 deletions(-)
diff --git a/arrow-array/src/array/fixed_size_binary_array.rs
b/arrow-array/src/array/fixed_size_binary_array.rs
index 9bf3b6ef38..85e5d266c9 100644
--- a/arrow-array/src/array/fixed_size_binary_array.rs
+++ b/arrow-array/src/array/fixed_size_binary_array.rs
@@ -130,6 +130,7 @@ impl FixedSizeBinaryArray {
///
/// 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.
+ /// You can use [`Self::try_new_with_len`] to provide the length
///
/// # Errors
///
@@ -141,7 +142,6 @@ impl FixedSizeBinaryArray {
values: Buffer,
nulls: Option<NullBuffer>,
) -> Result<Self, ArrowError> {
- 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}"
@@ -149,30 +149,60 @@ impl FixedSizeBinaryArray {
})?;
let len = match values.len().checked_div(value_size) {
- Some(len) => {
- if let Some(n) = nulls.as_ref() {
- if n.len() != len {
- return Err(ArrowError::InvalidArgumentError(format!(
- "Incorrect length of null buffer for
FixedSizeBinaryArray, expected {} got {}",
- len,
- n.len(),
- )));
- }
- }
+ Some(len) => len,
+ None => nulls.as_ref().map(|n| n.len()).unwrap_or(0),
+ };
- len
- }
- None => {
- if !values.is_empty() {
- return Err(ArrowError::InvalidArgumentError(
- "Buffer cannot have non-zero length if the value
length is zero".to_owned(),
- ));
- }
+ Self::try_new_with_len(value_length, values, nulls, len)
+ }
+
+ /// Create a new [`FixedSizeBinaryArray`] from the provided parts and
number of elements, returning an error on failure
+ ///
+ /// This is useful when the length cannot be determinated from the
provided values (in case of `value_length == 0`) or nulls (`nulls.is_none()`).
+ ///
+ /// # Errors
+ ///
+ /// * `value_length < 0`
+ /// * `values.len() / value_length != len`
+ /// * `value_length == 0 && values.len() != 0`
+ /// * `nulls.len() != len`
+ /// * `value_length != 0 && values.len() / value_length != len`
+ pub fn try_new_with_len(
+ value_length: i32,
+ values: Buffer,
+ nulls: Option<NullBuffer>,
+ len: usize,
+ ) -> Result<Self, ArrowError> {
+ 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}"
+ ))
+ })?;
- // 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)
+ if let Some(nulls) = &nulls {
+ if nulls.len() != len {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Incorrect length of null buffer for FixedSizeBinaryArray,
expected {} got {}",
+ len,
+ nulls.len(),
+ )));
}
- };
+ }
+
+ if value_size != 0 && values.len() / value_size != len {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Incorrect length of values buffer for FixedSizeBinaryArray,
expected {} got {}",
+ len,
+ values.len() / value_size,
+ )));
+ }
+
+ if value_size == 0 && !values.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(
+ "Buffer cannot have non-zero length if the value length is
zero".to_owned(),
+ ));
+ }
Ok(Self {
data_type,
@@ -1172,4 +1202,76 @@ mod tests {
"Invalid argument error: Buffer cannot have non-zero length if the
value length is zero"
);
}
+
+ #[test]
+ fn test_try_new_with_len() {
+ let buffer = Buffer::from_vec(vec![0_u8; 10]);
+
+ let a = FixedSizeBinaryArray::try_new_with_len(2, buffer.clone(),
None, 5).unwrap();
+ assert_eq!(a.len(), 5);
+
+ let nulls = NullBuffer::new_null(5);
+ let a = FixedSizeBinaryArray::try_new_with_len(2, buffer, Some(nulls),
5).unwrap();
+ assert_eq!(a.len(), 5);
+ assert_eq!(a.null_count(), 5);
+
+ let a = FixedSizeBinaryArray::try_new_with_len(2, Buffer::default(),
None, 0).unwrap();
+ assert_eq!(a.len(), 0);
+ }
+
+ #[test]
+ fn test_try_new_with_len_zero_width() {
+ // Zero-width with no nulls: the case where the length cannot be
inferred from the parts
+ let a = FixedSizeBinaryArray::try_new_with_len(0, Buffer::default(),
None, 5).unwrap();
+ assert_eq!(a.len(), 5);
+ assert_eq!(a.null_count(), 0);
+ assert_eq!(a.values().len(), 0);
+
+ let nulls = NullBuffer::new_null(3);
+ let a =
+ FixedSizeBinaryArray::try_new_with_len(0, Buffer::default(),
Some(nulls), 3).unwrap();
+ assert_eq!(a.len(), 3);
+ assert_eq!(a.null_count(), 3);
+ }
+
+ #[test]
+ fn test_try_new_with_len_negative_value_length() {
+ let buffer = Buffer::from_vec(vec![0_u8; 10]);
+ let err = FixedSizeBinaryArray::try_new_with_len(-1, buffer, None,
5).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Invalid argument error: Value length cannot be negative, got -1"
+ );
+ }
+
+ #[test]
+ fn test_try_new_with_len_incorrect_null_buffer_length() {
+ let buffer = Buffer::from_vec(vec![0_u8; 10]);
+ let nulls = NullBuffer::new_null(3);
+ let err = FixedSizeBinaryArray::try_new_with_len(2, buffer,
Some(nulls), 5).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Invalid argument error: Incorrect length of null buffer for
FixedSizeBinaryArray, expected 5 got 3"
+ );
+ }
+
+ #[test]
+ fn test_try_new_with_len_incorrect_values_buffer_length() {
+ let buffer = Buffer::from_vec(vec![0_u8; 10]);
+ let err = FixedSizeBinaryArray::try_new_with_len(2, buffer, None,
3).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Invalid argument error: Incorrect length of values buffer for
FixedSizeBinaryArray, expected 3 got 5"
+ );
+ }
+
+ #[test]
+ fn test_try_new_with_len_zero_width_non_empty_buffer() {
+ let buffer = Buffer::from_vec(vec![0_u8; 10]);
+ let err = FixedSizeBinaryArray::try_new_with_len(0, buffer, None,
5).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Invalid argument error: Buffer cannot have non-zero length if the
value length is zero"
+ );
+ }
}
diff --git a/arrow-row/src/fixed.rs b/arrow-row/src/fixed.rs
index 4268c42ace..d4cd3f8dde 100644
--- a/arrow-row/src/fixed.rs
+++ b/arrow-row/src/fixed.rs
@@ -450,7 +450,8 @@ pub fn decode_fixed_size_binary(
if size < 0 {
panic!("cannot decode FixedSizeBinary({size})");
}
- let mut values = MutableBuffer::new(size as usize * rows.len());
+ let num_rows = rows.len();
+ let mut values = MutableBuffer::new(size as usize * num_rows);
let nulls = decode_nulls(rows);
let encoded_len = size as usize + 1;
@@ -466,5 +467,6 @@ pub fn decode_fixed_size_binary(
}
}
- FixedSizeBinaryArray::new(size, values.into(), nulls)
+ // Need to set the length since when size is 0 and no nulls the length
could not be determined by FixedSizeBinaryArray
+ FixedSizeBinaryArray::try_new_with_len(size, values.into(), nulls,
num_rows).unwrap()
}
diff --git a/arrow-row/src/lib.rs b/arrow-row/src/lib.rs
index 17ede5a0c4..d02d29e4e9 100644
--- a/arrow-row/src/lib.rs
+++ b/arrow-row/src/lib.rs
@@ -2325,6 +2325,27 @@ mod tests {
use super::*;
+ fn all_sort_options() -> [SortOptions; 4] {
+ [
+ SortOptions {
+ descending: false,
+ nulls_first: false,
+ },
+ SortOptions {
+ descending: false,
+ nulls_first: true,
+ },
+ SortOptions {
+ descending: true,
+ nulls_first: false,
+ },
+ SortOptions {
+ descending: true,
+ nulls_first: true,
+ },
+ ]
+ }
+
#[test]
fn test_fixed_width() {
let cols = [
@@ -2388,6 +2409,66 @@ mod tests {
}
}
+ fn test_roundtrip(sort_option: SortOptions, col: ArrayRef) {
+ let converter = RowConverter::new(vec![SortField::new_with_options(
+ col.data_type().clone(),
+ sort_option,
+ )])
+ .unwrap();
+ let rows = converter.convert_columns(&[Arc::clone(&col)]).unwrap();
+ let back = converter.convert_rows(&rows).unwrap();
+ assert_eq!(back.len(), 1);
+ assert_eq!(&back[0], &col);
+ back[0].to_data().validate_full().unwrap();
+ }
+
+ #[test]
+ fn test_zero_width_fixed_size_binary_roundtrip() {
+ for sort_option in all_sort_options() {
+ // With a zero byte width and no nulls, the decoded length cannot
be
+ // inferred from the value or null buffers and must come from the
row count
+ for with_null in [true, false] {
+ let nulls = if with_null {
+ Some(NullBuffer::from(vec![true, false, true, false,
true]))
+ } else {
+ None
+ };
+ let col: ArrayRef = Arc::new(
+ FixedSizeBinaryArray::try_new_with_len(0,
Buffer::default(), nulls, 5).unwrap(),
+ );
+
+ test_roundtrip(sort_option, col);
+ }
+ }
+ }
+
+ #[test]
+ fn test_zero_width_fixed_size_list_roundtrip() {
+ for sort_option in all_sort_options() {
+ // With a zero byte width and no nulls, the decoded length cannot
be
+ // inferred from the value or null buffers and must come from the
row count
+ for with_null in [true, false] {
+ let nulls = if with_null {
+ Some(NullBuffer::from(vec![true, false, true, false,
true]))
+ } else {
+ None
+ };
+ let col: ArrayRef = Arc::new(
+ FixedSizeListArray::try_new_with_length(
+ Arc::new(Field::new("item", DataType::Boolean, false)),
+ 0,
+ new_empty_array(&DataType::Boolean),
+ nulls,
+ 5,
+ )
+ .unwrap(),
+ );
+
+ test_roundtrip(sort_option, col);
+ }
+ }
+ }
+
#[test]
fn test_decimal32() {
let converter =
RowConverter::new(vec