Github user rednaxelafx commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19082#discussion_r179367236
  
    --- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala
 ---
    @@ -257,6 +259,78 @@ case class HashAggregateExec(
          """.stripMargin
       }
     
    +  // Extracts all the input variable references for a given `aggExpr`. 
This result will be used
    +  // to split aggregation into small functions.
    +  private def getInputVariableReferences(
    +      context: CodegenContext,
    +      aggregateExpression: Expression,
    +      subExprs: Map[Expression, SubExprEliminationState]): Set[(String, 
String)] = {
    +    // `argSet` collects all the pairs of variable names and their types, 
the first in the pair is
    +    // a type name and the second is a variable name.
    +    val argSet = mutable.Set[(String, String)]()
    +    val stack = mutable.Stack[Expression](aggregateExpression)
    +    while (stack.nonEmpty) {
    +      stack.pop() match {
    +        case e if subExprs.contains(e) =>
    +          val exprCode = subExprs(e)
    +          if (CodegenContext.isJavaIdentifier(exprCode.value)) {
    --- End diff --
    
    Once we have @viirya 's https://github.com/apache/spark/pull/20043 merged 
we won't need the ugly `CodegenContext.isJavaIdentifier` hack any more >_<|||


---

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

Reply via email to