Skip to content

Standardize DType match arms to follow variant declaration order #7897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
connortsui20 wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Standardize DType match arms to follow variant declaration order #7897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions encodings/sparse/src/canonical.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,6 @@ pub(super) fn execute_sparse(parts: SparseParts, ctx: &mut ExecutionCtx) -> Vort
execute_sparse_primitives::<P>(&patches, &fill_value, ctx)?
})
}
DType::Struct(struct_fields, ..) => execute_sparse_struct(
struct_fields,
fill_value.as_struct(),
dtype.nullability(),
&patches,
len,
ctx,
)?,
DType::Decimal(decimal_dtype, nullability) => {
let canonical_decimal_value_type =
DecimalType::smallest_decimal_value_type(decimal_dtype);
Expand Down Expand Up @@ -157,8 +149,16 @@ pub(super) fn execute_sparse(parts: SparseParts, ctx: &mut ExecutionCtx) -> Vort
DType::FixedSizeList(.., nullability) => {
execute_sparse_fixed_size_list(&patches, &fill_value, len, *nullability, ctx)?
}
DType::Extension(_ext_dtype) => todo!(),
DType::Struct(struct_fields, ..) => execute_sparse_struct(
struct_fields,
fill_value.as_struct(),
dtype.nullability(),
&patches,
len,
ctx,
)?,
DType::Variant(_) => vortex_bail!("Sparse canonicalization does not support Variant"),
DType::Extension(_ext_dtype) => todo!(),
})
}

