Problem
#23182 fixed the performance regression in #23178 by avoiding repeated destructive EmitTo::First calls during terminal hash aggregate output.
The remaining MaterializedAggregateOutput unit test only verifies the slicing helper. It does not exercise AggregateExec, and it will not provide useful coverage when the current workaround is replaced by blocked/chunked state management.
This was a performance/complexity regression rather than a correctness bug. A result-only integration test would pass both the regressed and fixed implementations, while a wall-clock assertion would be unreliable in CI.
Proposed test
Add an integration-style regression test for the unordered Single/Final aggregate output path:
- Run
AggregateExec with more groups than batch_size.
- Use a test
GroupsAccumulator that fails if terminal drain falls back to destructive EmitTo::First.
- Verify that output batches remain bounded and contain the correct results.
- Keep the test independent of
MaterializedAggregateOutput, so direct blocked emission can replace the current workaround.
This can follow the same pattern as the partial aggregate regression test added in #23250.
The actual timing and memory behavior should continue to be validated by the drain benchmark in #24795.
Problem
#23182 fixed the performance regression in #23178 by avoiding repeated destructive
EmitTo::Firstcalls during terminal hash aggregate output.The remaining
MaterializedAggregateOutputunit test only verifies the slicing helper. It does not exerciseAggregateExec, and it will not provide useful coverage when the current workaround is replaced by blocked/chunked state management.This was a performance/complexity regression rather than a correctness bug. A result-only integration test would pass both the regressed and fixed implementations, while a wall-clock assertion would be unreliable in CI.
Proposed test
Add an integration-style regression test for the unordered Single/Final aggregate output path:
AggregateExecwith more groups thanbatch_size.GroupsAccumulatorthat fails if terminal drain falls back to destructiveEmitTo::First.MaterializedAggregateOutput, so direct blocked emission can replace the current workaround.This can follow the same pattern as the partial aggregate regression test added in #23250.
The actual timing and memory behavior should continue to be validated by the drain benchmark in #24795.