This is an automated email from the ASF dual-hosted git repository.
Jefffrey 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 951d9396e1 fix(arrow-array): check logical nulls, not is_nullable, for
a non-nullable list field (#11178)
951d9396e1 is described below
commit 951d9396e11b21169671ef52d9da558a7a4f0a66
Author: Amit Vijapur <[email protected]>
AuthorDate: Fri Sep 25 09:38:34 2026 +0100
fix(arrow-array): check logical nulls, not is_nullable, for a non-nullable
list field (#11178)
# Which issue does this PR close?
Closes #6538.
# Rationale for this change
`GenericListArray::try_new` and `GenericListViewArray::try_new` reject a
non-nullable `field` whenever `values.is_nullable()` is true.
`Array::is_nullable` may return `true` conservatively, and
`DictionaryArray::is_nullable` does so when the keys carry a null
buffer, even an all-valid one, or when the values hold a null that no
key references. Such a dictionary has zero logical nulls and is rejected
anyway.
The `UnionArray` case in the issue no longer reproduces: #6540 made
`UnionArray::is_nullable` ask its children. The dictionary case still
does.
# What changes are included in this PR?
Both constructors test `values.logical_null_count() != 0` instead, which
is what `FixedSizeListArray::try_new` already checks through
`logical_nulls()`. The doc bullets are updated to match.
# Are these changes tested?
A test in each file builds a list over an `Int8DictionaryArray` whose
keys have an all-valid null buffer under a non-nullable field, which
failed before the change, and then one with a null key, which still
fails with `cannot contain nulls`. `cargo test -p arrow-array` passes.
# Are there any user-facing changes?
A list or list-view array over a dictionary with no logical nulls can
now be built under a non-nullable field. Arrays that hold a logical null
are still rejected as before.
---
arrow-array/src/array/list_array.rs | 33 ++++++++++++++++++++++++++--
arrow-array/src/array/list_view_array.rs | 37 +++++++++++++++++++++++++++++---
2 files changed, 65 insertions(+), 5 deletions(-)
diff --git a/arrow-array/src/array/list_array.rs
b/arrow-array/src/array/list_array.rs
index 430750f48a..d13675a007 100644
--- a/arrow-array/src/array/list_array.rs
+++ b/arrow-array/src/array/list_array.rs
@@ -204,7 +204,7 @@ impl<OffsetSize: OffsetSizeTrait>
GenericListArray<OffsetSize> {
///
/// * `offsets.len() - 1 != nulls.len()`
/// * `offsets.last() > values.len()`
- /// * `!field.is_nullable() && values.is_nullable()`
+ /// * `!field.is_nullable() && values.logical_null_count() != 0`
/// * `field.data_type() != values.data_type()`
pub fn try_new(
field: FieldRef,
@@ -232,7 +232,7 @@ impl<OffsetSize: OffsetSizeTrait>
GenericListArray<OffsetSize> {
n.len(),
)));
}
- if !field.is_nullable() && values.is_nullable() {
+ if !field.is_nullable() && values.logical_null_count() != 0 {
return Err(ArrowError::InvalidArgumentError(format!(
"Non-nullable field of {}ListArray {:?} cannot contain nulls",
OffsetSize::PREFIX,
@@ -1353,6 +1353,35 @@ mod tests {
);
}
+ #[test]
+ fn test_try_new_non_nullable_field_dictionary_values() {
+ let keys = Int8Array::new(vec![0i8, 1].into(),
Some(NullBuffer::new_valid(2)));
+ let values = StringArray::from(vec!["x", "y"]);
+ let dict = Int8DictionaryArray::try_new(keys,
Arc::new(values)).unwrap();
+ let field = Arc::new(Field::new("element", dict.data_type().clone(),
false));
+ ListArray::try_new(
+ field,
+ OffsetBuffer::new(vec![0, 2].into()),
+ Arc::new(dict),
+ None,
+ )
+ .unwrap();
+
+ let keys = Int8Array::from(vec![Some(0i8), None]);
+ let values = StringArray::from(vec!["x", "y"]);
+ let dict = Int8DictionaryArray::try_new(keys,
Arc::new(values)).unwrap();
+ let field = Arc::new(Field::new("element", dict.data_type().clone(),
false));
+ let err = ListArray::try_new(
+ field,
+ OffsetBuffer::new(vec![0, 2].into()),
+ Arc::new(dict),
+ None,
+ )
+ .unwrap_err();
+
+ assert!(err.to_string().contains("cannot contain nulls"));
+ }
+
#[test]
fn test_from_fixed_size_list() {
let mut builder = FixedSizeListBuilder::new(Int32Builder::new(), 3);
diff --git a/arrow-array/src/array/list_view_array.rs
b/arrow-array/src/array/list_view_array.rs
index 1983e1a841..62b8b3ed1b 100644
--- a/arrow-array/src/array/list_view_array.rs
+++ b/arrow-array/src/array/list_view_array.rs
@@ -134,7 +134,7 @@ impl<OffsetSize: OffsetSizeTrait>
GenericListViewArray<OffsetSize> {
/// * `offsets.len() != sizes.len()`
/// * `offsets.len() != nulls.len()`
/// * `offsets[i] > values.len()`
- /// * `!field.is_nullable() && values.is_nullable()`
+ /// * `!field.is_nullable() && values.logical_null_count() != 0`
/// * `field.data_type() != values.data_type()`
/// * `0 <= offsets[i] <= length of the child array`
/// * `0 <= offsets[i] + size[i] <= length of the child array`
@@ -181,7 +181,7 @@ impl<OffsetSize: OffsetSizeTrait>
GenericListViewArray<OffsetSize> {
}
}
- if !field.is_nullable() && values.is_nullable() {
+ if !field.is_nullable() && values.logical_null_count() != 0 {
return Err(ArrowError::InvalidArgumentError(format!(
"Non-nullable field of {}ListViewArray {:?} cannot contain
nulls",
OffsetSize::PREFIX,
@@ -710,7 +710,7 @@ mod tests {
use crate::builder::{FixedSizeListBuilder, Int32Builder};
use crate::cast::AsArray;
use crate::types::Int32Type;
- use crate::{Int32Array, Int64Array};
+ use crate::{Int8Array, Int8DictionaryArray, Int32Array, Int64Array,
StringArray};
use super::*;
@@ -1140,6 +1140,37 @@ mod tests {
);
}
+ #[test]
+ fn test_try_new_non_nullable_field_dictionary_values() {
+ let keys = Int8Array::new(vec![0i8, 1].into(),
Some(NullBuffer::new_valid(2)));
+ let values = StringArray::from(vec!["x", "y"]);
+ let dict = Int8DictionaryArray::try_new(keys,
Arc::new(values)).unwrap();
+ let field = Arc::new(Field::new("element", dict.data_type().clone(),
false));
+ ListViewArray::try_new(
+ field,
+ ScalarBuffer::from(vec![0]),
+ ScalarBuffer::from(vec![2]),
+ Arc::new(dict),
+ None,
+ )
+ .unwrap();
+
+ let keys = Int8Array::from(vec![Some(0i8), None]);
+ let values = StringArray::from(vec!["x", "y"]);
+ let dict = Int8DictionaryArray::try_new(keys,
Arc::new(values)).unwrap();
+ let field = Arc::new(Field::new("element", dict.data_type().clone(),
false));
+ let err = ListViewArray::try_new(
+ field,
+ ScalarBuffer::from(vec![0]),
+ ScalarBuffer::from(vec![2]),
+ Arc::new(dict),
+ None,
+ )
+ .unwrap_err();
+
+ assert!(err.to_string().contains("cannot contain nulls"));
+ }
+
#[test]
fn test_from_fixed_size_list() {
let mut builder = FixedSizeListBuilder::new(Int32Builder::new(), 3);