Expand Down
4 changes: 2 additions & 2 deletions fuzz/src/array/compare.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub fn compare_canonical_array(
)
})
}
DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) => {
DType::List(..) | DType::FixedSizeList(..) | DType::Struct(..) => {
let scalar_vals: Vec<Scalar> = (0..array.len())
.map(|i| array.execute_scalar(i, ctx).vortex_expect("scalar_at"))
.collect();
Expand All @@ -173,7 +173,7 @@ pub fn compare_canonical_array(
}))
.into_array()
}
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
unreachable!("DType {d} not supported for fuzzing")
}
}
Expand Down
20 changes: 10 additions & 10 deletions fuzz/src/array/filter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ pub fn filter_canonical_array(
});
Ok(VarBinViewArray::from_iter(values, array.dtype().clone()).into_array())
}
DType::List(..) | DType::FixedSizeList(..) => {
let mut indices = Vec::new();
for (idx, bool) in filter.iter().enumerate() {
if *bool {
indices.push(idx);
}
}
take_canonical_array_non_nullable_indices(array, indices.as_slice(), ctx)
}
DType::Struct(..) => {
let struct_array = array.clone().execute::<StructArray>(ctx)?;
let filtered_children = struct_array
Expand All @@ -113,16 +122,7 @@ pub fn filter_canonical_array(
)
.map(|a| a.into_array())
}
DType::List(..) | DType::FixedSizeList(..) => {
let mut indices = Vec::new();
for (idx, bool) in filter.iter().enumerate() {
if *bool {
indices.push(idx);
}
}
take_canonical_array_non_nullable_indices(array, indices.as_slice(), ctx)
}
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
unreachable!("DType {d} not supported for fuzzing")
}
}
Expand Down
48 changes: 24 additions & 24 deletions fuzz/src/array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,24 +464,8 @@ fn actions_for_dtype(dtype: &DType) -> HashSet<ActionType> {
use ActionType::*;

match dtype {
DType::Struct(sdt, _) => {
// Struct supports: Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt
// Does NOT support: SearchSorted (requires scalar comparison), Compare, Cast, Sum, FillNull
let struct_actions = [Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt];
sdt.fields()
.map(|child| actions_for_dtype(&child))
.fold(struct_actions.into(), |acc, actions| {
acc.intersection(&actions).copied().collect()
})
}
DType::List(..) | DType::FixedSizeList(..) => {
// List supports: Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt
// Does NOT support: SearchSorted, Compare, Cast, Sum, FillNull
[Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt].into()
}
DType::Utf8(_) | DType::Binary(_) => {
// Utf8/Binary supports everything except Sum and FillNull
// Actions: Compress, Slice, Take, SearchSorted, Filter, Compare, Cast, MinMax, Mask, ScalarAt
DType::Null => {
// Null arrays support most operations but not Sum or MinMax (return None for dtype)
[
Compress,
Slice,
Expand All @@ -490,7 +474,7 @@ fn actions_for_dtype(dtype: &DType) -> HashSet<ActionType> {
Filter,
Compare,
Cast,
MinMax,
FillNull,
Mask,
ScalarAt,
]
Expand All @@ -500,8 +484,9 @@ fn actions_for_dtype(dtype: &DType) -> HashSet<ActionType> {
// These support all actions
ActionType::iter().collect()
}
DType::Null => {
// Null arrays support most operations but not Sum or MinMax (return None for dtype)
DType::Utf8(_) | DType::Binary(_) => {
// Utf8/Binary supports everything except Sum and FillNull
// Actions: Compress, Slice, Take, SearchSorted, Filter, Compare, Cast, MinMax, Mask, ScalarAt
[
Compress,
Slice,
Expand All @@ -510,18 +495,33 @@ fn actions_for_dtype(dtype: &DType) -> HashSet<ActionType> {
Filter,
Compare,
Cast,
FillNull,
MinMax,
Mask,
ScalarAt,
]
.into()
}
DType::List(..) | DType::FixedSizeList(..) => {
// List supports: Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt
// Does NOT support: SearchSorted, Compare, Cast, Sum, FillNull
[Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt].into()
}
DType::Struct(sdt, _) => {
// Struct supports: Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt
// Does NOT support: SearchSorted (requires scalar comparison), Compare, Cast, Sum, FillNull
let struct_actions = [Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt];
sdt.fields()
.map(|child| actions_for_dtype(&child))
.fold(struct_actions.into(), |acc, actions| {
acc.intersection(&actions).copied().collect()
})
}
// Currently, no support at all
DType::Variant(_) => unreachable!("Variant dtype shouldn't be fuzzed"),
DType::Extension(_) => {
// Extension types delegate to storage dtype, support most operations
ActionType::iter().collect()
}
// Currently, no support at all
DType::Variant(_) => unreachable!("Variant dtype shouldn't be fuzzed"),
}
}

Expand Down
4 changes: 2 additions & 2 deletions fuzz/src/array/search_sorted.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,13 @@ pub fn search_sorted_canonical_array(
};
SearchNullableSlice(opt_values).search_sorted(&Some(to_find), side)
}
DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) => {
DType::List(..) | DType::FixedSizeList(..) | DType::Struct(..) => {
let scalar_vals = (0..array.len())
.map(|i| array.execute_scalar(i, ctx))
.collect::<VortexResult<Vec<_>>>()?;
scalar_vals.search_sorted(&scalar.cast(array.dtype())?, side)
}
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
unreachable!("DType {d} not supported for fuzzing")
}
}
Expand Down
52 changes: 26 additions & 26 deletions fuzz/src/array/slice.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,19 @@ pub fn slice_canonical_array(
.into_array())
})
}
DType::Decimal(decimal_dtype, _) => {
let decimal_array = array.clone().execute::<DecimalArray>(ctx)?;
Ok(
match_each_decimal_value_type!(decimal_array.values_type(), |D| {
DecimalArray::new(
decimal_array.buffer::<D>().slice(start..stop),
*decimal_dtype,
validity,
)
})
.into_array(),
)
}
DType::Utf8(_) | DType::Binary(_) => {
let utf8 = array.clone().execute::<VarBinViewArray>(ctx)?;
let values =
Expand All @@ -64,20 +77,6 @@ pub fn slice_canonical_array(
)
.into_array())
}
DType::Struct(..) => {
let struct_array = array.clone().execute::<StructArray>(ctx)?;
let sliced_children = struct_array
.iter_unmasked_fields()
.map(|c| slice_canonical_array(c, start, stop, ctx))
.collect::<VortexResult<Vec<_>>>()?;
StructArray::try_new_with_dtype(
sliced_children,
struct_array.struct_fields().clone(),
stop - start,
validity,
)
.map(|a| a.into_array())
}
DType::List(..) => {
let list_array = array.clone().execute::<ListViewArray>(ctx)?;

Expand Down Expand Up @@ -110,20 +109,21 @@ pub fn slice_canonical_array(
FixedSizeListArray::try_new(elements, fsl_array.list_size(), validity, new_len)
.map(|a| a.into_array())
}
DType::Decimal(decimal_dtype, _) => {
let decimal_array = array.clone().execute::<DecimalArray>(ctx)?;
Ok(
match_each_decimal_value_type!(decimal_array.values_type(), |D| {
DecimalArray::new(
decimal_array.buffer::<D>().slice(start..stop),
*decimal_dtype,
validity,
)
})
.into_array(),
DType::Struct(..) => {
let struct_array = array.clone().execute::<StructArray>(ctx)?;
let sliced_children = struct_array
.iter_unmasked_fields()
.map(|c| slice_canonical_array(c, start, stop, ctx))
.collect::<VortexResult<Vec<_>>>()?;
StructArray::try_new_with_dtype(
sliced_children,
struct_array.struct_fields().clone(),
stop - start,
validity,
)
.map(|a| a.into_array())
}
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
unreachable!("DType {d} not supported for fuzzing")
}
}
Expand Down
4 changes: 2 additions & 2 deletions fuzz/src/array/sort.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub fn sort_canonical_array(array: &ArrayRef, ctx: &mut ExecutionCtx) -> VortexR
opt_values.sort();
Ok(VarBinViewArray::from_iter(opt_values, array.dtype().clone()).into_array())
}
DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) => {
DType::List(..) | DType::FixedSizeList(..) | DType::Struct(..) => {
let mut sort_indices = (0..array.len()).collect::<Vec<_>>();
sort_indices.sort_by(|a, b| {
let lhs = array.execute_scalar(*a, ctx).vortex_expect("scalar_at");
Expand All @@ -101,7 +101,7 @@ pub fn sort_canonical_array(array: &ArrayRef, ctx: &mut ExecutionCtx) -> VortexR
});
take_canonical_array_non_nullable_indices(array, &sort_indices, ctx)
}
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
unreachable!("DType {d} not supported for fuzzing")
}
}
Expand Down
32 changes: 16 additions & 16 deletions fuzz/src/array/take.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,21 +113,6 @@ pub fn take_canonical_array(
)
.into_array())
}
DType::Struct(..) => {
let struct_array = array.clone().execute::<StructArray>(ctx)?;
let taken_children = struct_array
.iter_unmasked_fields()
.map(|c| take_canonical_array_non_nullable_indices(c, indices_slice_non_opt, ctx))
.collect::<VortexResult<Vec<_>>>()?;

StructArray::try_new(
struct_array.names().clone(),
taken_children,
indices_slice_non_opt.len(),
validity,
)
.map(|a| a.into_array())
}
DType::List(..) | DType::FixedSizeList(..) => {
let mut builder = builder_with_capacity(
&array.dtype().union_nullability(nullable),
Expand All @@ -147,7 +132,22 @@ pub fn take_canonical_array(
}
Ok(builder.finish())
}
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
DType::Struct(..) => {
let struct_array = array.clone().execute::<StructArray>(ctx)?;
let taken_children = struct_array
.iter_unmasked_fields()
.map(|c| take_canonical_array_non_nullable_indices(c, indices_slice_non_opt, ctx))
.collect::<VortexResult<Vec<_>>>()?;

StructArray::try_new(
struct_array.names().clone(),
taken_children,
indices_slice_non_opt.len(),
validity,
)
.map(|a| a.into_array())
}
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
unreachable!("DType {d} not supported for fuzzing")
}
}
Expand Down
4 changes: 2 additions & 2 deletions vortex-array/src/aggregate_fn/fns/is_sorted/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,9 @@ impl AggregateFnVTable for IsSorted {
fn return_dtype(&self, _options: &Self::Options, input_dtype: &DType) -> Option<DType> {
match input_dtype {
DType::Null
| DType::Struct(..)
| DType::List(..)
| DType::FixedSizeList(..)
| DType::Struct(..)
| DType::Variant(..) => None,
_ => Some(DType::Bool(Nullability::NonNullable)),
}
Expand All @@ -253,9 +253,9 @@ impl AggregateFnVTable for IsSorted {
fn partial_dtype(&self, _options: &Self::Options, input_dtype: &DType) -> Option<DType> {
match input_dtype {
DType::Null
| DType::Struct(..)
| DType::List(..)
| DType::FixedSizeList(..)
| DType::Struct(..)
| DType::Variant(..) => None,
_ => Some(make_is_sorted_partial_dtype(input_dtype)),
}
Expand Down
10 changes: 5 additions & 5 deletions vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,13 @@ pub(crate) fn constant_uncompressed_size_in_bytes(
array.len(),
array.scalar().as_binary().value().map(|value| value.len()),
)?,
DType::Variant(_) => {
vortex_bail!("UncompressedSizeInBytes is not supported for Variant arrays")
}
DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) | DType::Extension(_) => {
DType::List(..) | DType::FixedSizeList(..) | DType::Struct(..) | DType::Extension(_) => {
let canonical = array.array().clone().execute::<Canonical>(ctx)?;
return canonical_uncompressed_size_in_bytes(&canonical, ctx);
}
DType::Variant(_) => {
vortex_bail!("UncompressedSizeInBytes is not supported for Variant arrays")
}
};

value_size
Expand Down Expand Up @@ -293,10 +293,10 @@ fn supports_uncompressed_size_in_bytes(dtype: &DType) -> bool {
DType::Struct(fields, _) => fields
.fields()
.all(|field| supports_uncompressed_size_in_bytes(&field)),
DType::Variant(_) => false,
DType::Extension(ext_dtype) => {
supports_uncompressed_size_in_bytes(ext_dtype.storage_dtype())
}
DType::Variant(_) => false,
DType::Null
| DType::Bool(_)
| DType::Primitive(..)
Expand Down
Loading
Loading