feat: `Mean` aggregate by blaginin · Pull Request #7201 · vortex-data/vortex

blaginin · 2026-03-29T21:23:15Z

No description provided.

Signed-off-by: blaginin <dima@spiraldb.com> Co-authored-by: Claude <noreply@anthropic.com>

Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: blaginin <dima@spiraldb.com>

gatesn · 2026-03-30T09:05:25Z

+/// Return the count of non-null elements in an array.
+///
+/// See [`Count`] for details.
+pub fn count(array: &ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult<u64> {


If this is counting things in an array, should it return a usize?

gatesn · 2026-03-30T09:08:11Z

+        false
+    }
+
+    fn accumulate(


We should be able to register a generic aggregate kernel to reduce count-non-null to be Array.validity().sum(), then we avoid decompressing all the data.

575672a

added generic kernels first, but then it's hard to express "I never want Count::accumulate to be called" - so switched to try_accumulate instead

gatesn · 2026-03-30T09:12:34Z

+}
+
+fn partial_struct_dtype() -> DType {
+    DType::Struct(


Can you static LazyLock this entire dtype inside this function?

joseph-isaacs

This should be decimal for decimals?

joseph-isaacs · 2026-03-30T09:34:05Z

+pub struct MeanPartial {
+    sum: f64,
+    count: u64,
+}


This will compute with a pretty large error?

$$\mu_n = \sum_{i=0}^n x_i/n \iff \mu_{n+1} = \mu_n + \frac{x_{n+1} - \mu_n}{1+n}$$

you need only hold the partial mean and count

You can incrementally push the next value into this partial, but you cannot combine two partials afaik?

For combining you do. $$\mu_{AB} = \frac{n_A \mu_A + n_B \mu_B}{n_A + n_B}$$

DF sum is T and ours is f64

Error comes from numbers of different scales being combined, not from number of OPS.

Since we are storing these on disk we cannot "just change it later".

DF only supports AVG for floats, decimals, and durations. And coerces all floats to f64.

| Input Type | DuckDB | DataFusion | Velox | |--------------|--------|------------|------------| | Integer types| DOUBLE | Float64 | DOUBLE | | FLOAT/REAL | DOUBLE | Float64 | REAL | | DOUBLE | DOUBLE | Float64 | DOUBLE | | DECIMAL(p,s) | DOUBLE | Float64 | DECIMAL(p,s)|

thought about this more - i think it should decimal for decimals? - happy to change

Signed-off-by: blaginin <github@blaginin.me>

codspeed-hq · 2026-04-02T15:20:32Z

Merging this PR will improve performance by 27.62%

⚡ 2 improved benchmarks
✅ 1161 untouched benchmarks

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
⚡	Simulation	`varbinview_zip_block_mask`	3.7 ms	2.9 ms	+27.62%
⚡	Simulation	`varbinview_zip_fragmented_mask`	7.3 ms	6.5 ms	+11.46%

_{Comparing db/count-and-mean (4401d73) with develop (c0c6fb9)}

Signed-off-by: blaginin <github@blaginin.me>

# Conflicts: # vortex-array/public-api.lock # vortex-array/src/aggregate_fn/fns/mod.rs

Signed-off-by: blaginin <github@blaginin.me>

Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: blaginin <github@blaginin.me>

Signed-off-by: blaginin <github@blaginin.me>

joseph-isaacs · 2026-04-09T09:23:34Z

we can factor this out of this PR, we could at a later date add the back to on disk version will be the same?

disk version will be the same

i blocked serialization for now?

we can factor this out of this PR

why?

joseph-isaacs · 2026-04-09T09:25:35Z

+/// - Decimals are kept as decimals with widened precision and scale
+///   (`+4` each, capped at [`MAX_PRECISION`] / [`MAX_SCALE`]), matching
+///   DataFusion's `coerce_avg_type`.


I think you definitely want higher for the sum, that is only 10k max sized values for an overflow

yeah, but Sum.partial_dtype() also adds +10

Then this comment need to explain what the +4 is for, 4 extra least sig figures?

Signed-off-by: blaginin <github@blaginin.me>

joseph-isaacs · 2026-04-21T16:03:00Z

+        let sum_cast = sum.cast(target.clone())?;
+        let count_cast = count.cast(target)?;
+        sum_cast.binary(count_cast, Operator::Div)


handle div by zero?

Signed-off-by: blaginin <github@blaginin.me>

joseph-isaacs · 2026-04-23T17:23:13Z

This is a mean? not avg?

blaginin · 2026-04-23T17:27:58Z

I think the pattern is avg is the function and mean is an alias

Signed-off-by: blaginin <github@blaginin.me>

joseph-isaacs · 2026-04-24T12:35:33Z

I think we want mean with avg as an alias

joseph-isaacs · 2026-04-24T12:35:50Z

or really arithmetic_mean

joseph-isaacs · 2026-04-27T13:21:43Z

+        let sum = sum_cast.as_primitive().typed_value::<f64>();
+        let count = count_cast.as_primitive().typed_value::<f64>();
+        let value = match (sum, count) {
+            (None, _) | (_, None) | (_, Some(0.0)) => return Ok(Scalar::null(target)), // Sum overflowed


empty or overflowed?

joseph-isaacs · 2026-04-27T13:22:44Z

+    /// Field name for the left child in the partial struct dtype.
+    fn left_name(&self) -> &'static str {
+        "left"
+    }
+
+    /// Field name for the right child in the partial struct dtype.
+    fn right_name(&self) -> &'static str {
+        "right"
+    }


why default these?

those don't matter, so may as well use left/right

joseph-isaacs · 2026-04-27T13:22:51Z

+
+impl<T: BinaryCombined> AggregateFnVTable for Combined<T> {
+    type Options = CombinedOptions<T>;
+    type Partial = (LeftPartial<T>, RightPartial<T>);


How will this partial be seralized?

as a struct scalar with left/right fields

joseph-isaacs · 2026-04-27T13:23:22Z

+fn struct_dtype(left_name: &str, right_name: &str, left: DType, right: DType) -> DType {
+    DType::Struct(
+        StructFields::new(
+            FieldNames::from_iter([FieldName::from(left_name), FieldName::from(right_name)]),
+            vec![left, right],
+        ),
+        Nullability::NonNullable,


method on combined?

Signed-off-by: blaginin <github@blaginin.me>

Count and Mean aggregates

6bafabe

Signed-off-by: blaginin <dima@spiraldb.com> Co-authored-by: Claude <noreply@anthropic.com>

blaginin self-assigned this Mar 29, 2026

validity cleanup

ee67d80

Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: blaginin <dima@spiraldb.com>

gatesn reviewed Mar 30, 2026

View reviewed changes

gatesn added the changelog/feature A new feature label Mar 30, 2026

joseph-isaacs requested changes Mar 30, 2026

View reviewed changes

blaginin added 3 commits April 2, 2026 14:24

Merge branch 'develop' into db/count-and-mean

2d38b62

generic kernels

4e16945

Signed-off-by: blaginin <github@blaginin.me>

try_accumulate feels cleaner

575672a

Signed-off-by: blaginin <github@blaginin.me>

blaginin changed the title ~~Count and Mean aggregates~~ feat: Count and Mean aggregates Apr 2, 2026

only mean; mean is not serializable

3c8b12b

Signed-off-by: blaginin <github@blaginin.me>

blaginin changed the title ~~feat: Count and Mean aggregates~~ feat: Mean aggregate Apr 2, 2026

blaginin and others added 4 commits April 2, 2026 17:56

lock file update

46db9fa

Signed-off-by: blaginin <github@blaginin.me>

Merge branch 'develop' into db/count-and-mean

53806a4

# Conflicts: # vortex-array/public-api.lock # vortex-array/src/aggregate_fn/fns/mod.rs

Combined

0515378

Signed-off-by: blaginin <github@blaginin.me>

new Mean

ce2e52c

Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: blaginin <github@blaginin.me>

blaginin commented Apr 8, 2026

View reviewed changes

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

blaginin added 3 commits April 8, 2026 16:23

decimals

0e56b5c

Signed-off-by: blaginin <github@blaginin.me>

block serialization

1e0d743

Signed-off-by: blaginin <github@blaginin.me>

fmt

f847221

Signed-off-by: blaginin <github@blaginin.me>

joseph-isaacs reviewed Apr 9, 2026

View reviewed changes

blaginin marked this pull request as ready for review April 9, 2026 14:25

blaginin and others added 5 commits April 9, 2026 15:26

Merge branch 'develop' into db/count-and-mean

be4fa68

Merge branch 'develop' into db/count-and-mean

e5a99ea

finalize_scalar is now mandatory

7f179cc

Signed-off-by: blaginin <github@blaginin.me>

fmt

738e302

Signed-off-by: blaginin <github@blaginin.me>

lock file update

3c426f0

Signed-off-by: blaginin <github@blaginin.me>

blaginin enabled auto-merge (squash) April 20, 2026 18:06

blaginin requested a review from joseph-isaacs April 20, 2026 18:06

joseph-isaacs reviewed Apr 21, 2026

View reviewed changes

blaginin added 4 commits April 21, 2026 12:12

rename to avg, block decimals

a9be15a

Signed-off-by: blaginin <github@blaginin.me>

lockfile

44f4b65

Signed-off-by: blaginin <github@blaginin.me>

lockfile again

348f88c

Signed-off-by: blaginin <github@blaginin.me>

avg_all_null_returns_null

75e8c5c

Signed-off-by: blaginin <github@blaginin.me>

mean is back

55c0d04

Signed-off-by: blaginin <github@blaginin.me>

joseph-isaacs reviewed Apr 27, 2026

View reviewed changes

joseph-isaacs disabled auto-merge April 27, 2026 13:23

joseph-isaacs approved these changes Apr 27, 2026

View reviewed changes

blaginin added 2 commits April 27, 2026 18:02

Merge branch 'develop' into db/count-and-mean

db815bf

move partial_struct_dtype

4401d73

Signed-off-by: blaginin <github@blaginin.me>

blaginin enabled auto-merge (squash) April 27, 2026 17:25

blaginin merged commit 1689d7a into develop Apr 27, 2026
59 checks passed

blaginin deleted the db/count-and-mean branch April 27, 2026 17:30

Conversation

blaginin commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joseph-isaacs left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joseph-isaacs Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joseph-isaacs Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joseph-isaacs Apr 2, 2026 • edited by blaginin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blaginin Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codspeed-hq Bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will improve performance by 27.62%

Performance Changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joseph-isaacs commented Apr 23, 2026

Uh oh!

blaginin commented Apr 23, 2026

Uh oh!

joseph-isaacs commented Apr 24, 2026

Uh oh!

joseph-isaacs commented Apr 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blaginin commented Mar 29, 2026 •

edited

Loading

joseph-isaacs left a comment •

edited

Loading

joseph-isaacs Mar 30, 2026 •

edited

Loading

joseph-isaacs Mar 30, 2026 •

edited

Loading

joseph-isaacs Apr 2, 2026 •

edited by blaginin

Loading

blaginin Apr 2, 2026 •

edited

Loading

codspeed-hq Bot commented Apr 2, 2026 •

edited

Loading