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

    https://github.com/apache/spark/pull/10855#discussion_r51310284
  
    --- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/TungstenAggregate.scala
 ---
    @@ -114,22 +116,38 @@ case class TungstenAggregate(
         }
       }
     
    +  // all the mode of aggregate expressions
    +  private val modes = aggregateExpressions.map(_.mode).distinct
    +
       override def supportCodegen: Boolean = {
    -    groupingExpressions.isEmpty &&
    -      // ImperativeAggregate is not supported right now
    -      
!aggregateExpressions.exists(_.aggregateFunction.isInstanceOf[ImperativeAggregate])
    +    // ImperativeAggregate is not supported right now
    +    
!aggregateExpressions.exists(_.aggregateFunction.isInstanceOf[ImperativeAggregate])
       }
     
    -  // The variables used as aggregation buffer
    -  private var bufVars: Seq[ExprCode] = _
    -
    -  private val modes = aggregateExpressions.map(_.mode).distinct
    -
       override def upstream(): RDD[InternalRow] = {
         child.asInstanceOf[CodegenSupport].upstream()
       }
     
       protected override def doProduce(ctx: CodegenContext): String = {
    +    if (groupingExpressions.isEmpty) {
    +      doProduceWithoutKeys(ctx)
    +    } else {
    +      doProduceWithKeys(ctx)
    +    }
    +  }
    +
    +  override def doConsume(ctx: CodegenContext, input: Seq[ExprCode]): 
String = {
    +    if (groupingExpressions.isEmpty) {
    +      doConsumeWithoutKeys(ctx, input)
    +    } else {
    +      doConsumeWithKeys(ctx, input)
    +    }
    +  }
    +
    +  // The variables used as aggregation buffer
    +  private var bufVars: Seq[ExprCode] = _
    --- End diff --
    
    I find it weird you've split this up this way. Can you comment that this is 
only used when its grouping with no agg?
    
    Why do you manage this in side the TungstenAggregation class but create the 
hashmap so differently? They seem logically serving the same purpose.


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to