Skip to content

mean() for decimals#8649

Merged
myrrc merged 1 commit into
developfrom
myrrc/decimal-mean
Jul 7, 2026
Merged

mean() for decimals#8649
myrrc merged 1 commit into
developfrom
myrrc/decimal-mean

Conversation

@myrrc

@myrrc myrrc commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Support mean() for decimals (keeps mean as Decimal). Support casting decimals to f64 (we need this for aggregate pushdown because avg(decimal) in duckdb is f64).

@myrrc myrrc requested review from a team and robert3005 July 3, 2026 10:20
@myrrc myrrc added the changelog/feature A new feature label Jul 3, 2026
@myrrc myrrc enabled auto-merge (squash) July 3, 2026 10:23
@codspeed-hq

codspeed-hq Bot commented Jul 3, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 10.63%

⚡ 6 improved benchmarks
❌ 1 regressed benchmark
✅ 1600 untouched benchmarks
⏩ 42 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation chunked_varbinview_canonical_into[(1000, 10)] 154.7 µs 190.8 µs -18.93%
Simulation bitwise_not_vortex_buffer_mut[128] 273.6 ns 215.3 ns +27.1%
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 chunked_varbinview_into_canonical[(100, 100)] 307 µs 272.1 µs +12.86%
Simulation chunked_varbinview_opt_canonical_into[(100, 100)] 341 µs 305.4 µs +11.65%
Simulation rebuild_naive 101.7 µs 91.4 µs +11.32%

Tip

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


Comparing myrrc/decimal-mean (b511dcf) with develop (8f72595)

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.

Comment thread vortex-array/src/aggregate_fn/fns/mean/mod.rs

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

Mostly some clarifying questions

Comment thread vortex-array/src/aggregate_fn/fns/mean/mod.rs Outdated
Comment thread vortex-array/src/aggregate_fn/fns/mean/mod.rs
Comment thread vortex-array/src/arrays/decimal/compute/cast.rs Outdated
Comment thread vortex-array/src/arrays/decimal/compute/cast.rs
Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
@myrrc myrrc force-pushed the myrrc/decimal-mean branch from 8c8ae7c to b511dcf Compare July 7, 2026 11:04
@myrrc myrrc requested a review from robert3005 July 7, 2026 11:04
@myrrc myrrc merged commit e6c312a into develop Jul 7, 2026
94 of 98 checks passed
@myrrc myrrc deleted the myrrc/decimal-mean branch July 7, 2026 12:04
myrrc added a commit that referenced this pull request Jul 9, 2026
This PR adds support for pushing down ungropped aggregates
min,max,sum,avg,mean,first,any_value,count(col), and count(*).

It consists of roughly 3 parts:
1. Optimizer pass which works same as scalar function pushdown, but
without visitors, because we can't express child extract with a visitor.
2. General accumulator handling in table_function.rs. This includes
computing partials in every thread (so that we read data in parallel)
and merging them, and also synchonization so that only one thread would
write the global aggregated row count
3. Handling of count_star() (count(*)) a.k.a row count. Row count is a
special accumulator because it doesn't need the array, and there's no
column it aggregates.

I've tried the implementation with CountStart being a separate
accumulator, and it is much more complex than current one, because you
can't provide a real column to accumulate on in select(), and you need
to create a dummy one. So, we pay the cost of a separate atomic and a
count_star positions vector, but our expression projection handling
stays simple.
We also reject count(*) if it's the only aggregate because duckdb
calculating it natively (just using chunk lengths) is faster than our
accumulator pipeline

On the API input note, nearly all aggregate functions except for
count_star() in duckdb have one non-const argument so input is a pair of
a column and an expression.

Depends on #8649

Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
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.

2 participants