mustafasrepo commented on code in PR #10651:
URL: https://github.com/apache/datafusion/pull/10651#discussion_r1622006126
##########
datafusion/physical-expr-common/src/aggregate/mod.rs:
##########
@@ -292,16 +307,38 @@ impl AggregateExpr for AggregateFunctionExpr {
is_distinct: self.is_distinct,
input_type: &self.input_type,
args_num: self.args.len(),
+ name: &self.name,
};
self.fun.groups_accumulator_supported(args)
}
fn create_groups_accumulator(&self) -> Result<Box<dyn GroupsAccumulator>> {
- self.fun.create_groups_accumulator()
+ let args = AccumulatorArgs {
+ data_type: &self.data_type,
+ schema: &self.schema,
+ ignore_nulls: self.ignore_nulls,
+ sort_exprs: &self.sort_exprs,
+ is_distinct: self.is_distinct,
+ input_type: &self.input_type,
+ args_num: self.args.len(),
+ name: &self.name,
+ };
+ self.fun.create_groups_accumulator(args)
}
fn order_bys(&self) -> Option<&[PhysicalSortExpr]> {
- (!self.ordering_req.is_empty()).then_some(&self.ordering_req)
+ if self.fun.has_ordering_requirements() &&
!self.ordering_req.is_empty() {
+ return Some(&self.ordering_req);
+ }
+ None
Review Comment:
For some data types, order of the operation may effect the results. Hence, I
think, we shouldn't ignore the requirement given by the user. For instance, for
floating point type, `sum` will produce different results for different
permutations of the input.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
