Skip to content

Add sum_v2 with SQL empty-input semantics - #9590

Open
mhk197 wants to merge 4 commits into
developfrom
mk/sum-v2
Open

Add sum_v2 with SQL empty-input semantics#9590
mhk197 wants to merge 4 commits into
developfrom
mk/sum-v2

Conversation

@mhk197

@mhk197 mhk197 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add SumV2 with the distinct vortex.sum_v2 ID and an explicit { sum, is_overflow, is_empty } partial
  • return null for empty or all-null input and for integer or decimal overflow, while preserving valid zero sums
  • register a specialized grouped primitive kernel and route pushed DuckDB SUM through SumV2
  • keep legacy Sum, Stat::Sum, file statistics, zoned compatibility, Mean, and list_sum unchanged
  • update DuckDB SQL logic expectations for all-null sums

This implements the follow-up direction from #9113 after its revert in #9324. It introduces a new aggregate ID instead of changing the serialized contract of vortex.sum. It follows the compatibility direction explored in #8705, using the requested sum_v2 name and migrating the DuckDB pushed aggregate.

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@codspeed-hq

codspeed-hq Bot commented Aug 24, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 4.12%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 3 improved benchmarks
❌ 4 regressed benchmarks
✅ 1974 untouched benchmarks
🆕 15 new benchmarks
⏩ 54 skipped benchmarks1
🗄️ 4 archived benchmarks run2

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation sum_i32_nulls_scattered 483.8 µs 647.5 µs -25.28%
Simulation canonicalize_sparse_list[(512, 7, 4)] 571.7 µs 654.9 µs -12.7%
Simulation canonicalize_sparse_list[(1024, 17, 8)] 508.7 µs 577.5 µs -11.92%
Simulation cold_misaligned[(16, 64)] 380.2 µs 431.3 µs -11.84%
Simulation sum_i32_nullable_all_valid 93.8 µs 79.6 µs +17.81%
WallTime words_gather_scalar_avx2[65536] 9.4 µs 8.3 µs +13.47%
Simulation take[small_m/shuffled/primitive/nonnull/chunks=2048/indices=16] 376 µs 341.8 µs +10.03%
🆕 Simulation canonical_sum_v2_f64_all_valid N/A 243.1 µs N/A
🆕 Simulation canonical_sum_v2_f64_clustered_nulls N/A 288.7 µs N/A
🆕 Simulation canonical_sum_v2_i32_clustered_nulls N/A 278.1 µs N/A
🆕 Simulation canonical_sum_v2_i32_nullable_all_valid N/A 246.1 µs N/A
🆕 Simulation sum_v2_f64_all_valid N/A 226.2 µs N/A
🆕 Simulation sum_v2_f64_clustered_nulls N/A 272.5 µs N/A
🆕 Simulation sum_v2_i32_clustered_nulls N/A 261.9 µs N/A
🆕 Simulation sum_v2_i32_nullable_all_valid N/A 220.1 µs N/A
🆕 Simulation sum_v2_f64 N/A 233 µs N/A
🆕 Simulation sum_v2_f64_nulls_clustered N/A 246.3 µs N/A
🆕 Simulation sum_v2_i32 N/A 101.8 µs N/A
🆕 Simulation sum_v2_i32_nulls_clustered N/A 122.8 µs N/A
🆕 Simulation sum_v2_i32_nulls_scattered N/A 486.5 µs N/A
... ... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing mk/sum-v2 (9c95149) with develop (e4b3421)

Open in CodSpeed

Footnotes

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

  2. 4 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them.

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197 mhk197 added the changelog/chore A trivial change label Aug 24, 2026
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>

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

LGTM for duckdb changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate SumV2 into duckdb pushdown Create SumV2

2 participants