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

    https://github.com/apache/spark/pull/19082#discussion_r136455832
  
    --- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala
 ---
    @@ -244,6 +246,92 @@ case class HashAggregateExec(
     
       protected override val shouldStopRequired = false
     
    +  // We assume a prefix has lower cases and a name has camel cases
    +  private val variableName = "^[a-z]+_[a-zA-Z]+[0-9]*".r
    +
    +  // Returns true if a given name id belongs to this `CodegenContext`
    +  private def isVariable(nameId: String): Boolean = nameId match {
    +    case variableName() => true
    +    case _ => false
    +  }
    +
    +  // Extracts all the outer references for a given `aggExpr`. This result 
will be used to split
    +  // aggregation into small functions.
    +  private def getOuterReferences(
    +      ctx: CodegenContext,
    +      aggExpr: Expression,
    +      subExprs: Map[Expression, SubExprEliminationState]): Set[(String, 
String)] = {
    +    val stack = mutable.Stack[Expression](aggExpr)
    +    val argSet = mutable.Set[(String, String)]()
    +    val addIfNotLiteral = (value: String, tpe: String) => {
    +      if (isVariable(value)) {
    +        argSet += ((tpe, value))
    +      }
    +    }
    +    while (stack.nonEmpty) {
    +      stack.pop() match {
    +        case e if subExprs.contains(e) =>
    +          val exprCode = subExprs(e)
    +          addIfNotLiteral(exprCode.value, ctx.javaType(e.dataType))
    +          addIfNotLiteral(exprCode.isNull, "boolean")
    +          // Since the children possibly has common expressions, we push 
them here
    +          stack.pushAll(e.children)
    +        case ref: BoundReference
    +            if ctx.currentVars != null && ctx.currentVars(ref.ordinal) != 
null =>
    +          val argVal = ctx.currentVars(ref.ordinal).value
    +          addIfNotLiteral(argVal, ctx.javaType(ref.dataType))
    +          addIfNotLiteral(ctx.currentVars(ref.ordinal).isNull, "boolean")
    +        case _: BoundReference =>
    +          argSet += (("InternalRow", ctx.INPUT_ROW))
    +        case e =>
    +          stack.pushAll(e.children)
    +      }
    +    }
    +
    +    argSet.toSet
    +  }
    +
    +  // Splits the aggregation into small functions because the HotSpot does 
not compile
    +  // too long functions.
    +  private def splitAggregateExpressions(
    +      ctx: CodegenContext,
    +      aggExprs: Seq[Expression],
    +      evalAndUpdateCodes: Seq[String],
    +      subExprs: Map[Expression, SubExprEliminationState],
    +      otherArgs: Seq[(String, String)] = Seq.empty): Seq[String] = {
    +    aggExprs.zipWithIndex.map { case (aggExpr, i) =>
    +      // The maximum number of parameters in Java methods is 255, so this 
method gives up splitting
    +      // the code if the number goes over the limit.
    +      // You can find more information about the limit in the JVM 
specification:
    +      //   - The number of method parameters is limited to 255 by the 
definition of a method
    +      //     descriptor, where the limit includes one unit for this in the 
case of instance
    +      //     or interface method invocations.
    +      val args = (getOuterReferences(ctx, aggExpr, subExprs) ++ 
otherArgs).toSeq
    +
    +      // This is for testing/benchmarking only
    +      val maxParamNumInJavaMethod =
    +          
sqlContext.getConf("spark.sql.codegen.aggregate.maxParamNumInJavaMethod", null) 
match {
    --- End diff --
    
    Can we add a check code if a user specify a value that is more than 255?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Reply via email to