cloud-fan commented on a change in pull request #26899: [SPARK-28332][SQL]
Reserve init value -1 only when do min max statistics in SQLMetrics
URL: https://github.com/apache/spark/pull/26899#discussion_r358868415
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/metric/SQLMetrics.scala
##########
@@ -50,22 +50,27 @@ class SQLMetric(val metricType: String, initValue: Long =
0L) extends Accumulato
override def reset(): Unit = _value = _zeroValue
override def merge(other: AccumulatorV2[Long, Long]): Unit = other match {
- case o: SQLMetric => _value += o.value
+ case o: SQLMetric => add(o.value)
case _ => throw new UnsupportedOperationException(
s"Cannot merge ${this.getClass.getName} with ${other.getClass.getName}")
}
override def isZero(): Boolean = _value == _zeroValue
- override def add(v: Long): Unit = _value += v
+ override def add(v: Long): Unit = {
+ if (_value < 0) _value = 0
+ if (v > 0) {
Review comment:
This if check only makes sense in `merge`. I'd suggest doing this check in
`merge`, so that `add` don't need to do extra check which may hurt perf.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
