Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21860#discussion_r211994713
  
    --- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala
 ---
    @@ -853,33 +853,43 @@ case class HashAggregateExec(
     
         val updateRowInHashMap: String = {
           if (isFastHashMapEnabled) {
    -        ctx.INPUT_ROW = fastRowBuffer
    -        val boundUpdateExpr = 
updateExpr.map(BindReferences.bindReference(_, inputAttr))
    -        val subExprs = 
ctx.subexpressionEliminationForWholeStageCodegen(boundUpdateExpr)
    -        val effectiveCodes = subExprs.codes.mkString("\n")
    -        val fastRowEvals = 
ctx.withSubExprEliminationExprs(subExprs.states) {
    -          boundUpdateExpr.map(_.genCode(ctx))
    -        }
    -        val updateFastRow = fastRowEvals.zipWithIndex.map { case (ev, i) =>
    -          val dt = updateExpr(i).dataType
    -          CodeGenerator.updateColumn(
    -            fastRowBuffer, dt, i, ev, updateExpr(i).nullable, 
isVectorizedHashMapEnabled)
    -        }
    +        if (isVectorizedHashMapEnabled) {
    +          ctx.INPUT_ROW = fastRowBuffer
    +          val boundUpdateExpr = 
updateExpr.map(BindReferences.bindReference(_, inputAttr))
    +          val subExprs = 
ctx.subexpressionEliminationForWholeStageCodegen(boundUpdateExpr)
    +          val effectiveCodes = subExprs.codes.mkString("\n")
    +          val fastRowEvals = 
ctx.withSubExprEliminationExprs(subExprs.states) {
    +            boundUpdateExpr.map(_.genCode(ctx))
    +          }
    +          val updateFastRow = fastRowEvals.zipWithIndex.map { case (ev, i) 
=>
    +            val dt = updateExpr(i).dataType
    +            CodeGenerator.updateColumn(
    +              fastRowBuffer, dt, i, ev, updateExpr(i).nullable, 
isVectorizedHashMapEnabled)
    +          }
     
    -        // If fast hash map is on, we first generate code to update row in 
fast hash map, if the
    -        // previous loop up hit fast hash map. Otherwise, update row in 
regular hash map.
    -        s"""
    -           |if ($fastRowBuffer != null) {
    -           |  // common sub-expressions
    -           |  $effectiveCodes
    -           |  // evaluate aggregate function
    -           |  ${evaluateVariables(fastRowEvals)}
    -           |  // update fast row
    -           |  ${updateFastRow.mkString("\n").trim}
    -           |} else {
    -           |  $updateRowInRegularHashMap
    -           |}
    -       """.stripMargin
    +          // If fast hash map is on, we first generate code to update row 
in fast hash map, if the
    +          // previous loop up hit fast hash map. Otherwise, update row in 
regular hash map.
    +          s"""
    +             |if ($fastRowBuffer != null) {
    +             |  // common sub-expressions
    +             |  $effectiveCodes
    +             |  // evaluate aggregate function
    +             |  ${evaluateVariables(fastRowEvals)}
    +             |  // update fast row
    +             |  ${updateFastRow.mkString("\n").trim}
    +             |} else {
    +             |  $updateRowInRegularHashMap
    +             |}
    +          """.stripMargin
    +        } else {
    +          s"""
    --- End diff --
    
    please add comment before this line to explain what happened in this branch.


---

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

Reply via email to