apache / apache/datafusion

Is there a way for accumulators to know when they are used in windows?

Open
#20,087 1 comment 0 reactions 0 assignees 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?

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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.