rui-mo commented on code in PR #13441:
URL: https://github.com/apache/gluten/pull/13441#discussion_r4383489461


##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -504,7 +508,13 @@ abstract class HashAggregateExecTransformer(
         val aggregateFunc = aggExpr.aggregateFunction
         val childrenNodes = aggExpr.mode match {
           case Partial | Complete =>
-            aggregateFunc.children.toList.map(toExpressionNode)
+            VeloxIntermediateData.getRawInputs(aggregateFunc).toList.map {
+              case literal: Literal if rowConstructed =>

Review Comment:
   Could you please clarify why we need to check rowConstructed for the Partial 
and Complete stages? I assume `rowConstructed` applies only to the PartialMerge 
and Final stages, where the intermediate inputs are combined into a single 
column using Velox’s row_constructor function.
   
   ```
     // Return whether the outputs partial aggregation should be combined for 
Bolt computing.
     // When the partial outputs are multiple-column, row construct is needed.
     private def rowConstructNeeded(): Boolean = aggregateExpressions.exists {
       case AggregateExpression(aggFunc, PartialMerge | Final, _, _, _) =>
         aggFunc.inputAggBufferAttributes.size > 1
       case _ => false
     }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to