Skip to content

Commit b4b632e

Browse files
committed
fix: Validity::to_logical asserts mask length equals argument length
I added an assertion to validity to_logical and fixed a bug which it found in VarBinViewArray's `take_into`.
1 parent 960acf6 commit b4b632e

File tree

4 files changed

+22
-5
lines changed

4 files changed

+22
-5
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ impl TakeFn<VarBinViewArray> for VarBinViewEncoding {
7878
// min-max valid range.
7979
// TODO(joe): impl validity_mask take
8080
let validity = array.validity().take(indices)?;
81-
let mask = validity.to_logical(array.len())?;
81+
let mask = validity.to_logical(indices.len())?;
8282
let indices = indices.clone().into_primitive()?;
8383

8484
match_each_integer_ptype!(indices.ptype(), |$I| {

vortex-array/src/compute/take.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,14 @@ pub fn take_into(
136136
#[cfg(debug_assertions)]
137137
{
138138
// If either the indices or the array are nullable, the result should be nullable.
139-
let expected_nullability =
140-
(indices.dtype().is_nullable() || array.dtype().is_nullable()).into();
139+
let expected_nullability = indices.dtype().nullability() | array.dtype().nullability();
141140
assert_eq!(
142141
builder.dtype(),
143142
&array.dtype().with_nullability(expected_nullability),
144-
"Take_into result should be nullable if either the indices or the array are nullable"
143+
"Take_into result ({}) should be nullable if, and only if, either the indices ({}) or the array are nullable ({})",
144+
builder.dtype(),
145+
indices.dtype().nullability().verbose_display(),
146+
array.dtype().nullability().verbose_display(),
145147
);
146148
}
147149

vortex-array/src/validity.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,10 @@ impl Validity {
300300
Ok(match self {
301301
Self::NonNullable | Self::AllValid => Mask::AllTrue(length),
302302
Self::AllInvalid => Mask::AllFalse(length),
303-
Self::Array(a) => Mask::try_from(a.clone())?,
303+
Self::Array(a) => {
304+
assert_eq!(a.len(), length);
305+
Mask::try_from(a.clone())?
306+
}
304307
})
305308
}
306309

vortex-dtype/src/nullability.rs

+12
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,18 @@ pub enum Nullability {
1111
Nullable,
1212
}
1313

14+
impl Nullability {
15+
/// A self-describing displayed form.
16+
///
17+
/// The usual Display renders [Nullability::NonNullable] as the empty string.
18+
pub fn verbose_display(&self) -> impl Display {
19+
match self {
20+
Nullability::NonNullable => "NonNullable",
21+
Nullability::Nullable => "Nullable",
22+
}
23+
}
24+
}
25+
1426
impl BitOr for Nullability {
1527
type Output = Nullability;
1628

0 commit comments

Comments
 (0)