hvanhovell commented on a change in pull request #32786:
URL: https://github.com/apache/spark/pull/32786#discussion_r647338534
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/AggregatingAccumulator.scala
##########
@@ -205,6 +213,15 @@ class AggregatingAccumulator private(
buffer = other.buffer
joinedRow = other.joinedRow
}
+
+ private[execution] def addState(other: AggregatingAccumulator): Unit = {
+ if (isZero) {
Review comment:
IMO this method does not really add value, it will just be confusing for
whomever comes after this and tries to understand why it is needed. While you
are right that the documentation for the accumulator says that merge for an
accumulator is not intended to be called on the executor, you are allowed to
widen the scope of a method in a subclass as it won't violate the contract.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
