[GitHub] spark pull request #21005: [SPARK-23898][SQL] Simplify add & subtract code g...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/21005 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21005: [SPARK-23898][SQL] Simplify add & subtract code g...
Github user maropu commented on a diff in the pull request: https://github.com/apache/spark/pull/21005#discussion_r179971588 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala --- @@ -117,16 +117,22 @@ abstract class BinaryArithmetic extends BinaryOperator with NullIntolerant { override def dataType: DataType = left.dataType - override lazy val resolved = childrenResolved && checkInputDataTypes().isSuccess + override lazy val resolved: Boolean = childrenResolved && checkInputDataTypes().isSuccess /** Name of the function for this expression on a [[Decimal]] type. */ def decimalMethod: String = sys.error("BinaryArithmetics must override either decimalMethod or genCode") + /** Name of the function for this expression on a [[CalendarInterval]] type. */ + def calendarIntervalMethod: String = +sys.error("BinaryArithmetics must override either calendarIntervalMethod or genCode") + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = dataType match { -case dt: DecimalType => +case _: DecimalType => defineCodeGen(ctx, ev, (eval1, eval2) => s"$eval1.$decimalMethod($eval2)") // byte and short are casted into int when add, minus, times or divide +case CalendarIntervalType => + defineCodeGen(ctx, ev, (eval1, eval2) => s"$eval1.$calendarIntervalMethod($eval2)") --- End diff -- How about moving these interval arithmetics to a new trait like `IntervalArithmetics`? BTW, we currently support `add` and `subtract` for intervals, but it seems `multiply` and `divide` is also supported in the standard? http://download.mimer.com/pub/developer/docs/html_110/Mimer_SQL_Engine_DocSet/Syntax_Rules5.html#wp1113535 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21005: [SPARK-23898][SQL] Simplify add & subtract code g...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/21005#discussion_r179967381 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala --- @@ -117,16 +117,22 @@ abstract class BinaryArithmetic extends BinaryOperator with NullIntolerant { override def dataType: DataType = left.dataType - override lazy val resolved = childrenResolved && checkInputDataTypes().isSuccess + override lazy val resolved: Boolean = childrenResolved && checkInputDataTypes().isSuccess /** Name of the function for this expression on a [[Decimal]] type. */ def decimalMethod: String = sys.error("BinaryArithmetics must override either decimalMethod or genCode") + /** Name of the function for this expression on a [[CalendarInterval]] type. */ + def calendarIntervalMethod: String = +sys.error("BinaryArithmetics must override either calendarIntervalMethod or genCode") + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = dataType match { -case dt: DecimalType => +case _: DecimalType => defineCodeGen(ctx, ev, (eval1, eval2) => s"$eval1.$decimalMethod($eval2)") // byte and short are casted into int when add, minus, times or divide --- End diff -- move this comment to 136? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21005: [SPARK-23898][SQL] Simplify add & subtract code g...
GitHub user hvanhovell opened a pull request: https://github.com/apache/spark/pull/21005 [SPARK-23898][SQL] Simplify add & subtract code generation ## What changes were proposed in this pull request? Code generation for the `Add` and `Subtract` expressions was not done using the `BinaryArithmetic.doCodeGen` method because these expressions also support `CalendarInterval`. This leads to a bit of duplication. This PR gets rid of that duplication by adding `calendarIntervalMethod` to `BinaryArithmetic` and doing the code generation for `CalendarInterval` in `BinaryArithmetic` instead. ## How was this patch tested? Existing tests. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hvanhovell/spark SPARK-23898 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/21005.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #21005 commit 15c13e356606b936f9755a3e8b1d203f06ffd01a Author: Herman van HovellDate: 2018-04-08T19:34:58Z Simplify add & subtract code generation by centralizing code generation for calendar intervals. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org