Skip to content

Commit 5770528

Browse files
committed
Standardize DType match arms to follow variant declaration order
Reorder `DType` match arms across the codebase to match the order of the `DType` enum declaration, and move `Extension` after `Variant` so that `Extension` is the last variant in the enum (and in all match statements). This is a purely cosmetic refactor with no functional changes: - The protobuf, flatbuffer, and serde wire formats are preserved (only the source order of match arms is changed, not the discriminant/tag values). - The public API surface is unchanged; `public-api.lock` files are alphabetical so they require no update. Signed-off-by: Claude <noreply@anthropic.com>
1 parent 2ee2033 commit 5770528

36 files changed

Lines changed: 308 additions & 308 deletions

File tree

encodings/sparse/src/canonical.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,6 @@ pub(super) fn execute_sparse(parts: SparseParts, ctx: &mut ExecutionCtx) -> Vort
114114
execute_sparse_primitives::<P>(&patches, &fill_value, ctx)?
115115
})
116116
}
117-
DType::Struct(struct_fields, ..) => execute_sparse_struct(
118-
struct_fields,
119-
fill_value.as_struct(),
120-
dtype.nullability(),
121-
&patches,
122-
len,
123-
ctx,
124-
)?,
125117
DType::Decimal(decimal_dtype, nullability) => {
126118
let canonical_decimal_value_type =
127119
DecimalType::smallest_decimal_value_type(decimal_dtype);
@@ -157,8 +149,16 @@ pub(super) fn execute_sparse(parts: SparseParts, ctx: &mut ExecutionCtx) -> Vort
157149
DType::FixedSizeList(.., nullability) => {
158150
execute_sparse_fixed_size_list(&patches, &fill_value, len, *nullability, ctx)?
159151
}
160-
DType::Extension(_ext_dtype) => todo!(),
152+
DType::Struct(struct_fields, ..) => execute_sparse_struct(
153+
struct_fields,
154+
fill_value.as_struct(),
155+
dtype.nullability(),
156+
&patches,
157+
len,
158+
ctx,
159+
)?,
161160
DType::Variant(_) => vortex_bail!("Sparse canonicalization does not support Variant"),
161+
DType::Extension(_ext_dtype) => todo!(),
162162
})
163163
}
164164

fuzz/src/array/compare.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ pub fn compare_canonical_array(
161161
)
162162
})
163163
}
164-
DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) => {
164+
DType::List(..) | DType::FixedSizeList(..) | DType::Struct(..) => {
165165
let scalar_vals: Vec<Scalar> = (0..array.len())
166166
.map(|i| array.execute_scalar(i, ctx).vortex_expect("scalar_at"))
167167
.collect();
@@ -173,7 +173,7 @@ pub fn compare_canonical_array(
173173
}))
174174
.into_array()
175175
}
176-
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
176+
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
177177
unreachable!("DType {d} not supported for fuzzing")
178178
}
179179
}

fuzz/src/array/filter.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,15 @@ pub fn filter_canonical_array(
9898
});
9999
Ok(VarBinViewArray::from_iter(values, array.dtype().clone()).into_array())
100100
}
101+
DType::List(..) | DType::FixedSizeList(..) => {
102+
let mut indices = Vec::new();
103+
for (idx, bool) in filter.iter().enumerate() {
104+
if *bool {
105+
indices.push(idx);
106+
}
107+
}
108+
take_canonical_array_non_nullable_indices(array, indices.as_slice(), ctx)
109+
}
101110
DType::Struct(..) => {
102111
let struct_array = array.clone().execute::<StructArray>(ctx)?;
103112
let filtered_children = struct_array
@@ -113,16 +122,7 @@ pub fn filter_canonical_array(
113122
)
114123
.map(|a| a.into_array())
115124
}
116-
DType::List(..) | DType::FixedSizeList(..) => {
117-
let mut indices = Vec::new();
118-
for (idx, bool) in filter.iter().enumerate() {
119-
if *bool {
120-
indices.push(idx);
121-
}
122-
}
123-
take_canonical_array_non_nullable_indices(array, indices.as_slice(), ctx)
124-
}
125-
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
125+
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
126126
unreachable!("DType {d} not supported for fuzzing")
127127
}
128128
}

