Skip to content

Commit 5a25819

Browse files
committed
Part 2 of making Variant a canonical array
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
1 parent 04776cc commit 5a25819

15 files changed

Lines changed: 85 additions & 25 deletions

File tree

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: 4 additions & 0 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;
@@ -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/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

vortex-array/src/arrays/variant/mod.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,26 +5,24 @@ mod vtable;
55

66
pub use self::vtable::Variant;
77
use crate::ArrayRef;
8-
use crate::dtype::DType;
9-
use crate::dtype::Nullability;
108

119
/// The canonical in-memory representation of variant (semi-structured) data.
1210
///
1311
/// Wraps a single child array that contains the actual variant-encoded data
1412
/// (e.g. a `ParquetVariantArray` or any other variant encoding).
13+
///
14+
/// Nullability is delegated to the child array: `VariantArray`'s dtype is
15+
/// always the child's dtype. The child's validity determines which rows are
16+
/// null.
1517
#[derive(Clone, Debug)]
1618
pub struct VariantArray {
17-
dtype: DType,
1819
child: ArrayRef,
1920
}
2021

2122
impl VariantArray {
22-
/// Creates a new VariantArray with the given nullability.
23-
pub fn new(child: ArrayRef, nullability: Nullability) -> Self {
24-
Self {
25-
dtype: DType::Variant(nullability),
26-
child,
27-
}
23+
/// Creates a new VariantArray. Nullability comes from the child's dtype.
24+
pub fn new(child: ArrayRef) -> Self {
25+
Self { child }
2826
}
2927

3028
/// Returns a reference to the underlying child array.

0 commit comments

Comments
 (0)