Skip to content

Commit 9c642f5

Browse files
committed
Preserve ListView take metadata
Signed-off-by: Daniel King <dan@spiraldb.com>
1 parent 9ffb47a commit 9c642f5

2 files changed

Lines changed: 4 additions & 33 deletions

File tree

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,14 @@ 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());
5453

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.
54+
// `take` returns nullable arrays; cast back to non-nullable (filling with zeros to represent
55+
// the null lists caused by null indices; the validity mask tracks nullness separately).
5856
let new_offsets = match_each_integer_ptype!(nullable_new_offsets.dtype().as_ptype(), |O| {
59-
nullable_new_offsets
60-
.mask(validity_array.clone())?
61-
.fill_null(Scalar::primitive(O::zero(), Nullability::NonNullable))?
57+
nullable_new_offsets.fill_null(Scalar::primitive(O::zero(), Nullability::NonNullable))?
6258
});
6359
let new_sizes = match_each_integer_ptype!(nullable_new_sizes.dtype().as_ptype(), |S| {
64-
nullable_new_sizes
65-
.mask(validity_array)?
66-
.fill_null(Scalar::primitive(S::zero(), Nullability::NonNullable))?
60+
nullable_new_sizes.fill_null(Scalar::primitive(S::zero(), Nullability::NonNullable))?
6761
});
6862

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

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

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -101,29 +101,6 @@ 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-
127104
#[test]
128105
fn test_take_constant_arrays() {
129106
// ListView-specific: Test with ConstantArray for offsets/sizes.

0 commit comments

Comments
 (0)