Add list_sum expression#8676
Open
mhk197 wants to merge 4 commits into
Open
Conversation
Merging this PR will improve performance by 12.58%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | chunked_varbinview_into_canonical[(1000, 10)] |
169.7 µs | 206.1 µs | -17.65% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
273.6 ns | 215.3 ns | +27.1% |
| ⚡ | Simulation | chunked_varbinview_canonical_into[(1000, 10)] |
190.5 µs | 154.6 µs | +23.17% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[1024] |
333.9 ns | 275.6 ns | +21.17% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[2048] |
427.8 ns | 369.4 ns | +15.79% |
| 🆕 | Simulation | fsl_sum_large |
N/A | 65.2 ms | N/A |
| 🆕 | Simulation | fsl_sum_medium |
N/A | 728.1 µs | N/A |
| 🆕 | Simulation | fsl_sum_small |
N/A | 57 µs | N/A |
| 🆕 | Simulation | list_sum_large |
N/A | 135.7 ms | N/A |
| 🆕 | Simulation | list_sum_medium |
N/A | 1.1 ms | N/A |
| 🆕 | Simulation | list_sum_nullable_elements_large |
N/A | 496 ms | N/A |
| 🆕 | Simulation | list_sum_nullable_elements_medium |
N/A | 4.9 ms | N/A |
| 🆕 | Simulation | list_sum_small |
N/A | 144 µs | N/A |
| 🆕 | Simulation | listview_sum_large |
N/A | 108.1 ms | N/A |
| 🆕 | Simulation | listview_sum_medium |
N/A | 900.2 µs | N/A |
| 🆕 | Simulation | listview_sum_small |
N/A | 74 µs | N/A |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing mk/list-sum-expr (8aad0cc) with develop (11f15a9)
Footnotes
-
42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Design for a vortex.list.sum scalar function: survey of DuckDB/DataFusion/ ClickHouse/Polars semantics, reuse of the grouped-sum machinery (GroupedAccumulator), SQL NULL-on-empty semantics for engine pushdown parity, NaN handling via NumericalAggregateOpts, and version-gated DataFusion integration (array_sum absent from the pinned DF 54). Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Implements the vortex.list.sum scalar function per the design doc: sums each list's elements through the grouped aggregate machinery (GroupedAccumulator<Sum>) so widening, overflow, and NaN semantics match sum() exactly, then nulls out empty and all-null lists to follow SQL SUM semantics (matching DuckDB list_sum and DataFusion array_sum). NaN handling is controlled by NumericalAggregateOpts, defaulting to skip_nans like sum(). Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
vortex.list.sumscalar function: the sum of each row's list elements, one output row per input row — the equivalent of DuckDB'slist_sum()and DataFusion'sarray_sum().Note that list entries that are empty or all null need to be nullified. However,
GroupedAccumulatorreturns 0 in these cases, so we need to either:GroupedAccumulatorand grouped sum (or even sum) kernels orThe first option is probably best but can be applied in a followup PR.