Skip to content

Commit 80f7b89

Browse files
committed
Mask null ListView take metadata
Signed-off-by: Daniel King <dan@spiraldb.com>
1 parent c76531d commit 80f7b89

2 files changed

Lines changed: 33 additions & 4 deletions

File tree

vortex-array/src/arrays/listview/compute/take.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,20 @@ fn apply_take(array: ArrayView<'_, ListView>, indices: &ArrayRef) -> VortexResul
5050
// duplicates.
5151
let nullable_new_offsets = offsets.take(indices.clone())?;
5252
let nullable_new_sizes = sizes.take(indices.clone())?;
53+
let validity_array = new_validity.to_array(indices.len());
5354

54-
// `take` returns nullable arrays; cast back to non-nullable (filling with zeros to represent
55-
// the null lists — the validity mask tracks nullness separately).
55+
// Null output rows may carry arbitrary physical offset/size payloads from either the indices
56+
// or the source rows. Mask with the final validity before filling so metadata placeholders are
57+
// safe and non-nullable.
5658
let new_offsets = match_each_integer_ptype!(nullable_new_offsets.dtype().as_ptype(), |O| {
57-
nullable_new_offsets.fill_null(Scalar::primitive(O::zero(), Nullability::NonNullable))?
59+
nullable_new_offsets
60+
.mask(validity_array.clone())?
61+
.fill_null(Scalar::primitive(O::zero(), Nullability::NonNullable))?
5862
});
5963
let new_sizes = match_each_integer_ptype!(nullable_new_sizes.dtype().as_ptype(), |S| {
60-
nullable_new_sizes.fill_null(Scalar::primitive(S::zero(), Nullability::NonNullable))?
64+
nullable_new_sizes
65+
.mask(validity_array.clone())?
66+
.fill_null(Scalar::primitive(S::zero(), Nullability::NonNullable))?
6167
});
6268

6369
// SAFETY: Take operation maintains all `ListViewArray` invariants:

vortex-array/src/arrays/listview/tests/take.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,29 @@ fn test_take_with_gaps() {
101101
);
102102
}
103103

104+
#[test]
105+
fn test_take_null_source_row_zeros_offset_size_payloads() {
106+
let elements = buffer![1i32, 2].into_array();
107+
let offsets = buffer![0u32, 999].into_array();
108+
let sizes = buffer![2u32, 999].into_array();
109+
let validity = Validity::from_iter([true, false]);
110+
let listview =
111+
unsafe { ListViewArray::new_unchecked(elements, offsets, sizes, validity) }.into_array();
112+
113+
let result = listview.take(buffer![1u32].into_array()).unwrap();
114+
let result_list = result
115+
.execute::<ListViewArray>(&mut SESSION.create_execution_ctx())
116+
.unwrap();
117+
118+
assert_eq!(result_list.offset_at(0), 0);
119+
assert_eq!(result_list.size_at(0), 0);
120+
assert!(
121+
result_list
122+
.is_invalid(0, &mut SESSION.create_execution_ctx())
123+
.unwrap()
124+
);
125+
}
126+
104127
#[test]
105128
fn test_take_constant_arrays() {
106129
// ListView-specific: Test with ConstantArray for offsets/sizes.

0 commit comments

Comments
 (0)