zhli1142015 commented on code in PR #5754:
URL: https://github.com/apache/incubator-gluten/pull/5754#discussion_r1618068576
##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -241,21 +226,21 @@ abstract class HashAggregateExecTransformer(
}
aggregateFunction match {
- case hllAdapter: HLLAdapter =>
+ case _ if aggregateFunction.aggBufferAttributes.size > 1 =>
+ generateMergeCompanionNode()
+ case _ =>
aggregateMode match {
- case Partial =>
- // For Partial mode output type is binary.
+ case Partial | PartialMerge =>
val partialNode = ExpressionBuilder.makeAggregateFunction(
VeloxAggregateFunctionsBuilder.create(args, aggregateFunction,
aggregateMode),
childrenNodeList,
modeKeyWord,
ConverterUtils.getTypeNode(
- hllAdapter.inputAggBufferAttributes.head.dataType,
- hllAdapter.inputAggBufferAttributes.head.nullable)
+ aggregateFunction.inputAggBufferAttributes.head.dataType,
Review Comment:
I'm little confused about this:
> @zhli1142015 Gluten searches agg functions in Velox based on the raw input
and final output signatures of the agg function. I think using
`inputAggBufferAttributes.head` to construct the `aggFuncNode` for partial agg
is inaccurate. It would be more appropriate to follow the original `dataType`.
Do you mean here I still need to use `dataType` for partial mode even
`dataType` and `inputAggBufferAttributes.head` are not same?
--
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]
