Is there a way for accumulators to know when they are used in windows?
- 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?
Accumulators (of aggregate functions) can be used in window functions, even if the aggregate function doesn't support sliding accumulators; see this issue:
- https://github.com/apache/datafusion/issues/19612
This forces accumulators to _not_ consume their internal state during `evaluate()`, which can lead to clones. For example, see `StringAgg`:
https://github.com/apache/datafusion/blob/a02e6836e7d449008eef592b177ea282595e027a/datafusion/functions-aggregate/src/string_agg.rs#L386-L394
Previously we'd be able to `std::mem::take` the string, but because of the above issue we need to clone it for correctness.
### Describe the solution you'd like
Can accumulators know when they are used in windows, which means they'd have to preserve their internal state, vs being used in regular aggregation where they can consume their internal state for potential optimization?
### Describe alternatives you've considered
Maybe not worth doing if the clone would have minimal performance impact?
Also maybe not an issue if we proceed with this issue:
- https://github.com/apache/datafusion/issues/5948
### Additional context
_No response_
Contributor guide
Research direction
Start with datafusion/functions-aggregate/src/string_agg.rs around the linked StringAgg implementation, then trace how accumulators are invoked by regular aggregates and window functions, including issue 19612. Done means agreeing on and implementing a way to distinguish those uses while preserving correctness and avoiding unnecessary state cloning; issue 5948 may affect the decision.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- database
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100