Skip to content

Commit 0ea8973

Browse files
authored
Part 2 of making Variant a canonical array (#7143)
## Summary Filling up missing parts of making Variant a canonical array. It still not fully supported but this is a step towards it. These changes started as part of #7130, but I figured they are just noise in that already too big of a PR. ## API Changes Makes variant officially a canonical array and type. --------- Signed-off-by: Adam Gutglick <adam@spiraldb.com>
1 parent cda251b commit 0ea8973

File tree

16 files changed

+97
-29
lines changed

16 files changed

+97
-29
lines changed

fuzz/src/array/fill_null.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ pub fn fill_null_canonical_array(
4444
| Canonical::List(_)
4545
| Canonical::FixedSizeList(_)
4646
| Canonical::Extension(_) => canonical.into_array().fill_null(fill_value.clone())?,
47+
Canonical::Variant(_) => unreachable!("Variant arrays are not fuzzed"),
4748
})
4849
}
4950

fuzz/src/array/mask.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ pub fn mask_canonical_array(canonical: Canonical, mask: &Mask) -> VortexResult<A
136136
.with_nullability(masked_storage.dtype().nullability());
137137
ExtensionArray::new(ext_dtype, masked_storage).into_array()
138138
}
139+
Canonical::Variant(_) => unreachable!("Variant arrays are not fuzzed"),
139140
})
140141
}
141142

fuzz/src/array/scalar_at.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,5 +95,6 @@ pub fn scalar_at_canonical_array(canonical: Canonical, index: usize) -> VortexRe
9595
scalar_at_canonical_array(array.storage_array().to_canonical()?, index)?;
9696
Scalar::extension_ref(array.ext_dtype().clone(), storage_scalar)
9797
}
98+
Canonical::Variant(_) => unreachable!("Variant arrays are not fuzzed"),
9899
})
99100
}

vortex-array/public-api.lock

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5020,7 +5020,7 @@ impl vortex_array::arrays::variant::VariantArray
50205020

50215021
pub fn vortex_array::arrays::variant::VariantArray::child(&self) -> &vortex_array::ArrayRef
50225022

5023-
pub fn vortex_array::arrays::variant::VariantArray::new(child: vortex_array::ArrayRef, nullability: vortex_array::dtype::Nullability) -> Self
5023+
pub fn vortex_array::arrays::variant::VariantArray::new(child: vortex_array::ArrayRef) -> Self
50245024

50255025
impl vortex_array::arrays::variant::VariantArray
50265026

@@ -8272,7 +8272,7 @@ impl vortex_array::arrays::variant::VariantArray
82728272

82738273
pub fn vortex_array::arrays::variant::VariantArray::child(&self) -> &vortex_array::ArrayRef
82748274

8275-
pub fn vortex_array::arrays::variant::VariantArray::new(child: vortex_array::ArrayRef, nullability: vortex_array::dtype::Nullability) -> Self
8275+
pub fn vortex_array::arrays::variant::VariantArray::new(child: vortex_array::ArrayRef) -> Self
82768276

82778277
impl vortex_array::arrays::variant::VariantArray
82788278

@@ -22096,6 +22096,8 @@ pub vortex_array::Canonical::Struct(vortex_array::arrays::StructArray)
2209622096

2209722097
pub vortex_array::Canonical::VarBinView(vortex_array::arrays::VarBinViewArray)
2209822098

22099+
pub vortex_array::Canonical::Variant(vortex_array::arrays::variant::VariantArray)
22100+
2209922101
impl vortex_array::Canonical
2210022102

