Skip to content

Commit e06e371

Browse files
authored
Faster BoolArray::min_max via true_count instead of set_slices (#7599)
There's no need to compute valid slices, we can just use true_count fix #7598 Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent 5b0c5f9 commit e06e371

1 file changed

Lines changed: 18 additions & 41 deletions

File tree

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

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

Lines changed: 18 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use std::ops::BitAnd;
55

66
use vortex_error::VortexResult;
7-
use vortex_mask::Mask;
7+
use vortex_mask::AllOr;
88

99
use super::MinMaxPartial;
1010
use super::MinMaxResult;
@@ -27,53 +27,30 @@ pub(super) fn accumulate_bool(
2727
.as_ref()
2828
.validity()?
2929
.execute_mask(array.as_ref().len(), ctx)?;
30-
let true_non_null = match &mask {
31-
Mask::AllTrue(_) => array.to_bit_buffer(),
32-
Mask::AllFalse(_) => return Ok(()),
33-
Mask::Values(v) => array.to_bit_buffer().bitand(v.bit_buffer()),
30+
let (true_count, valid_count) = match mask.bit_buffer() {
31+
AllOr::None => return Ok(()),
32+
AllOr::All => (array.to_bit_buffer().true_count(), array.as_ref().len()),
33+
AllOr::Some(validity) => (
34+
array.to_bit_buffer().bitand(validity).true_count(),
35+
validity.true_count(),
36+
),
3437
};
3538

36-
let mut true_slices = true_non_null.set_slices();
37-
38-
let Some(slice) = true_slices.next() else {
39-
// all false
40-
partial.merge(Some(MinMaxResult {
41-
min: Scalar::bool(false, NonNullable),
42-
max: Scalar::bool(false, NonNullable),
43-
}));
44-
return Ok(());
45-
};
46-
47-
if slice.0 == 0 && slice.1 == array.len() {
48-
// all true
49-
partial.merge(Some(MinMaxResult {
50-
min: Scalar::bool(true, NonNullable),
51-
max: Scalar::bool(true, NonNullable),
52-
}));
39+
if valid_count == 0 {
5340
return Ok(());
5441
}
5542

56-
// Check for valid false values when we have a partial validity mask
57-
match &mask {
58-
Mask::AllTrue(_) | Mask::AllFalse(_) => {}
59-
Mask::Values(v) => {
60-
let false_non_null = (!array.to_bit_buffer()).bitand(v.bit_buffer());
61-
let mut false_slices = false_non_null.set_slices();
62-
63-
if false_slices.next().is_none() {
64-
// No false values, so all valid values are true
65-
partial.merge(Some(MinMaxResult {
66-
min: Scalar::bool(true, NonNullable),
67-
max: Scalar::bool(true, NonNullable),
68-
}));
69-
return Ok(());
70-
}
71-
}
72-
}
43+
let (min, max) = if true_count == 0 {
44+
(false, false)
45+
} else if true_count == valid_count {
46+
(true, true)
47+
} else {
48+
(false, true)
49+
};
7350

7451
partial.merge(Some(MinMaxResult {
75-
min: Scalar::bool(false, NonNullable),
76-
max: Scalar::bool(true, NonNullable),
52+
min: Scalar::bool(min, NonNullable),
53+
max: Scalar::bool(max, NonNullable),
7754
}));
7855
Ok(())
7956
}

0 commit comments

Comments
 (0)