fuzz/src/array/mod.rs

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -464,24 +464,8 @@ fn actions_for_dtype(dtype: &DType) -> HashSet<ActionType> {
464464
use ActionType::*;
465465

466466
match dtype {
467-
DType::Struct(sdt, _) => {
468-
// Struct supports: Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt
469-
// Does NOT support: SearchSorted (requires scalar comparison), Compare, Cast, Sum, FillNull
470-
let struct_actions = [Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt];
471-
sdt.fields()
472-
.map(|child| actions_for_dtype(&child))
473-
.fold(struct_actions.into(), |acc, actions| {
474-
acc.intersection(&actions).copied().collect()
475-
})
476-
}
477-
DType::List(..) | DType::FixedSizeList(..) => {
478-
// List supports: Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt
479-
// Does NOT support: SearchSorted, Compare, Cast, Sum, FillNull
480-
[Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt].into()
481-
}
482-
DType::Utf8(_) | DType::Binary(_) => {
483-
// Utf8/Binary supports everything except Sum and FillNull
484-
// Actions: Compress, Slice, Take, SearchSorted, Filter, Compare, Cast, MinMax, Mask, ScalarAt
467+
DType::Null => {
468+
// Null arrays support most operations but not Sum or MinMax (return None for dtype)
485469
[
486470
Compress,
487471
Slice,
@@ -490,7 +474,7 @@ fn actions_for_dtype(dtype: &DType) -> HashSet<ActionType> {
490474
Filter,
491475
Compare,
492476
Cast,
493-
MinMax,
477+
FillNull,
494478
Mask,
495479
ScalarAt,
496480
]
@@ -500,8 +484,9 @@ fn actions_for_dtype(dtype: &DType) -> HashSet<ActionType> {
500484
// These support all actions
501485
ActionType::iter().collect()
502486
}
503-
DType::Null => {
504-
// Null arrays support most operations but not Sum or MinMax (return None for dtype)
487+
DType::Utf8(_) | DType::Binary(_) => {
488+
// Utf8/Binary supports everything except Sum and FillNull
489+
// Actions: Compress, Slice, Take, SearchSorted, Filter, Compare, Cast, MinMax, Mask, ScalarAt
505490
[
506491
Compress,
507492
Slice,
@@ -510,18 +495,33 @@ fn actions_for_dtype(dtype: &DType) -> HashSet<ActionType> {
510495
Filter,
511496
Compare,
512497
Cast,
513-
FillNull,
498+
MinMax,
514499
Mask,
515500
ScalarAt,
516501
]
517502
.into()
518503
}
504+
DType::List(..) | DType::FixedSizeList(..) => {
505+
// List supports: Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt
506+
// Does NOT support: SearchSorted, Compare, Cast, Sum, FillNull
507+
[Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt].into()
508+
}
509+
DType::Struct(sdt, _) => {
510+
// Struct supports: Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt
511+
// Does NOT support: SearchSorted (requires scalar comparison), Compare, Cast, Sum, FillNull
512+
let struct_actions = [Compress, Slice, Take, Filter, MinMax, Mask, ScalarAt];
513+
sdt.fields()
514+
.map(|child| actions_for_dtype(&child))
515+
.fold(struct_actions.into(), |acc, actions| {
516+
acc.intersection(&actions).copied().collect()
517+
})
518+
}
519+
// Currently, no support at all
520+
DType::Variant(_) => unreachable!("Variant dtype shouldn't be fuzzed"),
519521
DType::Extension(_) => {
520522
// Extension types delegate to storage dtype, support most operations
521523
ActionType::iter().collect()
522524
}
523-
// Currently, no support at all
524-
DType::Variant(_) => unreachable!("Variant dtype shouldn't be fuzzed"),
525525
}
526526
}
527527

fuzz/src/array/search_sorted.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,13 @@ pub fn search_sorted_canonical_array(
142142
};
143143
SearchNullableSlice(opt_values).search_sorted(&Some(to_find), side)
144144
}
145-
DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) => {
145+
DType::List(..) | DType::FixedSizeList(..) | DType::Struct(..) => {
146146
let scalar_vals = (0..array.len())
147147
.map(|i| array.execute_scalar(i, ctx))
148148
.collect::<VortexResult<Vec<_>>>()?;
149149
scalar_vals.search_sorted(&scalar.cast(array.dtype())?, side)
150150
}
151-
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
151+
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
152152
unreachable!("DType {d} not supported for fuzzing")
153153
}
154154
}

fuzz/src/array/slice.rs

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,19 @@ pub fn slice_canonical_array(
5454
.into_array())
5555
})
5656
}
57+
DType::Decimal(decimal_dtype, _) => {
58+
let decimal_array = array.clone().execute::<DecimalArray>(ctx)?;
59+
Ok(
60+
match_each_decimal_value_type!(decimal_array.values_type(), |D| {
61+
DecimalArray::new(
62+
decimal_array.buffer::<D>().slice(start..stop),
63+
*decimal_dtype,
64+
validity,
65+
)
66+
})
67+
.into_array(),
68+
)
69+
}
5770
DType::Utf8(_) | DType::Binary(_) => {
5871
let utf8 = array.clone().execute::<VarBinViewArray>(ctx)?;
5972
let values =
@@ -64,20 +77,6 @@ pub fn slice_canonical_array(
6477
)
6578
.into_array())
6679
}
67-
DType::Struct(..) => {
68-
let struct_array = array.clone().execute::<StructArray>(ctx)?;
69-
let sliced_children = struct_array
70-
.iter_unmasked_fields()
71-
.map(|c| slice_canonical_array(c, start, stop, ctx))
72-
.collect::<VortexResult<Vec<_>>>()?;
73-
StructArray::try_new_with_dtype(
74-
sliced_children,
75-
struct_array.struct_fields().clone(),
76-
stop - start,
77-
validity,
78-
)
79-
.map(|a| a.into_array())
80-
}
8180
DType::List(..) => {
8281
let list_array = array.clone().execute::<ListViewArray>(ctx)?;
8382

@@ -110,20 +109,21 @@ pub fn slice_canonical_array(
110109
FixedSizeListArray::try_new(elements, fsl_array.list_size(), validity, new_len)
111110
.map(|a| a.into_array())
112111
}
113-
DType::Decimal(decimal_dtype, _) => {
114-
let decimal_array = array.clone().execute::<DecimalArray>(ctx)?;
115-
Ok(
116-
match_each_decimal_value_type!(decimal_array.values_type(), |D| {
117-
DecimalArray::new(
118-
decimal_array.buffer::<D>().slice(start..stop),
119-
*decimal_dtype,
120-
validity,
121-
)
122-
})
123-
.into_array(),
112+
DType::Struct(..) => {
113+
let struct_array = array.clone().execute::<StructArray>(ctx)?;
114+
let sliced_children = struct_array
115+
.iter_unmasked_fields()
116+
.map(|c| slice_canonical_array(c, start, stop, ctx))
117+
.collect::<VortexResult<Vec<_>>>()?;
118+
StructArray::try_new_with_dtype(
119+
sliced_children,
120+
struct_array.struct_fields().clone(),
121+
stop - start,
122+
validity,
124123
)
124+
.map(|a| a.into_array())
125125
}
126-
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
126+
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
127127
unreachable!("DType {d} not supported for fuzzing")
128128
}
129129
}

fuzz/src/array/sort.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub fn sort_canonical_array(array: &ArrayRef, ctx: &mut ExecutionCtx) -> VortexR
9191
opt_values.sort();
9292
Ok(VarBinViewArray::from_iter(opt_values, array.dtype().clone()).into_array())
9393
}
94-
DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) => {
94+
DType::List(..) | DType::FixedSizeList(..) | DType::Struct(..) => {
9595
let mut sort_indices = (0..array.len()).collect::<Vec<_>>();
9696
sort_indices.sort_by(|a, b| {
9797
let lhs = array.execute_scalar(*a, ctx).vortex_expect("scalar_at");
@@ -101,7 +101,7 @@ pub fn sort_canonical_array(array: &ArrayRef, ctx: &mut ExecutionCtx) -> VortexR
101101
});
102102
take_canonical_array_non_nullable_indices(array, &sort_indices, ctx)
103103
}
104-
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
104+
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
105105
unreachable!("DType {d} not supported for fuzzing")
106106
}
107107
}

