uros-db commented on code in PR #46404:
URL: https://github.com/apache/spark/pull/46404#discussion_r1598038412
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Mode.scala:
##########
@@ -70,10 +76,30 @@ case class Mode(
buffer
}
- override def eval(buffer: OpenHashMap[AnyRef, Long]): Any = {
- if (buffer.isEmpty) {
+ override def eval(buff: OpenHashMap[AnyRef, Long]): Any = {
+ if (buff.isEmpty) {
return null
}
+ val buffer = if (isCollatedString(child)) {
+ val modeMap = buff.foldLeft(
Review Comment:
I think this is fine either way, I think we should generally aim to include
`collationId` as part of the expression field (as we've done for other
expressions), and there should be no harm since it's a lazy val
on the other hand, collationId is used only in one place here, but still I'd
keep it as it is
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
