apache / apache/datafusion

Consider deferred copying in `string_agg` `GroupsAccumulator`

Open
#21,156 0 comments 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?

We currently copy every input string in `update_batch` for `StringAggGroupsAccumulator`.

We could instead just bump the Arc refcount on the input batch and keep `` triples. Then assemble the actual results in `evaluate()` (this is similar to #20504 for `array_agg`). This would be quite a bit more complicated than the current approach, but it could be worth it to reduce the amount of data being copied. It will require some bookkeeping to ensure that the right state is reclaimed after a partial emit.

Note that the current string_agg benchmark uses 3 byte strings, so it would underestimate the impact of this optimization.

### Describe the solution you'd like

_No response_

### Describe alternatives you've considered

_No response_

### Additional context

_No response_

Contributor guide

Open the contributing guide

Research direction

Start at StringAggGroupsAccumulator::update_batch and evaluate(), then compare the deferred-copying approach described for array_agg in #20504. Determine how partial emits reclaim retained batches, and validate that results are assembled correctly while reducing input-string copying.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data-engineering
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.