[GitHub] mgaido91 commented on a change in pull request #23701: [SPARK-26741][SQL] Allow using aggregate expressions in ORDER BY clause

[GitBox <gi...@apache.org>]

mgaido91 commented on a change in pull request #23701: [SPARK-26741][SQL] Allow using aggregate expressions in ORDER BY clause
URL: https://github.com/apache/spark/pull/23701#discussion_r253010632
 
 

 ##########
 File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
 ##########
 @@ -1679,7 +1694,31 @@ class Analyzer(
     }
 
     def containsAggregate(condition: Expression): Boolean = {
-      condition.find(_.isInstanceOf[AggregateExpression]).isDefined
+      condition.find(e =>
+        e.isInstanceOf[AggregateExpression] ||
+          e.isInstanceOf[GroupingID] || e.isInstanceOf[Grouping]).isDefined
+    }
+
+    private def pushDownMissingAttrs(
+        missingAttrs: Seq[NamedExpression], plan: LogicalPlan): LogicalPlan = {
+      // Missing attributes can be unresolved attributes or resolved attributes which are not in
+      // the output attributes of the plan.
+      plan match {
+        case p: Project =>
+          // Recursively pushing down expressions on the child of current plan.
+          val newChild = pushDownMissingAttrs(missingAttrs, p.child)
+          Project(p.projectList ++ missingAttrs.map(_.toAttribute), newChild)
+
+        case a @ Aggregate(_, aggExprs, _) =>
+          a.copy(aggregateExpressions = aggExprs ++ missingAttrs)
+
+        // For other operators (eg. Filter), push down recursively
+        case n: UnaryNode =>
 
 Review comment:
   I don't think so. I always saw this case with HAVING which introduces a Filter and Project... I reduce this case to Filter if you see any value in doing that (honestly I don't see)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org