Skip to content

Commit f7294db

Browse files
authored
Skip empty chunks when aggregating over chunked arrays (#8168)
This was previously special cased for bool but it applies to all chunked array types this covers #8145 Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent 9f0dc85 commit f7294db

3 files changed

Lines changed: 31 additions & 5 deletions

File tree

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,6 @@ pub(super) fn accumulate_bool(
1919
array: &BoolArray,
2020
ctx: &mut ExecutionCtx,
2121
) -> VortexResult<()> {
22-
if array.is_empty() {
23-
return Ok(());
24-
}
25-
2622
let mask = array
2723
.as_ref()
2824
.validity()?

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,35 @@ mod tests {
660660
Ok(())
661661
}
662662

663+
/// Regression test for <https://github.com/vortex-data/vortex/issues/8145>.
664+
///
665+
/// A chunked array whose first chunk is an *empty* constant array — as produced by
666+
/// `fill_null` on an empty all-null chunk — returned `max = u32::MAX` because
667+
/// `ChunkedArrayAggregate` accumulated the empty chunk, folding its fill scalar into the
668+
/// running min/max. Empty chunks are now skipped during chunked aggregation.
669+
#[test]
670+
fn test_chunked_with_empty_constant_chunk() -> VortexResult<()> {
671+
let mut ctx = LEGACY_SESSION.create_execution_ctx();
672+
673+
let empty = ConstantArray::new(Scalar::primitive(u32::MAX, Nullability::NonNullable), 0)
674+
.into_array();
675+
let chunk1 = PrimitiveArray::new(buffer![7631471u32], Validity::NonNullable).into_array();
676+
let chunk2 = PrimitiveArray::new(buffer![0u32], Validity::NonNullable).into_array();
677+
let chunked = ChunkedArray::try_new(
678+
vec![empty, chunk1, chunk2],
679+
DType::Primitive(PType::U32, Nullability::NonNullable),
680+
)?;
681+
682+
assert_eq!(
683+
min_max(&chunked.into_array(), &mut ctx)?,
684+
Some(MinMaxResult {
685+
min: Scalar::primitive(0u32, Nullability::NonNullable),
686+
max: Scalar::primitive(7631471u32, Nullability::NonNullable),
687+
})
688+
);
689+
Ok(())
690+
}
691+
663692
#[test]
664693
fn test_varbin_all_nulls() -> VortexResult<()> {
665694
let array = VarBinArray::from_iter(

vortex-array/src/arrays/chunked/compute/aggregate.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ impl DynAggregateKernel for ChunkedArrayAggregate {
2626
};
2727

2828
let mut acc = aggregate_fn.accumulator(chunked.dtype())?;
29-
for chunk in chunked.iter_chunks() {
29+
// Skip empty chunks: they contribute no elements.
30+
for chunk in chunked.non_empty_chunks() {
3031
acc.accumulate(chunk, ctx)?;
3132
}
3233
// Return the partial (not finalized) result, since the outer accumulator

0 commit comments

Comments
 (0)