2210122103
pub fn vortex_array::Canonical::as_bool(&self) -> &vortex_array::arrays::BoolArray
@@ -22196,6 +22198,8 @@ pub vortex_array::CanonicalView::Struct(&'a vortex_array::arrays::StructArray)
2219622198

2219722199
pub vortex_array::CanonicalView::VarBinView(&'a vortex_array::arrays::VarBinViewArray)
2219822200

22201+
pub vortex_array::CanonicalView::Variant(&'a vortex_array::arrays::variant::VariantArray)
22202+
2219922203
impl core::convert::AsRef<dyn vortex_array::DynArray> for vortex_array::CanonicalView<'_>
2220022204

2220122205
pub fn vortex_array::CanonicalView<'_>::as_ref(&self) -> &dyn vortex_array::DynArray

vortex-array/src/aggregate_fn/fns/is_constant/mod.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ mod varbin;
1212

1313
use vortex_error::VortexExpect;
1414
use vortex_error::VortexResult;
15+
use vortex_error::vortex_bail;
1516
use vortex_mask::Mask;
1617

1718
use self::bool::check_bool_constant;
@@ -273,14 +274,14 @@ impl AggregateFnVTable for IsConstant {
273274

274275
fn return_dtype(&self, _options: &Self::Options, input_dtype: &DType) -> Option<DType> {
275276
match input_dtype {
276-
DType::Null => None,
277+
DType::Null | DType::Variant(..) => None,
277278
_ => Some(DType::Bool(Nullability::NonNullable)),
278279
}
279280
}
280281

281282
fn partial_dtype(&self, _options: &Self::Options, input_dtype: &DType) -> Option<DType> {
282283
match input_dtype {
283-
DType::Null => None,
284+
DType::Null | DType::Variant(..) => None,
284285
_ => Some(make_is_constant_partial_dtype(input_dtype)),
285286
}
286287
}
@@ -408,6 +409,9 @@ impl AggregateFnVTable for IsConstant {
408409
Canonical::List(l) => check_listview_constant(l, ctx)?,
409410
Canonical::FixedSizeList(f) => check_fixed_size_list_constant(f, ctx)?,
410411
Canonical::Null(_) => true,
412+
Canonical::Variant(_) => {
413+
vortex_bail!("Variant arrays don't support IsConstant")
414+
}
411415
};
412416

413417
if !batch_is_constant {

vortex-array/src/aggregate_fn/fns/is_sorted/mod.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,14 +257,22 @@ impl AggregateFnVTable for IsSorted {
257257

258258
fn return_dtype(&self, _options: &Self::Options, input_dtype: &DType) -> Option<DType> {
259259
match input_dtype {
260-
DType::Null | DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) => None,
260+
DType::Null
261+
| DType::Struct(..)
262+
| DType::List(..)
263+
| DType::FixedSizeList(..)
264+
| DType::Variant(..) => None,
261265
_ => Some(DType::Bool(Nullability::NonNullable)),
262266
}
263267
}
264268

265269
fn partial_dtype(&self, _options: &Self::Options, input_dtype: &DType) -> Option<DType> {
266270
match input_dtype {
267-
DType::Null | DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) => None,
271+
DType::Null
272+
| DType::Struct(..)
273+
| DType::List(..)
274+
| DType::FixedSizeList(..)
275+
| DType::Variant(..) => None,
268276
_ => Some(make_is_sorted_partial_dtype(input_dtype)),
269277
}
270278
}

vortex-array/src/aggregate_fn/fns/min_max/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,10 @@ impl AggregateFnVTable for MinMax {
271271
Canonical::Decimal(d) => accumulate_decimal(partial, d),
272272
Canonical::Extension(e) => accumulate_extension(partial, e, ctx),
273273
Canonical::Null(_) => Ok(()),
274-
Canonical::Struct(_) | Canonical::List(_) | Canonical::FixedSizeList(_) => {
274+
Canonical::Struct(_)
275+
| Canonical::List(_)
276+
| Canonical::FixedSizeList(_)
277+
| Canonical::Variant(_) => {
275278
vortex_bail!("Unsupported canonical type for min_max: {}", batch.dtype())
276279
}
277280
},

vortex-array/src/arrays/dict/execute.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
66
use vortex_error::VortexExpect;
77
use vortex_error::VortexResult;
8+
use vortex_error::vortex_bail;
89

910
use crate::Canonical;
1011
use crate::ExecutionCtx;
@@ -50,6 +51,9 @@ pub fn take_canonical(
5051
}
5152
Canonical::Struct(a) => Canonical::Struct(take_struct(&a, codes)),
5253
Canonical::Extension(a) => Canonical::Extension(take_extension(&a, codes, ctx)),
54+
Canonical::Variant(_) => {
55+
vortex_bail!("Variant arrays don't support Take")
56+
}
5357
})
5458
}
5559

vortex-array/src/arrays/filter/execute/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::sync::Arc;
99

1010
use vortex_error::VortexExpect;
1111
use vortex_error::VortexResult;
12+
use vortex_error::vortex_panic;
1213
use vortex_mask::Mask;
1314
use vortex_mask::MaskValues;
1415

@@ -94,5 +95,8 @@ pub(super) fn execute_filter(canonical: Canonical, mask: &Arc<MaskValues>) -> Ca
9495
.vortex_expect("ExtensionArray storage type somehow could not be filtered");
9596
Canonical::Extension(ExtensionArray::new(a.ext_dtype().clone(), filtered_storage))
9697
}
98+
Canonical::Variant(_) => {
99+
vortex_panic!("Variant arrays don't support filtering")
100+
}
97101
}
98102
}

vortex-array/src/arrays/masked/execute.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use std::ops::BitAnd;
77

88
use vortex_error::VortexResult;
9+
use vortex_error::vortex_bail;
910
use vortex_mask::Mask;
1011

1112
use crate::Canonical;
@@ -53,6 +54,9 @@ pub fn mask_validity_canonical(
5354
Canonical::Extension(a) => {
5455
Canonical::Extension(mask_validity_extension(a, validity_mask, ctx)?)
5556
}
57+
Canonical::Variant(_) => {
58+
vortex_bail!("Variant arrays don't masking validity")
59+
}
5660
})
5761
}
5862

0 commit comments

Comments
 (0)