fuzz/src/array/take.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -113,21 +113,6 @@ pub fn take_canonical_array(
113113
)
114114
.into_array())
115115
}
116-
DType::Struct(..) => {
117-
let struct_array = array.clone().execute::<StructArray>(ctx)?;
118-
let taken_children = struct_array
119-
.iter_unmasked_fields()
120-
.map(|c| take_canonical_array_non_nullable_indices(c, indices_slice_non_opt, ctx))
121-
.collect::<VortexResult<Vec<_>>>()?;
122-
123-
StructArray::try_new(
124-
struct_array.names().clone(),
125-
taken_children,
126-
indices_slice_non_opt.len(),
127-
validity,
128-
)
129-
.map(|a| a.into_array())
130-
}
131116
DType::List(..) | DType::FixedSizeList(..) => {
132117
let mut builder = builder_with_capacity(
133118
&array.dtype().union_nullability(nullable),
@@ -147,7 +132,22 @@ pub fn take_canonical_array(
147132
}
148133
Ok(builder.finish())
149134
}
150-
d @ (DType::Null | DType::Extension(_) | DType::Variant(_)) => {
135+
DType::Struct(..) => {
136+
let struct_array = array.clone().execute::<StructArray>(ctx)?;
137+
let taken_children = struct_array
138+
.iter_unmasked_fields()
139+
.map(|c| take_canonical_array_non_nullable_indices(c, indices_slice_non_opt, ctx))
140+
.collect::<VortexResult<Vec<_>>>()?;
141+
142+
StructArray::try_new(
143+
struct_array.names().clone(),
144+
taken_children,
145+
indices_slice_non_opt.len(),
146+
validity,
147+
)
148+
.map(|a| a.into_array())
149+
}
150+
d @ (DType::Null | DType::Variant(_) | DType::Extension(_)) => {
151151
unreachable!("DType {d} not supported for fuzzing")
152152
}
153153
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,9 +242,9 @@ impl AggregateFnVTable for IsSorted {
242242
fn return_dtype(&self, _options: &Self::Options, input_dtype: &DType) -> Option<DType> {
243243
match input_dtype {
244244
DType::Null
245-
| DType::Struct(..)
246245
| DType::List(..)
247246
| DType::FixedSizeList(..)
247+
| DType::Struct(..)
248248
| DType::Variant(..) => None,
249249
_ => Some(DType::Bool(Nullability::NonNullable)),
250250
}
@@ -253,9 +253,9 @@ impl AggregateFnVTable for IsSorted {
253253
fn partial_dtype(&self, _options: &Self::Options, input_dtype: &DType) -> Option<DType> {
254254
match input_dtype {
255255
DType::Null
256-
| DType::Struct(..)
257256
| DType::List(..)
258257
| DType::FixedSizeList(..)
258+
| DType::Struct(..)
259259
| DType::Variant(..) => None,
260260
_ => Some(make_is_sorted_partial_dtype(input_dtype)),
261261
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,13 +240,13 @@ pub(crate) fn constant_uncompressed_size_in_bytes(
240240
array.len(),
241241
array.scalar().as_binary().value().map(|value| value.len()),
242242
)?,
243-
DType::Variant(_) => {
244-
vortex_bail!("UncompressedSizeInBytes is not supported for Variant arrays")
245-
}
246-
DType::Struct(..) | DType::List(..) | DType::FixedSizeList(..) | DType::Extension(_) => {
243+
DType::List(..) | DType::FixedSizeList(..) | DType::Struct(..) | DType::Extension(_) => {
247244
let canonical = array.array().clone().execute::<Canonical>(ctx)?;
248245
return canonical_uncompressed_size_in_bytes(&canonical, ctx);
249246
}
247+
DType::Variant(_) => {
248+
vortex_bail!("UncompressedSizeInBytes is not supported for Variant arrays")
249+
}
250250
};
251251

252252
value_size
@@ -293,10 +293,10 @@ fn supports_uncompressed_size_in_bytes(dtype: &DType) -> bool {
293293
DType::Struct(fields, _) => fields
294294
.fields()
295295
.all(|field| supports_uncompressed_size_in_bytes(&field)),
296+
DType::Variant(_) => false,
296297
DType::Extension(ext_dtype) => {
297298
supports_uncompressed_size_in_bytes(ext_dtype.storage_dtype())
298299
}
299-
DType::Variant(_) => false,
300300
DType::Null
301301
| DType::Bool(_)
302302
| DType::Primitive(..)

0 commit comments

Comments
 (0)