Skip to content

fix(arrow-row): allow to convert non empty fixed size binary/list array with size length 0 and no nulls #10271

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

Merged
alamb merged 7 commits into from
Jul 2, 2026
Merged

fix(arrow-row): allow to convert non empty fixed size binary/list array with size length 0 and no nulls #10271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
872aaee
fix(arrow-row): allow to convert non empty fixed size binary/list array
rluvaton Jul 2, 2026
a67e7df
add tests
rluvaton Jul 2, 2026
904fd37
test(arrow-row): share zero-width roundtrip coverage
alamb Jul 2, 2026
b8c746b
Merge remote-tracking branch 'apache/main' into fix-row-decode-zero-w…
alamb Jul 2, 2026
0aa74ea
Revert "test(arrow-row): share zero-width roundtrip coverage"
alamb Jul 2, 2026
67530e1
reduce duplication
alamb Jul 2, 2026
baadd17
test(arrow-row): format zero-width roundtrip helper
alamb Jul 2, 2026
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
146 changes: 124 additions & 22 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 @@ -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
///
Expand All @@ -141,38 +142,67 @@ 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}"
))
})?;

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,
Expand Down Expand Up @@ -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"
);
}
}
6 changes: 4 additions & 2 deletions arrow-row/src/fixed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()
}
81 changes: 81 additions & 0 deletions arrow-row/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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]

@alamb alamb Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified that these two tests fail without the code changes


---- tests::test_zero_width_fixed_size_binary_roundtrip stdout ----

thread 'tests::test_zero_width_fixed_size_binary_roundtrip' (127439646) panicked at arrow-row/src/lib.rs:2434:17:
assertion `left == right` failed
  left: FixedSizeBinaryArray<0>
[
]
 right: FixedSizeBinaryArray<0>
[
  [],
  [],
  [],
  [],
  [],
]

---- tests::test_zero_width_fixed_size_list_roundtrip stdout ----

thread 'tests::test_zero_width_fixed_size_list_roundtrip' (127439647) panicked at arrow-row/src/lib.rs:2470:17:
assertion `left == right` failed
  left: FixedSizeListArray<0>
[
]
 right: FixedSizeListArray<0>
[
  BooleanArray
[
],
  BooleanArray
[
],
  BooleanArray
[
],
  BooleanArray
[
],
  BooleanArray
[
],
]


failures:
    tests::test_zero_width_fixed_size_binary_roundtrip
    tests::test_zero_width_fixed_size_list_roundtrip

test result: FAILED. 80 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.28s

error: test failed, to rerun pass `-p arrow-row --lib`

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![SortField::new(DataType::Decimal32(
Expand Down
6 changes: 4 additions & 2 deletions arrow-row/src/list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ pub unsafe fn decode_fixed_size_list(
)));
};

let num_rows = rows.len();
let nulls = fixed::decode_nulls(rows);

let null_element_encoded =
Expand Down Expand Up @@ -295,12 +296,13 @@ pub unsafe fn decode_fixed_size_list(
let mut children = unsafe { converter.convert_raw(&mut child_rows, validate_utf8) }?;
assert_eq!(children.len(), 1);

Ok(FixedSizeListArray::new(
FixedSizeListArray::try_new_with_length(
Arc::clone(element_field),
*size,
children.pop().unwrap(),
nulls,
))
num_rows,
)
}

/// Computes the encoded length for a single list element given its child rows.
Expand Down
Loading