cloud-fan commented on a change in pull request #27627:
URL: https://github.com/apache/spark/pull/27627#discussion_r433125761
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Sum.scala
##########
@@ -62,38 +62,74 @@ case class Sum(child: Expression) extends
DeclarativeAggregate with ImplicitCast
private lazy val sum = AttributeReference("sum", sumDataType)()
+ private lazy val isEmpty = AttributeReference("isEmpty", BooleanType,
nullable = false)()
+
private lazy val zero = Literal.default(sumDataType)
- override lazy val aggBufferAttributes = sum :: Nil
+ override lazy val aggBufferAttributes = resultType match {
+ case _: DecimalType => sum :: isEmpty :: Nil
+ case _ => sum :: Nil
+ }
- override lazy val initialValues: Seq[Expression] = Seq(
- /* sum = */ Literal.create(null, sumDataType)
- )
+ override lazy val initialValues: Seq[Expression] = resultType match {
+ case _: DecimalType => Seq(Literal(null, resultType), Literal(true,
BooleanType))
+ case _ => Seq(Literal(null, resultType))
+ }
override lazy val updateExpressions: Seq[Expression] = {
if (child.nullable) {
- Seq(
- /* sum = */
- coalesce(coalesce(sum, zero) + child.cast(sumDataType), sum)
- )
+ val updateSumExpr = coalesce(coalesce(sum, zero) +
child.cast(sumDataType), sum)
+ resultType match {
+ case _: DecimalType =>
+ Seq(updateSumExpr, isEmpty && child.isNull)
+ case _ => Seq(updateSumExpr)
+ }
} else {
- Seq(
- /* sum = */
- coalesce(sum, zero) + child.cast(sumDataType)
- )
+ val updateSumExpr = coalesce(sum, zero) + child.cast(sumDataType)
+ resultType match {
+ case _: DecimalType =>
+ Seq(updateSumExpr, Literal(false, BooleanType))
+ case _ => Seq(updateSumExpr)
+ }
}
}
+ /**
+ * For decimal type:
+ * If isEmpty is false and if sum is null, then it means we have an overflow.
+ *
+ * update of the sum is as follows:
+ * Check if either portion of the left.sum or right.sum has overflowed
+ * If it has, then the sum value will remain null.
+ * If it did not have overflow, then add the sum.left and sum.right and
check for overflow.
Review comment:
can we update the comment? we don't check overflow here now.
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
