Repository navigation
Simplify aggregate table for ordered cases #23318
Description
Is your feature request related to a problem or challenge?
Part of #22710
Follow up to #23309
Describe the solution you'd like
For ordered aggregation, we can likely simplify the implementation in a similar way to #23309.
The ordered aggregation case can be split into two variants:
- Fully ordered input: the input is ordered by
(a, b), and the query groups by(a, b) - Partially ordered input: the input is ordered by
a, and the query groups by(a, b)
For the partially ordered variant, we should still use regular hash aggregation, with the early-emit optimization. For the fully ordered variant, there may be a deeper optimization that avoids the hash table entirely.
This refactor is probably easier after we first add the fast path for the fully ordered case. 🤔
Describe alternatives you've considered
No response
Additional context
No response
Activity
hhhizzz commentedon Jul 6, 2026
I found a TPC-DS SF10 performance regression on current
mainthat appears related to the partially ordered aggregation path.Environment:
6a0e76ed692afaafd616e0d98d51bef63c1ff02dResult summary:
4d898e93f07b335b771231090.78 ms6a0e76ed69d8d980bdcf32876.21 msCurrent
mainis about+1785 ms/roundslower. The main stable contributors are q39 and q78.Per-query comparison:
1274 ms1962 ms688 ms655 ms1121 ms466 msBoth q39 and q78 contain
AggregateExec ... ordering_mode=PartiallySorted(...).The likely cause is that after #23181, these partially sorted aggregate cases are routed through the new ordered partial/final aggregate streams. For these TPC-DS queries, that path appears slower than the previous regular hash aggregation path with early emit. Debug metrics also suggest the extra time is concentrated in the aggregate emitting path.
This seems consistent with the point in this issue that partially ordered input should likely still use regular hash aggregation with the early-emit optimization, while fully ordered input may need a separate optimized path.
2010YOUY01 commentedon Jul 7, 2026
I suspect the root cause is something similar to #23251
For now we might want to do some patches without significant structural changes, eventually they should be reworked after #7065