Uh oh!
There was an error while loading. Please reload this page.
feat: add AggregateMode::PartialReduce for tree-reduce aggregation - #20019
Conversation
Add is_last_stage() method to AggregateMode (complementing the existing is_first_stage()) and refactor match statements throughout the aggregate execution code to use these helpers instead of explicit variant listing. This makes the code robust to adding new AggregateMode variants -- instead of updating every match arm, new variants only need to be added to is_first_stage() and is_last_stage(). Refactored sites: - create_schema: uses is_last_stage() - aggregate_expressions: uses is_first_stage() - finalize_aggregation: uses is_last_stage() - no_grouping.rs filter expressions: uses is_first_stage() - no_grouping.rs update vs merge dispatch: uses is_first_stage() - row_hash.rs filter expressions: uses is_first_stage() - row_hash.rs update vs merge dispatch: uses is_first_stage() - row_hash.rs create_batch_from_accumulators: uses is_last_stage()
0727a56 to
0ced71aComparenjsmith
commented
Jan 27, 2026
fixed the clippy/cargo fmt failures from CI |
adriangb
commented
Jan 27, 2026
Hi @njsmith, nice to see you over here :) |
0ced71a to
a854145CompareAdd a new PartialReduce variant to AggregateMode that takes partially- computed aggregate state as input and produces further-reduced (but still partial) aggregate state as output. This fills the missing cell in the input/output matrix: | Input: raw data | Input: state Output: values | Single/SP | Final/FP Output: state | Partial | PartialReduce <-- new This enables tree-reduce distributed aggregation patterns where multiple rounds of partial reduction happen before the final aggregation step. Changes: - Add PartialReduce variant to AggregateMode enum - Update is_first_stage() (false) and is_last_stage() (false) helpers - Update required_input_distribution() (UnspecifiedDistribution) - Add proto serialization support (PARTIAL_REDUCE = 5) - Add unit tests for mode helpers - Add integration tests for Partial->PartialReduce->Final pipeline - Add integration test for chained tree-reduce pattern
a854145 to
97b3d15Comparenjsmith
commented
Jan 27, 2026
🤦 now hopefully fixed CI (forgot that |
alamb
commented
Jan 27, 2026
FYI @gabotechs -- this may be of interest for the datafusion-distributed |
adriangb
left a comment
There was a problem hiding this comment.
This is a very clean way to organize the change, I was immediately able to understand the goal and implementation.
I think it's worth getting another set of eyes (or I can dig a bit deeper into the implications, I'm out of time today) but this looks great from my perspective!
| /// Checks whether this aggregation step produces final output values | ||
| /// (as opposed to intermediate accumulator state). | ||
| pub fn is_last_stage(&self) -> bool { |
There was a problem hiding this comment.
I think it could be helpful to have your table and the description of "tells us in which row/column" as docstrings on is_last_stage and is_first_stage.
Would it also be easier to understand if this was called output_final_results() or output_mode() -> OutputMode where enum OutputMode { Partial, Final }?
There was a problem hiding this comment.
Ping on this before we move to merge
There was a problem hiding this comment.
Sure happy to make the change, just didn't want to make noisy changes until people had chance to look things over
| /// ```text | ||
| /// Final | ||
| /// / \ | ||
| /// PartialReduce PartialReduce | ||
| /// / \ / \ | ||
| /// Partial Partial Partial Partial | ||
| /// ``` | ||
| PartialReduce, |
There was a problem hiding this comment.
This is looking really good 👍 I don't think a partial reduce implementation can get cleaner than this.
One question: do you think this could be beneficial for single-node DataFusion plans at some point? I can imagine how this is useful for distributed engine, but do you have any thoughts about single-node usage?
There was a problem hiding this comment.
Regarding distributed engines, this produces a good alternative for shuffling, and I imagine that for systems that rely on materializing shuffle results, this can be a pretty good alternative. However, I do also imagine that in-memory streaming-based distributed systems like Trino or https://github.com/datafusion-contrib/datafusion-distributed might not benefit as much from this pattern because:
- Shuffling is as cheap as it can get, as intermediate results are zero-copied over the network between workers
- Shuffling allows running the final aggregation step concurrently in multiple workers, while a tree-reduction approach limits the final aggregation to one worker
There was a problem hiding this comment.
I get the impression that https://github.com/apache/datafusion-ballista might be a good candidate for this aggregation pattern, but I'd bet that it might not yield solid performance improvements in https://github.com/datafusion-contrib/datafusion-distributed
There was a problem hiding this comment.
If you have heterogenous links between your compute (e.g. a bunch of machines, and each machine contains a bunch of cores, where same-machine cores have much higher bandwidth/lower latency between them than the cross-machine links), then this is still useful in the shuffle world I think?
Each node partitions local data into N threads -> Partial each partition -> within-machine shuffle -> PartialReduce -> cross-machine shuffle -> Final gives a 1/N-reduction in network transfer, right? (compared to shuffling each thread's Partial result directly)
Whether this would be worth the extra complexity depends on a bunch of deployment-specific constants of course.
There was a problem hiding this comment.
🤔 Yes, I think you are right there, I can imagine how applying an extra partial reduce right before shuffling over the wire can give solid improvements.
There was a problem hiding this comment.
I think this Partial Reduce mode makes sense
I find our other modes somewhat confusing (I am not sure why we have them and suspect we could make this a much cleaner 2 mode switch like "initial", "partial", and "final"
There was a problem hiding this comment.
@gabotechs@njsmith the benefits are real in aggregations where the cardinality of the aggregation key is large. That is why all the Aggregation frameworks that I am aware of (spark, hadoop, algebird etc) have a separate Combiner itermediate type/function for merging partial aggregates.
Both hadoop and spark do this by default and it's called map side combine. Big +1 for the work on this PR!
PS: the most elegant definition of generic aggregation that I know of is in the Algebird scala framework (developed originally at twitter ~15y ago): https://github.com/twitter/algebird/blob/develop/algebird-core/src/main/scala/com/twitter/algebird/Aggregator.scala#L367-L390
gabotechs
commented
Jan 29, 2026
IMO this is pretty much ready to do, I'll let @adriangb finish his review. Thanks for this addition! |
alamb
left a comment
There was a problem hiding this comment.
Looks good to me too -thanks @njsmith and @gabotechs
It would be nice if we could figure out how to clean up the mess of aggregate modes (Final and FinalPartitioned and Single seem like they should be merged) but that is for some other day perhaps
| /// ```text | ||
| /// Final | ||
| /// / \ | ||
| /// PartialReduce PartialReduce | ||
| /// / \ / \ | ||
| /// Partial Partial Partial Partial | ||
| /// ``` | ||
| PartialReduce, |
…_mode
Replace the boolean helpers with typed enums:
- is_first_stage() -> input_mode() -> AggregateInputMode { Raw, Partial }
- is_last_stage() -> output_mode() -> AggregateOutputMode { Partial, Final }
Add a table to AggregateMode docs showing how each variant maps to
its input/output modes.
This makes the semantics more explicit and self-documenting.njsmith
commented
Jan 30, 2026
Added the table to the docs and replaced |
adriangb
commented
Jan 30, 2026
Just to get a second opinion, @gabotechs do you agree this naming is clearer? |
gabotechs
commented
Jan 30, 2026
Yes! Not like the previous one was bad, but this one is slightly better |
Uh oh!
There was an error while loading. Please reload this page.
gabotechs
commented
Jan 30, 2026
…pache#20019) DataFusion's current `AggregateMode` enum has four variants covering three of the four cells in the input/output matrix: | | Input: raw data | Input: partial state | | - | - | - | | Output: final values | `Single` / `SinglePartitioned` | `Final` / `FinalPartitioned` | | Output: partial state | `Partial` | ??? | This PR adds `AggregateMode::PartialReduce` to fill in the missing cell: it takes partially-reduced values as input, and reduces them further, but without finalizing. This is useful because it's the key component needed to implement distributed tree-reduction (as seen in e.g. the Scuba or Honeycomb papers): a set of worker nodes each perform multithreaded `Partial` aggregations, feed those into a `PartialReduce` to reduce all of this node's values into a single row, and then a head node collects the outputs from all nodes' `PartialReduce` to feed into a `Final` reduction. PR can be reviewed commit by commit: first commit is pure refactor/simplification; most places we were matching on `AggregateMode` we were actually just trying to either check which row of the above table we were in, or else which column. So now we have `is_first_stage` (tells you which column) and `is_last_stage` (tells you which row) and we use them everywhere. Second commit adds `PartialReduce`, and is pretty small because `is_first_stage`/`is_last_stage` do most of the heavy lifting. It also adds a test demonstrating a minimal Partial -> PartialReduce -> Final tree-reduction.
…pache#20019) DataFusion's current `AggregateMode` enum has four variants covering three of the four cells in the input/output matrix: | | Input: raw data | Input: partial state | | - | - | - | | Output: final values | `Single` / `SinglePartitioned` | `Final` / `FinalPartitioned` | | Output: partial state | `Partial` | ??? | This PR adds `AggregateMode::PartialReduce` to fill in the missing cell: it takes partially-reduced values as input, and reduces them further, but without finalizing. This is useful because it's the key component needed to implement distributed tree-reduction (as seen in e.g. the Scuba or Honeycomb papers): a set of worker nodes each perform multithreaded `Partial` aggregations, feed those into a `PartialReduce` to reduce all of this node's values into a single row, and then a head node collects the outputs from all nodes' `PartialReduce` to feed into a `Final` reduction. PR can be reviewed commit by commit: first commit is pure refactor/simplification; most places we were matching on `AggregateMode` we were actually just trying to either check which row of the above table we were in, or else which column. So now we have `is_first_stage` (tells you which column) and `is_last_stage` (tells you which row) and we use them everywhere. Second commit adds `PartialReduce`, and is pretty small because `is_first_stage`/`is_last_stage` do most of the heavy lifting. It also adds a test demonstrating a minimal Partial -> PartialReduce -> Final tree-reduction.
DataFusion's current
AggregateModeenum has four variants covering three of the four cells in the input/output matrix:Single/SinglePartitionedFinal/FinalPartitionedPartialThis PR adds
AggregateMode::PartialReduceto fill in the missing cell: it takes partially-reduced values as input, and reduces them further, but without finalizing.This is useful because it's the key component needed to implement distributed tree-reduction (as seen in e.g. the Scuba or Honeycomb papers): a set of worker nodes each perform multithreaded
Partialaggregations, feed those into aPartialReduceto reduce all of this node's values into a single row, and then a head node collects the outputs from all nodes'PartialReduceto feed into aFinalreduction.PR can be reviewed commit by commit: first commit is pure refactor/simplification; most places we were matching on
AggregateModewe were actually just trying to either check which row of the above table we were in, or else which column. So now we haveis_first_stage(tells you which column) andis_last_stage(tells you which row) and we use them everywhere.Second commit adds
PartialReduce, and is pretty small becauseis_first_stage/is_last_stagedo most of the heavy lifting. It also adds a test demonstrating a minimal Partial -> PartialReduce -> Final tree-reduction.