2010YOUY01 commented on code in PR #23181:
URL: https://github.com/apache/datafusion/pull/23181#discussion_r3510258177
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -1069,6 +1101,16 @@ impl AggregateExec {
&& self.group_by.is_single()
}
+ fn should_use_ordered_final_aggregate_stream(&self) -> bool {
+ matches!(
+ self.mode,
+ AggregateMode::Final | AggregateMode::FinalPartitioned
Review Comment:
Yes, the existing behavior is, if there is memory limit, ordered aggregation
case fallback to regular hash aggregation path (no ordering optimization) for
the spilling.
This is intended for partially ordered case, since it might OOM, and we
haven't implement spilling for that.
For fully ordered cases, this is not necessary, since it ensures always use
bounded memory, but there is no existing testing coverage for that, probably in
a follow-up PR, we can enable ordered fast path even there is a memory limit.
Patch applied in
[c2ae7dc](https://github.com/apache/datafusion/pull/23181/commits/c2ae7dcd7f081edd4637ab9c94c9fb9dfef195df)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
