-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-30341][SQL] Overflow check for interval arithmetic operations #26995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 17 commits
829cfe7
516080c
67767c0
7293377
f100d88
ba44c5a
67645d4
7671d83
b679381
a8d29b6
c3a8531
ccb1fd5
d704c74
512570e
5651959
92e2668
aba10b2
e37caaf
988b51c
f80f0f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -110,49 +110,51 @@ object ExtractIntervalPart { | |||
| } | ||||
| } | ||||
|
|
||||
| abstract class IntervalNumOperation( | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can still implement everything in |
||||
| interval: Expression, | ||||
| num: Expression, | ||||
| operation: (CalendarInterval, Double) => CalendarInterval, | ||||
| operationName: String) | ||||
| abstract class IntervalNumOperation(interval: Expression, num: Expression, operationName: String) | ||||
| extends BinaryExpression with ImplicitCastInputTypes with Serializable { | ||||
|
|
||||
| override def left: Expression = interval | ||||
| override def right: Expression = num | ||||
|
|
||||
| override def inputTypes: Seq[AbstractDataType] = Seq(CalendarIntervalType, DoubleType) | ||||
|
|
||||
| override def dataType: DataType = CalendarIntervalType | ||||
|
|
||||
| override def nullable: Boolean = true | ||||
|
|
||||
| override def nullSafeEval(interval: Any, num: Any): Any = { | ||||
| try { | ||||
| operation(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double]) | ||||
| } catch { | ||||
| case _: java.lang.ArithmeticException => null | ||||
| } | ||||
| override def nullSafeEval(interval: Any, num: Any): Any = operationName match { | ||||
| case "multiply" => | ||||
| multiplyExact(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double]) | ||||
| case "divide" => | ||||
| if (num == 0) return null | ||||
|
||||
| if (${eval2.isNull} || $isZero) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a new operator, I think it's OK to always follow SQL standard here, since we are moving into this direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shall we fix numerics / 0 when ANSI is on too in a followup?
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import scala.language.implicitConversions | |
| import org.apache.spark.SparkFunSuite | ||
| import org.apache.spark.sql.catalyst.util.DateTimeConstants._ | ||
| import org.apache.spark.sql.catalyst.util.IntervalUtils.stringToInterval | ||
| import org.apache.spark.sql.internal.SQLConf | ||
| import org.apache.spark.sql.types.Decimal | ||
| import org.apache.spark.unsafe.types.{CalendarInterval, UTF8String} | ||
|
|
||
|
|
@@ -197,10 +198,23 @@ class IntervalExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { | |
| } | ||
|
|
||
| test("multiply") { | ||
| def check(interval: String, num: Double, expected: String): Unit = { | ||
| checkEvaluation( | ||
| MultiplyInterval(Literal(stringToInterval(interval)), Literal(num)), | ||
| if (expected == null) null else stringToInterval(expected)) | ||
| def check( | ||
| interval: String, | ||
| num: Double, | ||
| expected: String, | ||
| checkException: Boolean = false): Unit = { | ||
| Seq("true", "false").foreach { v => | ||
| withSQLConf(SQLConf.ANSI_ENABLED.key -> v) { | ||
| if (checkException) { | ||
| checkExceptionInExpression[ArithmeticException]( | ||
| MultiplyInterval(Literal(stringToInterval(interval)), Literal(num)), | ||
| "integer overflow") | ||
| } else { | ||
| checkEvaluation(MultiplyInterval(Literal(stringToInterval(interval)), Literal(num)), | ||
| if (expected == null) null else stringToInterval(expected)) | ||
|
||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| check("0 seconds", 10, "0 seconds") | ||
|
|
@@ -211,14 +225,27 @@ class IntervalExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { | |
| check("-100 years -1 millisecond", 0.5, "-50 years -500 microseconds") | ||
| check("2 months 4 seconds", -0.5, "-1 months -2 seconds") | ||
| check("1 month 2 microseconds", 1.5, "1 months 15 days 3 microseconds") | ||
| check("2 months", Int.MaxValue, null) | ||
| check("2 months", Int.MaxValue, null, checkException = true) | ||
| } | ||
|
|
||
| test("divide") { | ||
| def check(interval: String, num: Double, expected: String): Unit = { | ||
| checkEvaluation( | ||
| DivideInterval(Literal(stringToInterval(interval)), Literal(num)), | ||
| if (expected == null) null else stringToInterval(expected)) | ||
| def check( | ||
| interval: String, | ||
| num: Double, | ||
| expected: String, | ||
| checkException: Boolean = false): Unit = { | ||
| Seq("true", "false").foreach { v => | ||
| withSQLConf(SQLConf.ANSI_ENABLED.key -> v) { | ||
| if (checkException) { | ||
| checkExceptionInExpression[ArithmeticException]( | ||
| DivideInterval(Literal(stringToInterval(interval)), Literal(num)), | ||
| "integer overflow") | ||
| } else { | ||
| checkEvaluation(DivideInterval(Literal(stringToInterval(interval)), Literal(num)), | ||
| if (expected == null) null else stringToInterval(expected)) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| check("0 seconds", 10, "0 seconds") | ||
|
|
@@ -229,6 +256,7 @@ class IntervalExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { | |
| check("-1 month 2 microseconds", -0.25, "4 months -8 microseconds") | ||
| check("1 month 3 microsecond", 1.5, "20 days 2 microseconds") | ||
| check("1 second", 0, null) | ||
| check(s"${Int.MaxValue} months", 0.9, null, checkException = true) | ||
| } | ||
|
|
||
| test("make interval") { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.