Repository navigation
Commit 201d03b
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.
`list_sum` now sums with `SumV2` (#9621), so its empty and all-null list
masking is gone; it only has to restore the one case dense ids cannot
express, a null list.
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: one `u32` per element,
read once to sum and once to clear empty flags, plus a mask that nulls
the lists dense ids cannot represent. The adapter already avoids the
larger costs - it reuses the element array as the values array whenever
the groups cover it in order, and shares one set of group ranges and
validity with the mask - but the id array itself remains.
Fastest-of-100 from `cargo bench -p vortex-array --bench list_sum`,
develop (1fe8dda) vs this branch on the same machine. Medians are noisy
on this host, so these are the fastest samples:
| benchmark | develop | this branch |
|---|---:|---:|
| `fsl_sum_large` | 7.2 us | 11.1 us |
| `fsl_sum_medium` | 5.8 us | 8.2 us |
| `fsl_sum_small` | 4.8 us | 6.6 us |
| `list_sum_large` | 11.5 us | 18.5 us |
| `list_sum_medium` | 9.9 us | 13.8 us |
| `list_sum_small` | 9.0 us | 10.8 us |
| `list_sum_nullable_elements_large` | 21.9 us | 20.9 us |
| `list_sum_nullable_elements_medium` | 14.9 us | 15.3 us |
| `listview_sum_large` | 7.3 us | 14.1 us |
| `listview_sum_medium` | 6.1 us | 9.7 us |
| `listview_sum_small` | 5.2 us | 7.2 us |
Lists that are already grouped are 20-90% slower; only the
nullable-element cases come out level. `SumV2`'s list kernel reduces one
range at a time and never touches a per-element id, which is what dense
ids cost here.
Closing this wants a grouped kernel registered for run-encoded group
ids, so a list adapter can hand over `(offsets, group ids)` without
materializing one id per element and the kernel can reduce a run at a
time. The two-encoding kernel lookup added here already supports
registering it; the kernel itself is follow-up work, not part of this
rebase.
## Validation
- `cargo +nightly fmt --all`
- `cargo build -p vortex-array --all-targets`
- `cargo test -p vortex-array --lib` (3,511 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 1fe8dda commit 201d03b
17 files changed
Lines changed: 2705 additions & 1098 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