Skip to content

Commit 263d8d9

Browse files
committed
IsSortedKernel
Signed-off-by: Nicholas Gates <nick@nickgates.com>
1 parent 80613d3 commit 263d8d9

1 file changed

Lines changed: 54 additions & 42 deletions

File tree

  • vortex-array/src/aggregate_fn/fns/is_sorted

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

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -175,15 +175,18 @@ impl IsSorted {
175175
}
176176
let first_value = batch.scalar_at(0)?.into_nullable();
177177
let last_value = batch.scalar_at(batch.len() - 1)?.into_nullable();
178-
Ok(Scalar::struct_(
179-
partial_dtype,
180-
vec![
181-
Scalar::bool(is_sorted, Nullability::NonNullable),
182-
Scalar::bool(strict, Nullability::NonNullable),
183-
first_value,
184-
last_value,
185-
],
186-
))
178+
// SAFETY: We constructed partial_dtype and the children match its field dtypes exactly.
179+
Ok(unsafe {
180+
Scalar::struct_unchecked(
181+
partial_dtype,
182+
[
183+
Scalar::bool(is_sorted, Nullability::NonNullable),
184+
Scalar::bool(strict, Nullability::NonNullable),
185+
first_value,
186+
last_value,
187+
],
188+
)
189+
})
187190
}
188191
}
189192

@@ -314,44 +317,53 @@ impl AggregateFnVTable for IsSorted {
314317

315318
fn flush(&self, partial: &mut Self::Partial) -> VortexResult<Scalar> {
316319
let dtype = make_is_sorted_partial_dtype(&partial.element_dtype);
317-
let result = match (&partial.first_value, &partial.last_value) {
320+
321+
// Take ownership of the values to avoid cloning.
322+
let first = partial.first_value.take();
323+
let last = partial.last_value.take();
324+
let is_sorted = partial.is_sorted;
325+
let strict = partial.strict;
326+
327+
// Reset state.
328+
partial.is_sorted = true;
329+
330+
let result = match (first, last) {
318331
(None, _) => {
319332
// Empty accumulator — return null struct.
320333
Scalar::null(dtype)
321334
}
322-
(Some(first_value), Some(last_value)) => Scalar::struct_(
323-
dtype,
324-
vec![
325-
Scalar::bool(partial.is_sorted, Nullability::NonNullable),
326-
Scalar::bool(partial.strict, Nullability::NonNullable),
327-
first_value
328-
.clone()
329-
.cast(&partial.element_dtype.as_nullable())?,
330-
last_value
331-
.clone()
332-
.cast(&partial.element_dtype.as_nullable())?,
333-
],
334-
),
335-
(Some(first_value), None) => Scalar::struct_(
336-
dtype,
337-
vec![
338-
Scalar::bool(partial.is_sorted, Nullability::NonNullable),
339-
Scalar::bool(partial.strict, Nullability::NonNullable),
340-
first_value
341-
.clone()
342-
.cast(&partial.element_dtype.as_nullable())?,
343-
first_value
344-
.clone()
345-
.cast(&partial.element_dtype.as_nullable())?,
346-
],
347-
),
335+
(Some(first_value), Some(last_value)) => {
336+
// Values are already nullable from into_nullable_unchecked in accumulate.
337+
// SAFETY: We constructed partial_dtype and the children match its field dtypes.
338+
unsafe {
339+
Scalar::struct_unchecked(
340+
dtype,
341+
[
342+
Scalar::bool(is_sorted, Nullability::NonNullable),
343+
Scalar::bool(strict, Nullability::NonNullable),
344+
first_value,
345+
last_value,
346+
],
347+
)
348+
}
349+
}
350+
(Some(first_value), None) => {
351+
let cloned = first_value.clone();
352+
// SAFETY: We constructed partial_dtype and the children match its field dtypes.
353+
unsafe {
354+
Scalar::struct_unchecked(
355+
dtype,
356+
[
357+
Scalar::bool(is_sorted, Nullability::NonNullable),
358+
Scalar::bool(strict, Nullability::NonNullable),
359+
first_value,
360+
cloned,
361+
],
362+
)
363+
}
364+
}
348365
};
349366

350-
// Reset state.
351-
partial.is_sorted = true;
352-
partial.first_value = None;
353-
partial.last_value = None;
354-
355367
Ok(result)
356368
}
357369

@@ -449,7 +461,7 @@ impl AggregateFnVTable for IsSorted {
449461
Canonical::Extension(e) => check_extension_sorted(e, partial.strict, ctx)?,
450462
Canonical::Null(_) => !partial.strict,
451463
// Struct, List, FixedSizeList should have been filtered out by return_dtype
452-
_ => false,
464+
_ => unreachable!(),
453465
};
454466

455467
if !batch_is_sorted {

0 commit comments

Comments
 (0)