mustafasrepo commented on code in PR #8662:
URL: https://github.com/apache/arrow-datafusion/pull/8662#discussion_r1437005035
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -1014,17 +942,15 @@ fn aggregate_expressions(
.iter()
.map(|agg| {
let mut result = agg.expressions().clone();
- // In partial mode, append ordering requirements to
expressions' results.
- // Ordering requirements are used by subsequent executors to
satisfy the required
- // ordering for
`AggregateMode::FinalPartitioned`/`AggregateMode::Final` modes.
- if matches!(mode, AggregateMode::Partial) {
- if let Some(ordering_req) = agg.order_bys() {
- let ordering_exprs = ordering_req
- .iter()
- .map(|item| item.expr.clone())
- .collect::<Vec<_>>();
- result.extend(ordering_exprs);
- }
+ // Append ordering requirements to expressions' results.
+ // This way order sensitive aggregators can satisfy requirement
+ // themselves.
+ if let Some(ordering_req) = agg.order_bys() {
Review Comment:
Since aggregators themselves handle ordering. We append ordering expression
values to the field also for all modes.
--
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]
