This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11359-0028aaa593bed532ce7893c5a0aca17b0a7139e9 in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 615f0bc611679a404dcb0d777ed59dd80caa986c Author: Neil Conway <[email protected]> AuthorDate: Sun Oct 4 03:24:16 2026 +0000 fix(arrow-cast): only cast visible elements for lists and maps (#11359) # Which issue does this PR close? - N/A # Rationale for this change Casting a List, LargeList, ListView or Map cast its whole child array. When the input to the cast was sliced, this was both inefficient and could also result in spurious errors, if a value outside the visible span made the cast fail (with `safe: false`). For lists and maps, we now cast only the range of child values that the rows span, and rebase the offsets. For ListView, a row can point anywhere in the values buffer, so the visible elements might not form a contiguous span. Casting only the visible values would be expensive and require copies, so we instead settle for an overestimate: we use the lowest starting offset and highest ending offset of any row. This is an improvement over the previous behavior, but it does result in unnecessarily casting values that are inside that range but not part of any row. # What changes are included in this PR? * Only look at the elements in the visible span, for each of the casts in question * The value buffer of the cast result is potentially smaller than the input value buffer, in which case we need to revise the offset buffer * Add unit tests # Are these changes tested? Yes; existing tests pass, new tests added. # Are there any significant user-facing changes? No. # AI usage Developed and revised with Claude Code (Opus 5.5). I have reviewed and understand the resulting code. --- arrow-cast/src/cast/list.rs | 81 ++++++++++++++++++++++++++----- arrow-cast/src/cast/map.rs | 14 ++++-- arrow-cast/src/cast/mod.rs | 114 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 194 insertions(+), 15 deletions(-) diff --git a/arrow-cast/src/cast/list.rs b/arrow-cast/src/cast/list.rs index 6eac84c4dd..e34f808b69 100644 --- a/arrow-cast/src/cast/list.rs +++ b/arrow-cast/src/cast/list.rs @@ -16,6 +16,7 @@ // under the License. use crate::cast::*; +use arrow_buffer::ScalarBuffer; /// Converts a non-list array to a list array where every element is a single element /// list. `NULL`s in the original array become `[NULL]` (i.e. output list array @@ -282,15 +283,72 @@ pub(crate) fn cast_list_values<O: OffsetSizeTrait>( cast_options: &CastOptions, ) -> Result<ArrayRef, ArrowError> { let list = array.as_list::<O>(); - let values = cast_with_options(list.values(), to.data_type(), cast_options)?; + let (offsets, values) = list_visible_values(list); + let values = cast_with_options(&values, to.data_type(), cast_options)?; Ok(Arc::new(GenericListArray::<O>::try_new( to.clone(), - list.offsets().clone(), + offsets, values, list.nulls().cloned(), )?)) } +/// Returns the part of `list.values()` that the rows use, with the offsets +/// adjusted to start at 0. For a sliced list, this drops the values outside +/// the slice. +fn list_visible_values<O: OffsetSizeTrait>( + list: &GenericListArray<O>, +) -> (OffsetBuffer<O>, ArrayRef) { + let offsets = list.offsets(); + let first = offsets.first(); + let values = list + .values() + .slice(first.as_usize(), (offsets.last() - first).as_usize()); + (offsets.clone().subtract(first), values) +} + +/// Returns the values of `list_view` from the lowest offset to the highest end of +/// its rows, and the offsets of the rows into those values. As rows can be in any +/// order, these values can include some that no row uses. +fn list_view_visible_values<O: OffsetSizeTrait>( + list_view: &GenericListViewArray<O>, +) -> (ScalarBuffer<O>, ArrayRef) { + let offsets = list_view.offsets(); + let sizes = list_view.sizes(); + let values = list_view.values(); + + let mut start = usize::MAX; + let mut end = 0; + for (offset, size) in offsets.iter().zip(sizes.iter()) { + if size.as_usize() > 0 { + start = start.min(offset.as_usize()); + end = end.max(offset.as_usize() + size.as_usize()); + } + } + if end == 0 { + // All rows are empty + start = 0; + } + + if start == 0 && end == values.len() { + return (offsets.clone(), Arc::clone(values)); + } + + // The offsets of empty rows can be outside the new range, so set them to 0 + let offsets = offsets + .iter() + .zip(sizes.iter()) + .map(|(offset, size)| { + if size.as_usize() > 0 { + O::usize_as(offset.as_usize() - start) + } else { + O::usize_as(0) + } + }) + .collect(); + (offsets, values.slice(start, end - start)) +} + /// Casting between list view arrays of same offset size; we cast only the inner type. pub(crate) fn cast_list_view_values<O: OffsetSizeTrait>( array: &dyn Array, @@ -298,10 +356,11 @@ pub(crate) fn cast_list_view_values<O: OffsetSizeTrait>( cast_options: &CastOptions, ) -> Result<ArrayRef, ArrowError> { let list = array.as_list_view::<O>(); - let values = cast_with_options(list.values(), to.data_type(), cast_options)?; + let (offsets, values) = list_view_visible_values(list); + let values = cast_with_options(&values, to.data_type(), cast_options)?; Ok(Arc::new(GenericListViewArray::<O>::try_new( to.clone(), - list.offsets().clone(), + offsets, list.sizes().clone(), values, list.nulls().cloned(), @@ -315,8 +374,7 @@ pub(crate) fn cast_list<I: OffsetSizeTrait, O: OffsetSizeTrait>( cast_options: &CastOptions, ) -> Result<ArrayRef, ArrowError> { let list = array.as_list::<I>(); - let values = list.values(); - let offsets = list.offsets(); + let (offsets, values) = list_visible_values(list); let nulls = list.nulls().cloned(); if offsets.last().as_usize() > O::MAX_OFFSET { @@ -328,7 +386,7 @@ pub(crate) fn cast_list<I: OffsetSizeTrait, O: OffsetSizeTrait>( } // Recursively cast values - let values = cast_with_options(values, field.data_type(), cast_options)?; + let values = cast_with_options(&values, field.data_type(), cast_options)?; let offsets: Vec<_> = offsets.iter().map(|x| O::usize_as(x.as_usize())).collect(); // Safety: valid offsets and checked for overflow @@ -409,12 +467,12 @@ pub(crate) fn cast_list_view<I: OffsetSizeTrait, O: OffsetSizeTrait>( cast_options: &CastOptions, ) -> Result<ArrayRef, ArrowError> { let list_view = array.as_list_view::<I>(); + let (offsets, values) = list_view_visible_values(list_view); // Recursively cast values - let values = cast_with_options(list_view.values(), to_field.data_type(), cast_options)?; + let values = cast_with_options(&values, to_field.data_type(), cast_options)?; - let offsets = list_view - .offsets() + let offsets = offsets .iter() .map(|offset| { let offset = offset.as_usize(); @@ -459,7 +517,8 @@ pub(crate) fn cast_list_to_list_view<I: OffsetSizeTrait, O: OffsetSizeTrait>( cast_options: &CastOptions, ) -> Result<ArrayRef, ArrowError> { let list = array.as_list::<I>(); - let (_field, offsets, values, nulls) = list.clone().into_parts(); + let (offsets, values) = list_visible_values(list); + let nulls = list.nulls().cloned(); let len = offsets.len() - 1; let mut sizes = Vec::with_capacity(len); diff --git a/arrow-cast/src/cast/map.rs b/arrow-cast/src/cast/map.rs index 7395d32efe..d4a3ae1611 100644 --- a/arrow-cast/src/cast/map.rs +++ b/arrow-cast/src/cast/map.rs @@ -37,16 +37,22 @@ pub(crate) fn cast_map_values( "map is missing value field".to_string(), ))?; - let key_array = cast_with_options(from.keys(), key_field.data_type(), cast_options)?; - let value_array = cast_with_options(from.values(), value_field.data_type(), cast_options)?; + let offsets = from.offsets(); + let first = offsets.first(); + // Only the entries that the offsets refer to, which excludes entries outside a slice + let entries = from + .entries() + .slice(first.as_usize(), (offsets.last() - first).as_usize()); + let key_array = cast_with_options(entries.column(0), key_field.data_type(), cast_options)?; + let value_array = cast_with_options(entries.column(1), value_field.data_type(), cast_options)?; Ok(Arc::new(MapArray::try_new( entries_field.clone(), - from.offsets().clone(), + offsets.clone().subtract(first), StructArray::try_new( Fields::from(vec![key_field, value_field]), vec![key_array, value_array], - from.entries().nulls().cloned(), + entries.nulls().cloned(), )?, from.nulls().cloned(), to_ordered, diff --git a/arrow-cast/src/cast/mod.rs b/arrow-cast/src/cast/mod.rs index cb347d4e98..cd858859ea 100644 --- a/arrow-cast/src/cast/mod.rs +++ b/arrow-cast/src/cast/mod.rs @@ -10381,6 +10381,120 @@ mod tests { test::<i64>(); } + /// Casts `input` to each of `targets` with `safe` true and false, and checks that the + /// result equals the cast of `expected` and has the same number of child values + fn check_cast_ignores_hidden_values( + input: &dyn Array, + expected: &dyn Array, + targets: &[DataType], + ) { + for target in targets { + for safe in [true, false] { + let options = CastOptions { + safe, + ..Default::default() + }; + let actual = cast_with_options(input, target, &options).unwrap(); + let expected = cast_with_options(expected, target, &options).unwrap(); + assert_eq!(actual.as_ref(), expected.as_ref()); + assert_eq!( + actual.to_data().child_data()[0].len(), + expected.to_data().child_data()[0].len() + ); + } + } + } + + #[test] + fn test_cast_sliced_list_ignores_hidden_values() { + fn test<O: OffsetSizeTrait>() { + // The first and last rows are outside the slice, and their values do not fit + // in Int16 + let input = GenericListArray::<O>::from_iter_primitive::<Int32Type, _, _>([ + Some(vec![Some(i32::MAX)]), + Some(vec![Some(1), Some(2)]), + Some(vec![]), + Some(vec![Some(3)]), + Some(vec![Some(i32::MAX); 2]), + ]); + let expected = GenericListArray::<O>::from_iter_primitive::<Int32Type, _, _>([ + Some(vec![Some(1), Some(2)]), + Some(vec![]), + Some(vec![Some(3)]), + ]); + // With a non-nullable child field, safe casts fail if they produce nulls + let field = Arc::new(Field::new_list_field(DataType::Int16, false)); + let targets = [ + DataType::List(field.clone()), + DataType::LargeList(field.clone()), + DataType::ListView(field.clone()), + DataType::LargeListView(field), + ]; + check_cast_ignores_hidden_values(&input.slice(1, 3), &expected, &targets); + } + test::<i32>(); + test::<i64>(); + } + + #[test] + fn test_cast_sliced_list_view_ignores_hidden_values() { + fn test<O: OffsetSizeTrait>() { + // Rows [[i32::MAX], [3], [], [1, 2], [i32::MAX]], out of order in the values. + // The empty row's offset is past the values of the other rows in the slice. + let input = GenericListViewArray::<O>::new( + Arc::new(Field::new_list_field(DataType::Int32, true)), + ScalarBuffer::from_iter([0, 3, 5, 1, 4].map(O::usize_as)), + ScalarBuffer::from_iter([1, 1, 0, 2, 1].map(O::usize_as)), + Arc::new(Int32Array::from(vec![i32::MAX, 1, 2, 3, i32::MAX])), + None, + ); + let expected = GenericListViewArray::<O>::from_iter_primitive::<Int32Type, _, _>([ + Some(vec![Some(3)]), + Some(vec![]), + Some(vec![Some(1), Some(2)]), + ]); + let field = Arc::new(Field::new_list_field(DataType::Int16, false)); + let targets = [ + DataType::ListView(field.clone()), + DataType::LargeListView(field), + ]; + check_cast_ignores_hidden_values(&input.slice(1, 3), &expected, &targets); + } + test::<i32>(); + test::<i64>(); + } + + #[test] + fn test_cast_sliced_map_ignores_hidden_entries() { + // The first and last rows are outside the slice, and their keys do not fit in Int16 + let input = MapArray::from_vec_of_maps::<Int32Array, Int32Array, _, _>( + vec![ + Some(vec![(i32::MAX, Some(0))]), + Some(vec![(1, Some(10)), (2, Some(20))]), + Some(vec![(3, Some(30))]), + Some(vec![(i32::MAX, Some(0))]), + ], + false, + ); + let expected = MapArray::from_vec_of_maps::<Int32Array, Int32Array, _, _>( + vec![ + Some(vec![(1, Some(10)), (2, Some(20))]), + Some(vec![(3, Some(30))]), + ], + false, + ); + let entries = Field::new_struct( + "entries", + vec![ + Field::new("keys", DataType::Int16, false), + Field::new("values", DataType::Int32, true), + ], + false, + ); + let target = DataType::Map(Arc::new(entries), false); + check_cast_ignores_hidden_values(&input.slice(1, 2), &expected, &[target]); + } + #[test] fn test_cast_list_to_fsl() { // There four noteworthy cases we should handle:
