Skip to content

Commit 66dfe17

Browse files
authored
fix: Validity take must never return nullable Boolean arrays (#2370)
This is the beginning of a few fixes for nullable indices.
1 parent a4f5ca2 commit 66dfe17

File tree

2 files changed

+33
-2
lines changed

2 files changed

+33
-2
lines changed

vortex-array/src/array/primitive/compute/take.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,37 @@ unsafe fn take_primitive_unchecked<T: NativePType, I: NativePType + AsPrimitive<
102102

103103
#[cfg(test)]
104104
mod test {
105+
use vortex_buffer::buffer;
106+
use vortex_scalar::Scalar;
107+
105108
use crate::array::primitive::compute::take::take_primitive;
109+
use crate::array::{BoolArray, PrimitiveArray};
110+
use crate::compute::{scalar_at, take};
111+
use crate::validity::Validity;
112+
use crate::IntoArray as _;
106113

107114
#[test]
108115
fn test_take() {
109116
let a = vec![1i32, 2, 3, 4, 5];
110117
let result = take_primitive(&a, &[0, 0, 4, 2]);
111118
assert_eq!(result.as_slice(), &[1i32, 1, 5, 3]);
112119
}
120+
121+
#[test]
122+
fn test_take_with_null_indices() {
123+
let values = PrimitiveArray::new(
124+
buffer![1i32, 2, 3, 4, 5],
125+
Validity::Array(BoolArray::from_iter([true, true, false, false, true]).into_array()),
126+
);
127+
let indices = PrimitiveArray::new(
128+
buffer![0, 3, 4],
129+
Validity::Array(BoolArray::from_iter([true, true, false]).into_array()),
130+
);
131+
let actual = take(values, indices).unwrap();
132+
assert_eq!(scalar_at(&actual, 0).unwrap(), Scalar::from(Some(1)));
133+
// position 3 is null
134+
assert_eq!(scalar_at(&actual, 1).unwrap(), Scalar::null_typed::<i32>());
135+
// the third index is null
136+
assert_eq!(scalar_at(&actual, 2).unwrap(), Scalar::null_typed::<i32>());
137+
}
113138
}

vortex-array/src/validity.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ use serde::{Deserialize, Serialize};
88
use vortex_dtype::{DType, Nullability};
99
use vortex_error::{vortex_bail, vortex_err, vortex_panic, VortexExpect as _, VortexResult};
1010
use vortex_mask::{AllOr, Mask, MaskValues};
11+
use vortex_scalar::Scalar;
1112

1213
use crate::array::{BoolArray, ConstantArray};
13-
use crate::compute::{filter, scalar_at, slice, take};
14+
use crate::compute::{fill_null, filter, scalar_at, slice, take};
1415
use crate::patches::Patches;
1516
use crate::{Array, IntoArray, IntoArrayVariant};
1617

@@ -245,7 +246,12 @@ impl Validity {
245246
AllOr::Some(buf) => Ok(Validity::from(buf.clone())),
246247
},
247248
Self::AllInvalid => Ok(Self::AllInvalid),
248-
Self::Array(a) => Ok(Self::Array(take(a, indices)?)),
249+
Self::Array(is_valid) => {
250+
let maybe_is_valid = take(is_valid, indices)?;
251+
// Null indices invalidite that position.
252+
let is_valid = fill_null(maybe_is_valid, Scalar::from(false))?;
253+
Ok(Self::Array(is_valid))
254+
}
249255
}
250256
}
251257

0 commit comments

Comments
 (0)