apache / apache/datafusion

[EPIC] Further clean up aggregation implementation

Open
#25,006 1 comment 1 reaction 1 assignee Claimed by @2010YOUY01 View on GitHub
enhancement
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Is your feature request related to a problem or challenge?

Follows up #22710

### Describe the solution you'd like

We have completed the migration functionally in #22710, there are some remaining cleanup items to be done after that:

- [ ] Remove the old implementation in `grouped_hash_stream.rs` after 56.0 release (keep 1 more release in case of major bugs, and we can still use config to switch back)
- [ ] https://github.com/apache/datafusion/issues/23113
- [ ] Use builder pattern for constructors: https://github.com/apache/datafusion/pull/23657#discussion_r3626958100
- [x] The existing control flow repeatedly construct `Controlflow::Break`, caused large LoC: https://github.com/apache/datafusion/pull/23657#discussion_r3626964015
- [x] https://github.com/apache/datafusion/pull/23657#discussion_r3626971239
- [ ] https://github.com/apache/datafusion/issues/24317
- [ ] https://github.com/apache/datafusion/issues/24318

For coordination: I plan to do them shortly, but if we're about to ship major features like https://github.com/apache/datafusion/issues/24704, feel free to ping me and I'll pause to avoid conflicts.

### Describe alternatives you've considered

_No response_

### Additional context

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.