Repository navigation
Commit 6051e06
committed
Support out of order grouped aggregate accumulation
Rebases #8379 onto develop and addresses the outstanding review
comments.
The grouped accumulator now takes caller-assigned dense `GroupIds`
(`{ids: ArrayRef, num_groups}`) directly, instead of requiring values to
be pre-sorted into complete list groups. Ids may repeat, arrive out of
order, or be absent from a batch.
## Changes
- Dispatch grouped kernels on both the values encoding and the group-id
encoding, with wildcard fallbacks in each dimension.
- Cache materialized and validated group ids across clones, so several
aggregates over the same ids validate once.
- Let each aggregate own its dense grouped state (`GroupedState`), with
the generic one-partial-per-group implementation as the fallback.
- Store grouped count state as one `u64` per group and grouped sum state
in monomorphic `u64`/`i64`/`f64`/decimal vectors, choosing the decimal
width once from the sum dtype.
- Port `SumV2` onto the same dense sums, adding the per-group empty flag
its SQL semantics need, so both sums share one accumulation kernel.
- Merge count and sum partial arrays without `execute_scalar` per row.
- Adapt between typed scatter for unclustered ids and per-run reduction
for clustered ids.
- Use one stable counting-sort gather per batch in the universal
fallback instead of one `take` per group.
- Keep `GroupedArray::dense_input` as the list-group adapter for
`list_sum`, which restores list semantics (null, empty, and all-null
lists sum to null) on top of the dense result.
## Rebase notes
`develop` reverted SQL null-on-empty semantics for `Sum` (#9324) and
re-landed them as a separate `SumV2` (#9590), so this branch follows
both contracts: `Sum` starts a group at zero and treats a null partial
as overflow, while `SumV2` keeps `{sum, is_overflow, is_empty}` and
finalizes an untouched group to null. `SumAggregateOpts` is gone, and
both aggregates take `NumericalAggregateOpts`.
`SumV2`'s list-shaped `PrimitiveGroupedSumV2EncodingKernel` is replaced
by a dense kernel that reuses `Sum`'s accumulation and then clears the
empty flag of every group that saw a valid value, so `sum_v2` keeps a
fast grouped path rather than falling back to per-group accumulators.
The single-partial `flush` shortcut from #9597 no longer applies: dense
state produces one partial array per flush, so there is no chunk to
collapse, and its three tests went with it.
## Review comments
- Group ids are an `ArrayRef` so kernels can dispatch on their encoding
(Constant, RLE, ...), and `GroupIds` is the "Group struct" the
accumulator API talks in.
- The hand-rolled `for_each_valid_idx` helpers are gone: the kernels
take the mask's bit buffer, handle the all-true and all-false cases
directly, and let `BitBuffer::for_each_set_index` walk the rest
instead of materializing the mask's index vector.
- The run-detection helpers shared by the count and sum kernels moved to
`aggregate_fn::kernels` instead of being duplicated per aggregate.
- The accumulator tracks `num_groups` itself, so `flush_partials` and
`finish` no longer take it; `ensure_groups` is exposed for callers
that need trailing groups no batch mentioned.
- `merge_group` names its parameters `into_group` / `from_group`.
- The benchmark input builder is now `DenseGroupedInput::clustered`.
## list_sum performance
`list_sum` is the one caller that has to build dense ids for input that
is already grouped, so it pays for the new API. The adapter avoids the
two costs that dominated: it reuses the element array as the values
array whenever the groups already cover it in order (null lists keep
their own ids, and their meaningless sums are masked out afterwards),
and it shares one set of group ranges and validity between the
adaptation and the mask.
What remains is the id array itself, roughly half a nanosecond per
element. Medians from `cargo bench -p vortex-array --bench list_sum`,
develop (d0a59fe) vs this branch on the same machine:
| benchmark | develop | this branch |
|---|---:|---:|
| `fsl_sum_large` | 5.1 us | 7.3 us |
| `fsl_sum_medium` | 3.4 us | 4.8 us |
| `fsl_sum_small` | 2.4 us | 3.2 us |
| `list_sum_large` | 11.2 us | 13.7 us |
| `list_sum_medium` | 8.9 us | 9.9 us |
| `list_sum_small` | 7.6 us | 7.5 us |
| `list_sum_nullable_elements_large` | 57.6 us | 18.1 us |
| `list_sum_nullable_elements_medium` | 31.2 us | 12.3 us |
| `listview_sum_large` | 6.9 us | 9.6 us |
| `listview_sum_medium` | 4.9 us | 6.0 us |
| `listview_sum_small` | 3.8 us | 3.9 us |
Lists whose elements are all valid are 20-40% slower; lists with
nullable elements are 2.5-3x faster, since the old path sliced the
element mask per group. Closing the remaining gap wants a grouped
kernel registered for run-encoded group ids, which the new two-encoding
kernel lookup already supports - a follow-up rather than part of this
change.
## Validation
- `cargo +nightly fmt --all`
- `cargo build -p vortex-array --all-targets`
- `cargo test -p vortex-array --lib` (3,509 passed, 1 ignored)
- `cargo test --doc -p vortex-array`
- `cargo bench -p vortex-array --bench list_sum`
- `cargo bench -p vortex-array --bench aggregate_grouped -- --test`
- `cargo clippy -p vortex-array --all-targets --all-features`
- `RUSTDOCFLAGS="-D warnings" cargo doc --no-deps -p vortex-array`
A workspace-wide build ran out of disk in this environment; the change
is confined to `vortex-array`, and no other crate references the grouped
accumulator API.
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Robert Kruszewski <robert@spiraldb.com>
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8VcUXWmDmNH2JjJY4aDQ81 parent d0a59fe commit 6051e06
17 files changed
Lines changed: 2735 additions & 1121 deletions
File tree
- vortex-array
- benches
- src
- aggregate_fn
- fns
- count
- mean
- sum_v2
- sum
- scalar_fn/fns
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| 13 | + | |
13 | 14 | | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
17 | 18 | | |
18 | 19 | | |
| 20 | + | |
19 | 21 | | |
20 | | - | |
21 | 22 | | |
22 | 23 | | |
23 | 24 | | |
24 | | - | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
28 | 27 | | |
29 | 28 | | |
30 | 29 | | |
| |||
40 | 39 | | |
41 | 40 | | |
42 | 41 | | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
43 | 58 | | |
44 | 59 | | |
45 | 60 | | |
| |||
52 | 67 | | |
53 | 68 | | |
54 | 69 | | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
66 | 74 | | |
67 | | - | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
68 | 88 | | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
| 89 | + | |
| 90 | + | |
77 | 91 | | |
78 | 92 | | |
79 | | - | |
| 93 | + | |
80 | 94 | | |
81 | 95 | | |
82 | 96 | | |
83 | 97 | | |
84 | 98 | | |
85 | 99 | | |
86 | | - | |
| 100 | + | |
87 | 101 | | |
88 | 102 | | |
89 | 103 | | |
90 | 104 | | |
91 | 105 | | |
92 | | - | |
| 106 | + | |
93 | 107 | | |
94 | 108 | | |
95 | 109 | | |
| |||
99 | 113 | | |
100 | 114 | | |
101 | 115 | | |
102 | | - | |
| 116 | + | |
103 | 117 | | |
104 | 118 | | |
105 | 119 | | |
106 | 120 | | |
107 | 121 | | |
108 | | - | |
| 122 | + | |
109 | 123 | | |
110 | 124 | | |
111 | 125 | | |
112 | 126 | | |
113 | 127 | | |
114 | 128 | | |
115 | | - | |
| 129 | + | |
116 | 130 | | |
117 | 131 | | |
118 | 132 | | |
119 | 133 | | |
120 | 134 | | |
121 | | - | |
| 135 | + | |
122 | 136 | | |
123 | 137 | | |
124 | 138 | | |
| |||
129 | 143 | | |
130 | 144 | | |
131 | 145 | | |
132 | | - | |
| 146 | + | |
133 | 147 | | |
134 | 148 | | |
135 | 149 | | |
136 | 150 | | |
137 | 151 | | |
138 | | - | |
| 152 | + | |
139 | 153 | | |
140 | 154 | | |
141 | 155 | | |
142 | 156 | | |
143 | 157 | | |
144 | | - | |
| 158 | + | |
145 | 159 | | |
146 | 160 | | |
147 | 161 | | |
148 | 162 | | |
149 | 163 | | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
154 | 179 | | |
155 | 180 | | |
156 | 181 | | |
157 | | - | |
| 182 | + | |
158 | 183 | | |
159 | | - | |
| 184 | + | |
| 185 | + | |
160 | 186 | | |
161 | | - | |
162 | | - | |
163 | | - | |
164 | | - | |
165 | | - | |
166 | | - | |
167 | | - | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
168 | 192 | | |
169 | | - | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
170 | 200 | | |
171 | 201 | | |
172 | 202 | | |
| |||
233 | 263 | | |
234 | 264 | | |
235 | 265 | | |
236 | | - | |
237 | | - | |
238 | | - | |
239 | | - | |
240 | | - | |
241 | | - | |
242 | | - | |
243 | | - | |
244 | | - | |
245 | | - | |
246 | | - | |
247 | | - | |
248 | | - | |
249 | | - | |
250 | | - | |
251 | | - | |
252 | | - | |
253 | | - | |
254 | | - | |
255 | | - | |
256 | | - | |
257 | | - | |
258 | | - | |
259 | | - | |
260 | | - | |
261 | | - | |
262 | | - | |
263 | | - | |
264 | | - | |
265 | | - | |
266 | | - | |
267 | 266 | | |
268 | | - | |
269 | | - | |
270 | | - | |
271 | | - | |
272 | | - | |
273 | | - | |
274 | | - | |
275 | | - | |
276 | | - | |
277 | | - | |
278 | | - | |
279 | | - | |
280 | | - | |
281 | | - | |
282 | | - | |
283 | | - | |
284 | | - | |
285 | | - | |
286 | | - | |
287 | | - | |
288 | | - | |
289 | | - | |
290 | | - | |
291 | | - | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
296 | | - | |
297 | | - | |
298 | | - | |
299 | | - | |
300 | | - | |
| 267 | + | |
301 | 268 | | |
302 | 269 | | |
303 | 270 | | |
304 | | - | |
| 271 | + | |
305 | 272 | | |
306 | 273 | | |
307 | 274 | | |
308 | | - | |
309 | | - | |
| 275 | + | |
| 276 | + | |
310 | 277 | | |
311 | 278 | | |
312 | | - | |
| 279 | + | |
313 | 280 | | |
314 | 281 | | |
315 | | - | |
316 | | - | |
317 | | - | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
318 | 285 | | |
319 | 286 | | |
320 | | - | |
| 287 | + | |
321 | 288 | | |
322 | 289 | | |
323 | | - | |
324 | | - | |
325 | | - | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
326 | 293 | | |
327 | 294 | | |
328 | | - | |
| 295 | + | |
329 | 296 | | |
330 | 297 | | |
331 | | - | |
332 | | - | |
333 | | - | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
334 | 301 | | |
335 | 302 | | |
336 | 303 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
172 | 172 | | |
173 | 173 | | |
174 | 174 | | |
175 | | - | |
| 175 | + | |
176 | 176 | | |
177 | 177 | | |
178 | 178 | | |
| |||
0 commit comments