alamb commented on code in PR #24486:
URL: https://github.com/apache/datafusion/pull/24486#discussion_r3847042915
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -4488,10 +4460,10 @@ mod tests {
Ok(())
}
- /// Spilling behavior is not implemented for partial-reduce stream yet, so
fall
- /// back to the existing `GroupedHashAggregateStream`
+ /// Partial-reduce hash aggregation returns `ResourcesExhausted` when its
Review Comment:
As I mentioned elsewhere, I think that since PartialReduce mode is just
converting one partial state to another partial intermediate state as an
optimization before sending over the network, it would actually be better here
to emit any gathered state on OOM pressure and start re-aggregating (or just
start copying the input directly to the output) rather than error here
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -4488,10 +4460,10 @@ mod tests {
Ok(())
}
- /// Spilling behavior is not implemented for partial-reduce stream yet, so
fall
- /// back to the existing `GroupedHashAggregateStream`
+ /// Partial-reduce hash aggregation returns `ResourcesExhausted` when its
Review Comment:
I made a PR to validate this expectation
- https://github.com/apache/datafusion/pull/24640
--
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]
