You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add integration regression test for terminal hash aggregate drain #24849
#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.
Activity
test: add regression coverage for terminal hash aggregate drain (apac…