Skip to content

Add list_sum expression#8676

Merged
robert3005 merged 6 commits into
developfrom
mk/list-sum-expr
Jul 9, 2026
Merged

Add list_sum expression#8676
robert3005 merged 6 commits into
developfrom
mk/list-sum-expr

Conversation

@mhk197

@mhk197 mhk197 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Adds a vortex.list.sum scalar function: the sum of each row's list elements, one output row per input row — the equivalent of DuckDB's list_sum() and DataFusion's array_sum().

Note that list entries that are empty or all null need to be nullified. However, GroupedAccumulator returns 0 in these cases, so we need to either:

  1. Change the behavior of GroupedAccumulator and grouped sum (or even sum) kernels or
  2. Do a second pass of list element validity and sizes, construct a mask for the sums, and apply it.

The first option is probably best but can be applied in a followup PR.

@codspeed-hq

codspeed-hq Bot commented Jul 7, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 18.28%

⚡ 1 improved benchmark
❌ 2 regressed benchmarks
✅ 1616 untouched benchmarks
🆕 11 new benchmarks
⏩ 42 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation copy_nullable[65536] 573.2 µs 1,027.7 µs -44.23%
Simulation copy_non_nullable[65536] 794.2 µs 908.6 µs -12.59%
Simulation bitwise_not_vortex_buffer_mut[128] 273.6 ns 244.4 ns +11.93%
🆕 Simulation fsl_sum_large N/A 65.2 ms N/A
🆕 Simulation fsl_sum_medium N/A 727.8 µs N/A
🆕 Simulation fsl_sum_small N/A 56.5 µ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 495.9 ms N/A
🆕 Simulation list_sum_nullable_elements_medium N/A 4.9 ms N/A
🆕 Simulation list_sum_small N/A 143.4 µs N/A
🆕 Simulation listview_sum_large N/A 108.1 ms N/A
🆕 Simulation listview_sum_medium N/A 899.6 µs N/A
🆕 Simulation listview_sum_small N/A 73.4 µ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 (f3354dd) with develop (24ad8fd)

Open in CodSpeed

Footnotes

  1. 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.

@mhk197 mhk197 added the changelog/feature A new feature label Jul 8, 2026
@mhk197 mhk197 changed the title Add list_sum expression Add list_sum expression Jul 8, 2026
@mhk197 mhk197 marked this pull request as ready for review July 9, 2026 03:23
@mhk197 mhk197 requested a review from a team July 9, 2026 03:24
mhk197 added 4 commits July 8, 2026 20:24
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>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197 mhk197 force-pushed the mk/list-sum-expr branch from c5f717f to 8aad0cc Compare July 9, 2026 03:31

@myrrc myrrc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's push list_sum down to Duckdb and write a sqllogictest for this (can be in a followup PR). I'd also like to see a performance benchmark query, I'll prepare the machinery for that.


// Canonicalize upfront because `mask_empty_lists` needs access to list elements validity
// and sizes.
let canonical = array.clone().execute_until::<AnyCanonical>(ctx)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want AnyColumnar here and handle constant/canonical version at once

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is handled above in execute

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok no, the code above will be forced to canonicalise

@mhk197 mhk197 Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah we want to canonicalize a 1 row constant array, do sum, and then broadcast. will fix

mhk197 added 2 commits July 9, 2026 12:34
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197 mhk197 requested review from myrrc and robert3005 July 9, 2026 19:40
@robert3005 robert3005 merged commit 1bcaf3f into develop Jul 9, 2026
78 of 80 checks passed
@robert3005 robert3005 deleted the mk/list-sum-expr branch July 9, 2026 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants