andygrove commented on code in PR #2457:
URL: https://github.com/apache/arrow-datafusion/pull/2457#discussion_r866949413
##########
datafusion/core/src/sql/planner.rs:
##########
@@ -1144,30 +1144,45 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
group_by_exprs: Vec<Expr>,
aggr_exprs: Vec<Expr>,
) -> Result<(LogicalPlan, Vec<Expr>, Option<Expr>)> {
+ // create the aggregate plan
+ let plan = LogicalPlanBuilder::from(input.clone())
+ .aggregate(group_by_exprs.clone(), aggr_exprs.clone())?
+ .build()?;
+
+ // in this next section of code we are re-writing the projection to
refer to columns
+ // output by the aggregate plan. For example, if the projection
contains the expression
+ // `SUM(a)` then we replace that with a reference to a column
`#SUM(a)` produced by
+ // the aggregate plan.
+
+ // combine the original grouping and aggregate expressions into one
list (note that
+ // we do not add the "having" expression since that is not part of the
projection)
let aggr_projection_exprs = group_by_exprs
.iter()
.chain(aggr_exprs.iter())
.cloned()
.collect::<Vec<Expr>>();
- let plan = LogicalPlanBuilder::from(input.clone())
- .aggregate(group_by_exprs, aggr_exprs)?
- .build()?;
+ // now attempt to resolve columns and replace with fully-qualified
columns
+ let aggr_projection_exprs = aggr_projection_exprs
+ .iter()
+ .map(|expr| resolve_columns(expr, &input))
Review Comment:
This is the fix .. we were comparing qualified and unqualified names before
this, leading to failures in some cases
--
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]
