avantgardnerio commented on code in PR #3345:
URL: https://github.com/apache/arrow-datafusion/pull/3345#discussion_r961926593
##########
datafusion/optimizer/src/single_distinct_to_groupby.rs:
##########
@@ -159,27 +159,24 @@ fn optimize_children(plan: &LogicalPlan) ->
Result<LogicalPlan> {
from_plan(plan, &expr, &new_inputs)
}
-fn is_single_distinct_agg(plan: &LogicalPlan) -> bool {
+fn is_single_distinct_agg(plan: &LogicalPlan) -> Result<bool> {
match plan {
LogicalPlan::Aggregate(Aggregate { aggr_expr, .. }) => {
let mut fields_set = HashSet::new();
- aggr_expr
- .iter()
- .filter(|expr| {
- let mut is_distinct = false;
- if let Expr::AggregateFunction { distinct, args, .. } =
expr {
- is_distinct = *distinct;
- args.iter().for_each(|expr| {
- fields_set.insert(expr.name().unwrap());
- })
+ let mut count = 0;
+ for expr in aggr_expr {
+ if let Expr::AggregateFunction { distinct, args, .. } = expr {
+ if *distinct {
+ count += 1;
}
- is_distinct
- })
- .count()
- == aggr_expr.len()
- && fields_set.len() == 1
+ for expr in args {
+ fields_set.insert(expr.name()?);
Review Comment:
Higher-order functions look good on interview code, but they could use a
better story regarding error-propagation :/
--
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...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
