You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mgaido91 <gi...@git.apache.org> on 2018/07/02 21:56:09 UTC

[GitHub] spark pull request #21599: [SPARK-24598][SQL] Overflow on arithmetic operati...

Github user mgaido91 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21599#discussion_r199632638
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala ---
    @@ -128,17 +128,31 @@ abstract class BinaryArithmetic extends BinaryOperator with NullIntolerant {
       def calendarIntervalMethod: String =
         sys.error("BinaryArithmetics must override either calendarIntervalMethod or genCode")
     
    +  def checkOverflowCode(result: String, op1: String, op2: String): String =
    +    sys.error("BinaryArithmetics must override either checkOverflowCode or genCode")
    +
       override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = dataType match {
         case _: DecimalType =>
           defineCodeGen(ctx, ev, (eval1, eval2) => s"$eval1.$decimalMethod($eval2)")
         case CalendarIntervalType =>
           defineCodeGen(ctx, ev, (eval1, eval2) => s"$eval1.$calendarIntervalMethod($eval2)")
    +    // In the following cases, overflow can happen, so we need to check the result is valid.
    +    // Otherwise we throw an ArithmeticException
    --- End diff --
    
    @gatorsmile @hvanhovell do you have time to check this and give your opinion here? Thanks.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org