This is an automated email from the ASF dual-hosted git repository.
sunchao 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 4a0621d2a7 fix: validate union child bounds without truncating lengths
(#10847)
4a0621d2a7 is described below
commit 4a0621d2a770ba893479b63354d2b503081bec73
Author: Chao Sun <[email protected]>
AuthorDate: Tue Sep 8 13:12:09 2026 -0700
fix: validate union child bounds without truncating lengths (#10847)
## Why are the changes needed?
### Which issue does this PR close?
Closes https://github.com/apache/arrow-rs/issues/10845.
### Rationale for this change
A dense Union can have a child longer than `i32::MAX` while referencing
only valid `i32` offsets. `UnionArray::try_new` truncates child lengths
to `i32`, so the length can collide with its missing-type sentinel or
fail the bounds check. A `NullArray` reproduces this without a large
allocation.
## What changes were proposed in this PR?
### What changes are included in this PR?
Keep child lengths as `Option<usize>`, separating a missing type ID from
a valid length. Reject negative offsets before converting them to
`usize` for comparison. Add coverage for lengths `i32::MAX + 1` and
`i32::MAX + 2`, offsets zero and `i32::MAX`, and invalid negative
offsets.
### Are there any user-facing changes?
Previously rejected valid Unions are accepted. Invalid type IDs and
out-of-bounds offsets still return errors. No public API changes.
## How was this PR tested?
### Are these changes tested?
- The large-child test failed on the unchanged base with the type-ID
error, then passed with the fix; both constructed arrays pass full Arrow
validation.
- `cargo test --offline -p arrow-array --lib`: 719 passed, 1 ignored.
- `cargo clippy --offline -p arrow-array --all-targets --all-features --
-D warnings`: passed.
- `cargo fmt --all -- --check`: passed.
- Independent source review checked missing IDs, empty children, signed
offsets, and the safety of the lookup after type-ID validation.
AI assistance: Codex generated the implementation, regression tests, and
PR text, and performed the stated local checks and source review. This
does not claim a separate human review.
---
arrow-array/src/array/union_array.rs | 43 +++++++++++++++++++++++++++++++-----
1 file changed, 37 insertions(+), 6 deletions(-)
diff --git a/arrow-array/src/array/union_array.rs
b/arrow-array/src/array/union_array.rs
index 1a4d2c6654..17b740c29c 100644
--- a/arrow-array/src/array/union_array.rs
+++ b/arrow-array/src/array/union_array.rs
@@ -207,15 +207,15 @@ impl UnionArray {
// Create mapping from type id to array lengths.
let max_id = fields.iter().map(|(i, _)| i).max().unwrap_or_default()
as usize;
- let mut array_lens = vec![i32::MIN; max_id + 1];
+ let mut array_lens = vec![None; max_id + 1];
for (cd, (field_id, _)) in children.iter().zip(fields.iter()) {
- array_lens[field_id as usize] = cd.len() as i32;
+ array_lens[field_id as usize] = Some(cd.len());
}
// Type id values must match one of the fields.
for id in &type_ids {
match array_lens.get(*id as usize) {
- Some(x) if *x != i32::MIN => {}
+ Some(Some(_)) => {}
_ => {
return Err(ArrowError::InvalidArgumentError(
"Type Ids values must match one of the field type
ids".to_owned(),
@@ -227,8 +227,9 @@ impl UnionArray {
// Check the value offsets are in bounds.
if let Some(offsets) = &offsets {
let mut iter = type_ids.iter().zip(offsets.iter());
- if iter.any(|(type_id, &offset)| offset < 0 || offset >=
array_lens[*type_id as usize])
- {
+ if iter.any(|(type_id, &offset)| {
+ offset < 0 || offset as usize >= array_lens[*type_id as
usize].unwrap()
+ }) {
return Err(ArrowError::InvalidArgumentError(
"Offsets must be non-negative and within the length of the
Array".to_owned(),
));
@@ -1042,7 +1043,7 @@ mod tests {
use crate::builder::UnionBuilder;
use crate::cast::AsArray;
use crate::types::{Float32Type, Float64Type, Int32Type, Int64Type};
- use crate::{Float64Array, Int32Array, Int64Array, StringArray};
+ use crate::{Float64Array, Int32Array, Int64Array, NullArray, StringArray};
use crate::{Int8Array, RecordBatch};
use arrow_buffer::Buffer;
use arrow_schema::{Field, Schema};
@@ -1844,6 +1845,27 @@ mod tests {
assert_eq!(array.len(), 7);
}
+ #[test]
+ fn test_dense_union_large_child() {
+ let fields =
+ UnionFields::try_new([3], [Field::new("nulls", DataType::Null,
true)]).unwrap();
+
+ // NullArray represents these lengths without allocating a values
buffer.
+ for child_len in [i32::MAX as usize + 1, i32::MAX as usize + 2] {
+ let array = UnionArray::try_new(
+ fields.clone(),
+ vec![3, 3].into(),
+ Some(vec![0, i32::MAX].into()),
+ vec![Arc::new(NullArray::new(child_len))],
+ )
+ .unwrap();
+
+ assert_eq!(array.child(3).len(), child_len);
+ assert_eq!(array.value(1).len(), 1);
+ array.to_data().validate_full().unwrap();
+ }
+ }
+
#[test]
fn test_invalid() {
let fields = UnionFields::try_new(
@@ -1899,6 +1921,15 @@ mod tests {
"Invalid argument error: Offsets must be non-negative and within
the length of the Array"
);
+ let offsets = Some(vec![0, -1, 0].into());
+ let err = UnionArray::try_new(fields.clone(), type_ids.clone(),
offsets, children.clone())
+ .unwrap_err();
+
+ assert_eq!(
+ err.to_string(),
+ "Invalid argument error: Offsets must be non-negative and within
the length of the Array"
+ );
+
let offsets = Some(vec![0, 1].into());
let err =
UnionArray::try_new(fields.clone(), type_ids.clone(), offsets,
children).unwrap_err();
