You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2019/12/24 03:48:50 UTC
[GitHub] [spark] yaooqinn opened a new pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
yaooqinn opened a new pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995
### What changes were proposed in this pull request?
1. For the interval arithmetic functions, e.g. `add`/`subtract`/`negative`/`multiply`/`divide`, enable overflow check when `ANSI` is on.
2. For `add`/`subtract`/`negative`, fix non-checked overflow to result `NULL` when an overflow happens and `ANSI` is off as same as `multiply`/`divide`.
```sql
select interval '-2147483648 months' - interval '1 months';
178956970 years 7 months
```
```sql
select negative(interval '-2147483648 months');
-178956970 years -8 months
```
### Why are the changes needed?
1. bug fix
2. `ANSI` support
### Does this PR introduce any user-facing change?
yes,r `add`/`subtract`/`negative` result in `NULL` if overflow
### How was this patch tested?
add uts
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569626579
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569210478
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20640/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570140086
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116013/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568661213
**[Test build #115692 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115692/testReport)** for PR 26995 at commit [`67767c0`](https://github.com/apache/spark/commit/67767c043671a2c0960494ab06e022317cec12d1).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570152566
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20815/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569624540
**[Test build #115949 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115949/testReport)** for PR 26995 at commit [`512570e`](https://github.com/apache/spark/commit/512570eb0b06189e5407ad3cd1b52ff538bc2ee6).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569624872
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20741/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568673988
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362402679
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -197,10 +198,22 @@ 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)), expected)
Review comment:
we can add a `val expr = MultiplyInterval(Literal(stringToInterval(interval)), Literal(num))` at beginning, to save code duplication.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570271507
**[Test build #116038 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116038/testReport)** for PR 26995 at commit [`f80f0f3`](https://github.com/apache/spark/commit/f80f0f37c0e26eba797ee9ab047e7df3ec4ffb1f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gatorsmile commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r370434285
##########
File path: sql/core/src/test/resources/sql-tests/results/interval.sql.out
##########
@@ -1101,3 +1102,45 @@ select interval '1 ' day
struct<INTERVAL '1 days':interval>
-- !query 113 output
1 days
+
+
+-- !query 114
+select -(a) from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 114 schema
+struct<(- a):interval>
+-- !query 114 output
+-178956970 years -8 months
+
+
+-- !query 115
+select a - b from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 115 schema
+struct<(a - b):interval>
+-- !query 115 output
+1 months
+
+
+-- !query 116
+select b + interval '1 month' from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 116 schema
+struct<(b + INTERVAL '1 months'):interval>
+-- !query 116 output
+-178956970 years -8 months
+
+
+-- !query 117
+select a * 1.1 from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 117 schema
+struct<>
+-- !query 117 output
+java.lang.ArithmeticException
+integer overflow
Review comment:
Can we change it and make them consistent like what you discussed above https://github.com/apache/spark/pull/26995#discussion_r361088588?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568682318
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20499/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568781162
**[Test build #115746 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115746/testReport)** for PR 26995 at commit [`7671d83`](https://github.com/apache/spark/commit/7671d83bb1ccc67fc2beef4a3a26fa44e06da8e7).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362406768
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -197,10 +198,22 @@ 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)), expected)
Review comment:
yes, we can also use `safeStringToInterval(expected) == null` to choose way to check too
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570272157
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116038/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361103770
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -218,6 +252,11 @@ object BinaryArithmetic {
""")
case class Add(left: Expression, right: Expression) extends BinaryArithmetic {
+ override def nullable: Boolean = dataType match {
+ case CalendarIntervalType if !checkOverflow => true
Review comment:
cast from string and cast from int are different and don't need to be consistent. But interval +- and interval */ should be consistent.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569612185
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20735/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568744584
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569663318
**[Test build #115942 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115942/testReport)** for PR 26995 at commit [`ccb1fd5`](https://github.com/apache/spark/commit/ccb1fd50faa4620e1d9f227da044f74c44472c14).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361675825
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
`WHEN ANSI OFF`
1. If we use `NULL ON OVERFLOW` policy for all arithmetics, means interval `add/subtract/negate` break their backward compatibility to fit `multiply/divide`, seem not a good choice.
2. If we remain to use `NULL ON OVERFLOW` policy for `multiply/divide` and java overflow style for `add `etc, this breaks no backward compatibility but also keeps the inconsistency. seem not a good choice either.
3. If we throw exceptions for `multiply/divide` for overflows and remain java overflow style for `add` etc, this breaks no backward compatibility but also raises much worse inconsistency issue.
4. If we throw exceptions for all interval arithmetics, this is just `ANSI ON`.
5. Behavior of current implementation, we do not break backward compatibility either. Things only go wrong when it reaches the `CalendarInterval.MAX_VALUE`. It seems the best choice among the worst.
Things became odd since we didn't support fraction representations for intervals, but support `multiply/divide` intervals with fractions. a.k.a we do not have consistent rules https://github.com/apache/spark/pull/26592 for how the fraction part of month go days, how day fraction go milliseconds.
There seems no other systems can refer to for our `ANSI OFF` mode.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570196840
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116023/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570149468
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116021/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361675825
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
`WHEN ANSI OFF`
1. If we use `NULL ON OVERFLOW` policy for all arithmetics, means interval `add/subtract/negate` break their backward compatibility to fit `multiply/divide`, seem not a good choice.
2. If we remain to use `NULL ON OVERFLOW` policy for `multiply/divide` and java overflow style for `add `etc, this breaks no backward compatibility but also keeps the inconsistency. seem not a good choice either.
3. If we throw exceptions for `multiply/divide` for overflows and remain java overflow style for `add` etc, this breaks no backward compatibility but also raises much worse inconsistency issue.
4. If we throw exceptions for all interval arithmetics, this is just `ANSI ON`.
5. Behavior of current implementation, we do not break backward compatibility either. Things only go "wrong" only when it reaches the `CalendarInterval.MAX_VALUE` or `CalendarInterval.MIN_VALUE`. It seems the best choice among the worst.
Things became odd since we didn't support fraction representations for intervals, but support `multiply/divide` intervals with fractions. a.k.a we do not have consistent rules https://github.com/apache/spark/pull/26592 for how the fraction part of month go days, how day fraction go milliseconds.
There seems no other systems can refer to for our `ANSI OFF` mode.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568644297
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20481/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-572066833
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116249/console here is another flaky failure, maybe related to Jenkins env, e.g. DNS, I'm not sure
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569306897
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115860/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362402624
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -197,10 +198,22 @@ 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)), expected)
+ } else {
+ checkEvaluation(MultiplyInterval(Literal(stringToInterval(interval)), Literal(num)),
+ if (expected == null) null else stringToInterval(expected))
Review comment:
can `expected` be null?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362401636
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -125,34 +125,22 @@ abstract class IntervalNumOperation(
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
- }
+ operation(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
}
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- nullSafeCodeGen(ctx, ev, (interval, num) => {
- val iu = IntervalUtils.getClass.getName.stripSuffix("$")
- s"""
- try {
- ${ev.value} = $iu.$operationName($interval, $num);
- } catch (java.lang.ArithmeticException e) {
- ${ev.isNull} = true;
- }
- """
- })
+ val iu = IntervalUtils.getClass.getName.stripSuffix("$")
+ defineCodeGen(ctx, ev, (interval, num) => s"$iu.$operationName($interval, $num)")
}
- override def prettyName: String = operationName + "_interval"
+ override def prettyName: String = operationName.stripSuffix("Exact") + "_interval"
Review comment:
`.stripSuffix("Exact")` is not needed anymore.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570196837
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568731503
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568738845
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20531/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan edited a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan edited a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570199095
seems multiple PRs start to fail `org.apache.spark.sql.kafka010.KafkaDelegationTokenSuite`. cc @HeartSaVioR
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568682318
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20499/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569682488
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115950/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569611832
**[Test build #115942 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115942/testReport)** for PR 26995 at commit [`ccb1fd5`](https://github.com/apache/spark/commit/ccb1fd50faa4620e1d9f227da044f74c44472c14).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
maropu commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568663021
Why did you target interval only in this pr? If we change the overflow behaivour for arighmetic operatrions, I think we need to carefully handle it cuz it has impacts on the existing behaivours.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568647470
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569871344
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569848473
**[Test build #115977 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115977/testReport)** for PR 26995 at commit [`92e2668`](https://github.com/apache/spark/commit/92e2668c11f4d26205690d972b28b3a2d92eddf5).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569306895
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569624872
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20741/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568734497
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115735/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568742722
**[Test build #115740 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115740/testReport)** for PR 26995 at commit [`67645d4`](https://github.com/apache/spark/commit/67645d4d3415735fcd464338ed6d0dcd52c32088).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568781338
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568731328
**[Test build #115735 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115735/testReport)** for PR 26995 at commit [`f100d88`](https://github.com/apache/spark/commit/f100d8825684885ccb9518da3d252a38fdd51517).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569216429
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115847/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568802703
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115746/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569612173
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570123660
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570123660
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569218614
**[Test build #115851 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115851/testReport)** for PR 26995 at commit [`b679381`](https://github.com/apache/spark/commit/b6793816c16da1eea33dbb9d63e90a9b25394d45).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361096801
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -218,6 +252,11 @@ object BinaryArithmetic {
""")
case class Add(left: Expression, right: Expression) extends BinaryArithmetic {
+ override def nullable: Boolean = dataType match {
+ case CalendarIntervalType if !checkOverflow => true
Review comment:
```
spark-sql> select cast('128' as tinyint);
NULL
Time taken: 0.029 seconds, Fetched 1 row(s)
spark-sql> select cast(128 as tinyint);
-128
```
not quite related to this pr, the cast logic seems unconsisent too
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568682043
**[Test build #115703 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115703/testReport)** for PR 26995 at commit [`7293377`](https://github.com/apache/spark/commit/729337756261a26a2dc376ede9db05a52f238c76).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gatorsmile commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-577991412
Since the mode ANSI is introduced in Spark 3.0 and off by default, this is not a behavior change
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362402318
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -197,10 +198,22 @@ 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) {
Review comment:
BTW we can add a `val expr = MultiplyInterval(Literal(stringToInterval(interval)), Literal(num))` at the beginning, to avoid code duplication.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu edited a comment on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
maropu edited a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568669077
We cannot check it outside `BinaryArithmetic` like ~numeric~ decimal `CheckOverflow`? It seems this pr includes the same try-catch patterns.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569848610
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570140086
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116013/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362374933
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -211,14 +222,17 @@ 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, CalendarInterval.MAX_VALUE.toString, 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))
+ Seq("true", "false").foreach { v =>
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> v) {
+ checkEvaluation(DivideInterval(Literal(stringToInterval(interval)), Literal(num)),
+ if (expected == null) null else stringToInterval(expected))
Review comment:
yes, I will add a check for this.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568663921
> Why did you target interval only in this pr? If we change the overflow behaivour for arighmetic operatrions, I think we need to carefully handle it cuz it has impacts on the existing behaivours.
Because the numeric types are handled in arithmetics properly in both ANSI or non-ANSI mode based on my check. This PR try to fix the interval to follow what the numeric ones do.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568734493
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568644122
**[Test build #115685 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115685/testReport)** for PR 26995 at commit [`829cfe7`](https://github.com/apache/spark/commit/829cfe7a41f243cbef48746e13f2cc2b9cd6053e).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569871347
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115977/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568738845
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20531/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362365706
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -416,13 +418,36 @@ object IntervalUtils {
/**
* Unary minus, return the negated the calendar interval value.
*
- * @param interval the interval to be negated
- * @return a new calendar interval instance with all it parameters negated from the origin one.
+ * @throws ArithmeticException if the result overflows any field value
+ */
+ def negateExact(interval: CalendarInterval): CalendarInterval = {
+ val months = Math.negateExact(interval.months)
+ val days = Math.negateExact(interval.days)
+ val microseconds = Math.negateExact(interval.microseconds)
+ new CalendarInterval(months, days, microseconds)
+ }
+
+ /**
+ * Unary minus, return the negated the calendar interval value.
+ *
Review comment:
unnecessary blank line
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570152566
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20815/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568781162
**[Test build #115746 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115746/testReport)** for PR 26995 at commit [`7671d83`](https://github.com/apache/spark/commit/7671d83bb1ccc67fc2beef4a3a26fa44e06da8e7).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570205377
@cloud-fan Thanks for pinging me. I'll take a look at these failures.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568738584
**[Test build #115737 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115737/testReport)** for PR 26995 at commit [`ba44c5a`](https://github.com/apache/spark/commit/ba44c5a7a5cee3e46464103fb31e36aaa2a2c12d).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361084406
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -37,6 +37,11 @@ case class UnaryMinus(child: Expression) extends UnaryExpression
with ExpectsInputTypes with NullIntolerant {
private val checkOverflow = SQLConf.get.ansiEnabled
+ override def nullable: Boolean = dataType match {
+ case CalendarIntervalType if !checkOverflow => true
Review comment:
isn't it true for all types?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568673923
**[Test build #115692 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115692/testReport)** for PR 26995 at commit [`67767c0`](https://github.com/apache/spark/commit/67767c043671a2c0960494ab06e022317cec12d1).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569653331
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115949/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569210474
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569218899
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569252299
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115851/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569624540
**[Test build #115949 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115949/testReport)** for PR 26995 at commit [`512570e`](https://github.com/apache/spark/commit/512570eb0b06189e5407ad3cd1b52ff538bc2ee6).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568781341
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20540/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361930716
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
ok, make sense
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570149462
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568781341
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20540/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568731507
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20529/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568763817
**[Test build #115740 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115740/testReport)** for PR 26995 at commit [`67645d4`](https://github.com/apache/spark/commit/67645d4d3415735fcd464338ed6d0dcd52c32088).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class MultiplyInterval(interval: Expression, num: Expression)`
* `case class DivideInterval(interval: Expression, num: Expression)`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568687587
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115703/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361637453
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
+ Int.MaxValue
+ } else if (monthInLong < Int.MinValue) {
+ Int.MinValue
+ } else {
+ monthInLong.toInt
+ }
+ val days = daysWithFraction + DAYS_PER_MONTH * (monthsWithFraction - truncatedMonths)
+ val dayInLong = days.toLong
+ val truncatedDays = if (dayInLong > Int.MaxValue) {
+ Int.MaxValue
+ } else if (monthInLong < Int.MinValue) {
+ Int.MinValue
+ } else {
+ dayInLong.toInt
+ }
+ val micros = microsWithFraction + MICROS_PER_DAY * (days - truncatedDays)
+ new CalendarInterval(truncatedMonths, truncatedDays.toInt, micros.round)
+ }
+
+ /**
+ * Unary minus, return the negated the calendar interval value.
+ *
+ * @param interval the interval to be negated
+ * @return a new calendar interval instance with all it parameters negated from the origin one.
Review comment:
it's a private class so maybe no need to add `@param` and `@return`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362366194
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -110,50 +110,57 @@ object ExtractIntervalPart {
}
}
-abstract class IntervalNumOperation(
Review comment:
We can still implement everything in `IntervalNumOperation`, the subclasses only need to provide the method.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570196677
**[Test build #116023 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116023/testReport)** for PR 26995 at commit [`f80f0f3`](https://github.com/apache/spark/commit/f80f0f37c0e26eba797ee9ab047e7df3ec4ffb1f).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569626583
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20742/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362389346
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -75,12 +75,20 @@ case class UnaryMinus(child: Expression) extends UnaryExpression
"""})
case _: CalendarIntervalType =>
val iu = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$iu.negate($c)")
Review comment:
nit:
```
val method = if (checkOverflow) "negateExact" elese "negate"
defineCodeGen(ctx, ev, c => s"$iu.$method($c)")
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570145827
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570197557
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20830/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361656171
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
the behavior with current implementation follows `select cast(9223372036854775807L * 1.1D as Long);` results 9223372036854775807L
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568659725
**[Test build #115690 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115690/testReport)** for PR 26995 at commit [`516080c`](https://github.com/apache/spark/commit/516080cbea19905fbfa177abced0f5853792e9fe).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570149468
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116021/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569306897
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115860/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570199095
seems all the PRs start to fail `org.apache.spark.sql.kafka010.KafkaDelegationTokenSuite`. cc @HeartSaVioR
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
maropu commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568669077
We cannot check it outside `BinaryArithmetic` like numeric `CheckOverflow`? It seems this pr includes the same try-catch patterns.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569611832
**[Test build #115942 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115942/testReport)** for PR 26995 at commit [`ccb1fd5`](https://github.com/apache/spark/commit/ccb1fd50faa4620e1d9f227da044f74c44472c14).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568660002
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20487/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568668786
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115690/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570152207
**[Test build #116023 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116023/testReport)** for PR 26995 at commit [`f80f0f3`](https://github.com/apache/spark/commit/f80f0f37c0e26eba797ee9ab047e7df3ec4ffb1f).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570152560
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568802698
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gatorsmile commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-577949404
When ANSI is on, interval add/subtract/negative/multiply/divide will overflow if any field overflows. However, when ANSI is off and overflow happens, why multiply/divide will throw an exception but add/subtract/negative will not throw an exception? This looks inconsistent.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569612185
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20735/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361094602
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -218,6 +252,11 @@ object BinaryArithmetic {
""")
case class Add(left: Expression, right: Expression) extends BinaryArithmetic {
+ override def nullable: Boolean = dataType match {
+ case CalendarIntervalType if !checkOverflow => true
Review comment:
then let's be consistent and follow java overflow behavior when ansi is false.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569663705
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115942/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi edited a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi edited a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-572060738
@yaooqinn thanks for sharing. Yep, maybe the kerberos principal is wrong again. Not sure how this can be flaky though since it's hardcoded...
The mentioned `AuthFailed` is just the end-result of the `KrbException`.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570123663
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20805/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570199030
**[Test build #116038 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116038/testReport)** for PR 26995 at commit [`f80f0f3`](https://github.com/apache/spark/commit/f80f0f37c0e26eba797ee9ab047e7df3ec4ffb1f).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570152207
**[Test build #116023 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116023/testReport)** for PR 26995 at commit [`f80f0f3`](https://github.com/apache/spark/commit/f80f0f37c0e26eba797ee9ab047e7df3ec4ffb1f).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568738844
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570140081
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568738584
**[Test build #115737 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115737/testReport)** for PR 26995 at commit [`ba44c5a`](https://github.com/apache/spark/commit/ba44c5a7a5cee3e46464103fb31e36aaa2a2c12d).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570196837
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gatorsmile commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r370473845
##########
File path: sql/core/src/test/resources/sql-tests/results/interval.sql.out
##########
@@ -1101,3 +1102,45 @@ select interval '1 ' day
struct<INTERVAL '1 days':interval>
-- !query 113 output
1 days
+
+
+-- !query 114
+select -(a) from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 114 schema
+struct<(- a):interval>
+-- !query 114 output
+-178956970 years -8 months
+
+
+-- !query 115
+select a - b from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 115 schema
+struct<(a - b):interval>
+-- !query 115 output
+1 months
+
+
+-- !query 116
+select b + interval '1 month' from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 116 schema
+struct<(b + INTERVAL '1 months'):interval>
+-- !query 116 output
+-178956970 years -8 months
+
+
+-- !query 117
+select a * 1.1 from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 117 schema
+struct<>
+-- !query 117 output
+java.lang.ArithmeticException
+integer overflow
Review comment:
Please create a 3.0 blocker JIRA.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362391555
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -110,49 +110,51 @@ object ExtractIntervalPart {
}
}
-abstract class IntervalNumOperation(
- 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
Review comment:
This is a new operator, I think it's OK to always follow SQL standard here, since we are moving into this direction.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361675825
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
`WHEN ANSI OFF`
There seems no
1. If we use `NULL ON OVERFLOW` policy for all arithmetics, means interval `add/subtract/negate` break their backward compatibility to fit `multiply/divide`, seem not a good choice.
2. If we remain to use `NULL ON OVERFLOW` policy for `multiply/divide` and java overflow style for `add `etc, this breaks no backward compatibility but also keeps the inconsistency. seem not a good choice either.
3. If we throw exceptions for `multiply/divide` for overflows and remain java overflow style for `add` etc, this breaks no backward compatibility but also raises much worse inconsistency issue.
4. If we throw exceptions for all interval arithmetics, this is just `ANSI ON`.
5. Behavior of current implementation, we do not break backward compatibility either. Things only go wrong when it reaches the `CalendarInterval.MAX_VALUE`. It seems the best choice among the worst.
Things became odd since we didn't support fraction representations for intervals, but support `multiply/divide` intervals with fractions. a.k.a we do not have consistent rules https://github.com/apache/spark/pull/26592 for how the fraction part of month go days, how day fraction go milliseconds.
There seems no other systems can refer to for our `ANSI OFF` mode.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569848612
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20769/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569653252
**[Test build #115949 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115949/testReport)** for PR 26995 at commit [`512570e`](https://github.com/apache/spark/commit/512570eb0b06189e5407ad3cd1b52ff538bc2ee6).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362402157
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -197,10 +198,22 @@ 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) {
Review comment:
We don't need the `checkException` flag
```
if (expected == null) {
checkExceptionInExpression
} else {
...
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569264474
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20653/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569626583
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20742/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-572046792
```scala
Found ticket for zkclient/localhost@EXAMPLE.COM to go to krbtgt/EXAMPLE.COM@EXAMPLE.COM expiring on Fri Jan 03 04:30:09 PST 2020
Service ticket not found in the subject
KrbException: Server not found in Kerberos database (7) - Server not found in Kerberos database
at sun.security.krb5.KrbTgsRep.<init>(KrbTgsRep.java:73)
at sun.security.krb5.KrbTgsReq.getReply(KrbTgsReq.java:251)
at sun.security.krb5.KrbTgsReq.sendAndGetCreds(KrbTgsReq.java:262)
at sun.security.krb5.internal.CredentialsUtil.serviceCreds(CredentialsUtil.java:308)
at sun.security.krb5.internal.CredentialsUtil.acquireServiceCreds(CredentialsUtil.java:126)
at sun.security.krb5.Credentials.acquireServiceCreds(Credentials.java:458)
at sun.security.jgss.krb5.Krb5Context.initSecContext(Krb5Context.java:693)
at sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:248)
at sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:179)
at com.sun.security.sasl.gsskerb.GssKrb5Client.evaluateChallenge(GssKrb5Client.java:192)
at org.apache.zookeeper.client.ZooKeeperSaslClient$1.run(ZooKeeperSaslClient.java:323)
at org.apache.zookeeper.client.ZooKeeperSaslClient$1.run(ZooKeeperSaslClient.java:320)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at org.apache.zookeeper.client.ZooKeeperSaslClient.createSaslToken(ZooKeeperSaslClient.java:320)
at org.apache.zookeeper.client.ZooKeeperSaslClient.createSaslToken(ZooKeeperSaslClient.java:305)
at org.apache.zookeeper.client.ZooKeeperSaslClient.sendSaslPacket(ZooKeeperSaslClient.java:377)
at org.apache.zookeeper.client.ZooKeeperSaslClient.initialize(ZooKeeperSaslClient.java:415)
at org.apache.zookeeper.ClientCnxn$SendThread.run(ClientCnxn.java:1149)
Caused by: KrbException: Identifier doesn't match expected value (906)
at sun.security.krb5.internal.KDCRep.init(KDCRep.java:140)
at sun.security.krb5.internal.TGSRep.init(TGSRep.java:65)
at sun.security.krb5.internal.TGSRep.<init>(TGSRep.java:60)
at sun.security.krb5.KrbTgsRep.<init>(KrbTgsRep.java:55)
... 18 more
```
Hi @gaborgsomogyi you might need look at this exception which is throwed right before the AuthFailedException
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-572034921
Just FYI I spent my time a bit to deal with `org.apache.spark.sql.kafka010.KafkaDelegationTokenSuite`, but no luck. I'd like to ask @gaborgsomogyi to help resolving the issue, as he authored the test and had investigated before for similar issue.
Hi @gaborgsomogyi , welcome back! Would you mind to try dealing with this flaky test? Thanks in advance!
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568644295
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570197557
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20830/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-577953559
> Compared with the previous release, any behavior change we made in this PR?
When ANSI is on, interval add/subtract/negative will check overflow compared with 2.4
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569682485
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568742912
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569626335
**[Test build #115950 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115950/testReport)** for PR 26995 at commit [`5651959`](https://github.com/apache/spark/commit/56519596e4ebc886fae5be302bd56bab14514b21).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570197549
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569682485
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569210474
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570272147
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362365872
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -197,10 +198,20 @@ 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,
+ modes: Seq[String] = Seq("true", "false"), checkException: Boolean = false): Unit = {
Review comment:
where do we set `modes`?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568744587
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115737/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569871344
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361651024
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
hmm, then for multiply/divide overflow we return `null`s for other arithmetics we return overflowed values? this seems weird too
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568668720
**[Test build #115690 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115690/testReport)** for PR 26995 at commit [`516080c`](https://github.com/apache/spark/commit/516080cbea19905fbfa177abced0f5853792e9fe).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570152560
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361636765
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -112,49 +113,96 @@ object ExtractIntervalPart {
}
}
-abstract class IntervalNumOperation(
- interval: Expression,
- num: Expression,
- operation: (CalendarInterval, Double) => CalendarInterval,
- operationName: String)
+abstract class IntervalNumOperation(interval: Expression, num: Expression)
extends BinaryExpression with ImplicitCastInputTypes with Serializable {
+
+ protected val checkOverflow: Boolean = SQLConf.get.ansiEnabled
+
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
+}
+
+case class MultiplyInterval(interval: Expression, num: Expression)
+ extends IntervalNumOperation(interval, num) {
+
+ override def prettyName: String = "multiply_interval"
override def nullSafeEval(interval: Any, num: Any): Any = {
try {
- operation(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ if (checkOverflow) {
+ multiplyExact(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ } else {
+ multiply(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ }
} catch {
- case _: java.lang.ArithmeticException => null
+ case _: ArithmeticException if !checkOverflow => null
}
}
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (interval, num) => {
val iu = IntervalUtils.getClass.getName.stripSuffix("$")
+ val operationName = if (checkOverflow) "multiplyExact" else "multiply"
s"""
try {
${ev.value} = $iu.$operationName($interval, $num);
- } catch (java.lang.ArithmeticException e) {
- ${ev.isNull} = true;
+ } catch (ArithmeticException e) {
+ if ($checkOverflow) {
+ throw e;
+ } else {
+ ${ev.isNull} = true;
+ }
}
"""
})
}
-
- override def prettyName: String = operationName + "_interval"
}
-case class MultiplyInterval(interval: Expression, num: Expression)
- extends IntervalNumOperation(interval, num, multiply, "multiply")
-
case class DivideInterval(interval: Expression, num: Expression)
- extends IntervalNumOperation(interval, num, divide, "divide")
+ extends IntervalNumOperation(interval, num) {
+
+ override def prettyName: String = "divide_interval"
+
+ override def nullSafeEval(interval: Any, num: Any): Any = {
+ try {
+ if (num == 0) return null
+ if (checkOverflow) {
+ divideExact(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ } else {
+ divide(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ }
+ } catch {
+ case _: ArithmeticException if !checkOverflow => null
Review comment:
is it needed?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r370438991
##########
File path: sql/core/src/test/resources/sql-tests/results/interval.sql.out
##########
@@ -1101,3 +1102,45 @@ select interval '1 ' day
struct<INTERVAL '1 days':interval>
-- !query 113 output
1 days
+
+
+-- !query 114
+select -(a) from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 114 schema
+struct<(- a):interval>
+-- !query 114 output
+-178956970 years -8 months
+
+
+-- !query 115
+select a - b from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 115 schema
+struct<(a - b):interval>
+-- !query 115 output
+1 months
+
+
+-- !query 116
+select b + interval '1 month' from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 116 schema
+struct<(b + INTERVAL '1 months'):interval>
+-- !query 116 output
+-178956970 years -8 months
+
+
+-- !query 117
+select a * 1.1 from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 117 schema
+struct<>
+-- !query 117 output
+java.lang.ArithmeticException
+integer overflow
Review comment:
I am OK with your suggestion，but first let us also cc@cloud-fan
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362400457
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Average.scala
##########
@@ -81,7 +80,8 @@ case class Average(child: Expression) extends DeclarativeAggregate with Implicit
case _: DecimalType =>
DecimalPrecision.decimalAndDecimal(sum / count.cast(DecimalType.LongDecimal)).cast(resultType)
case CalendarIntervalType =>
- DivideInterval(sum.cast(resultType), count.cast(DoubleType))
+ val newCount = If(EqualTo(count, Literal(0L)), Literal(null, LongType), count)
Review comment:
`avg(interval)` is also new in 3.0 right? We can also fail here if this is the SQL standard.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362389157
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -110,49 +110,51 @@ object ExtractIntervalPart {
}
}
-abstract class IntervalNumOperation(
- 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
Review comment:
shall we throw exception for divide by 0?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570197217
retest this please
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569653331
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115949/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r370438991
##########
File path: sql/core/src/test/resources/sql-tests/results/interval.sql.out
##########
@@ -1101,3 +1102,45 @@ select interval '1 ' day
struct<INTERVAL '1 days':interval>
-- !query 113 output
1 days
+
+
+-- !query 114
+select -(a) from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 114 schema
+struct<(- a):interval>
+-- !query 114 output
+-178956970 years -8 months
+
+
+-- !query 115
+select a - b from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 115 schema
+struct<(a - b):interval>
+-- !query 115 output
+1 months
+
+
+-- !query 116
+select b + interval '1 month' from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 116 schema
+struct<(b + INTERVAL '1 months'):interval>
+-- !query 116 output
+-178956970 years -8 months
+
+
+-- !query 117
+select a * 1.1 from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 117 schema
+struct<>
+-- !query 117 output
+java.lang.ArithmeticException
+integer overflow
Review comment:
I am OK with your suggestion，but first let us also cc @cloud-fan
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570289671
thanks, merging to master!
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570145543
**[Test build #116021 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116021/testReport)** for PR 26995 at commit [`988b51c`](https://github.com/apache/spark/commit/988b51c27fcc32ebfa8e8379ac4ae2913626c66d).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568763885
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115740/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-572060738
@yaooqinn thanks for sharing. Yep, maybe the kerberos principal is wrong again. Not sure how this can be flaky though since it's hardcoded...
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r372905544
##########
File path: sql/core/src/test/resources/sql-tests/results/interval.sql.out
##########
@@ -1101,3 +1102,45 @@ select interval '1 ' day
struct<INTERVAL '1 days':interval>
-- !query 113 output
1 days
+
+
+-- !query 114
+select -(a) from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 114 schema
+struct<(- a):interval>
+-- !query 114 output
+-178956970 years -8 months
+
+
+-- !query 115
+select a - b from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 115 schema
+struct<(a - b):interval>
+-- !query 115 output
+1 months
+
+
+-- !query 116
+select b + interval '1 month' from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 116 schema
+struct<(b + INTERVAL '1 months'):interval>
+-- !query 116 output
+-178956970 years -8 months
+
+
+-- !query 117
+select a * 1.1 from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 117 schema
+struct<>
+-- !query 117 output
+java.lang.ArithmeticException
+integer overflow
Review comment:
IIRC a problem is: the new operators in 3.0 (`interval * double` and `interval / double`) do not have a reasonable java style overflow behavior.
The actual operation is `(int * double).toInt`, and `Double.PositiveInfinity.toInt` returns `Int.Max`, which can be confusing.
Since ansi mode is off by default, we are introducing a new behavior that is non-ansi, which is weird. There are 2 options:
1. `interval * double` and `interval / double` return max or min int/long value when overflow. `interval / 0` returns null.
2. revert these 2 operators.
@gatorsmile what do you think?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361084406
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -37,6 +37,11 @@ case class UnaryMinus(child: Expression) extends UnaryExpression
with ExpectsInputTypes with NullIntolerant {
private val checkOverflow = SQLConf.get.ansiEnabled
+ override def nullable: Boolean = dataType match {
+ case CalendarIntervalType if !checkOverflow => true
Review comment:
isn't it true for all types?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568742722
**[Test build #115740 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115740/testReport)** for PR 26995 at commit [`67645d4`](https://github.com/apache/spark/commit/67645d4d3415735fcd464338ed6d0dcd52c32088).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568672756
`CheckOverflow` is for decimal only, not all numerics
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361146102
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -218,6 +252,11 @@ object BinaryArithmetic {
""")
case class Add(left: Expression, right: Expression) extends BinaryArithmetic {
+ override def nullable: Boolean = dataType match {
+ case CalendarIntervalType if !checkOverflow => true
Review comment:
changed behavior to a) ANSI on: exception, b) ANSI off: java style overflow
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570272147
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569217745
retest this please
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gatorsmile commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-577952790
Compared with the previous release, any behavior change we made in this PR?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568668780
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568763882
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568673994
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115692/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568647472
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115685/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568644122
**[Test build #115685 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115685/testReport)** for PR 26995 at commit [`829cfe7`](https://github.com/apache/spark/commit/829cfe7a41f243cbef48746e13f2cc2b9cd6053e).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569626335
**[Test build #115950 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115950/testReport)** for PR 26995 at commit [`5651959`](https://github.com/apache/spark/commit/56519596e4ebc886fae5be302bd56bab14514b21).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568661213
**[Test build #115692 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115692/testReport)** for PR 26995 at commit [`67767c0`](https://github.com/apache/spark/commit/67767c043671a2c0960494ab06e022317cec12d1).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362399970
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -75,12 +75,20 @@ case class UnaryMinus(child: Expression) extends UnaryExpression
"""})
case _: CalendarIntervalType =>
val iu = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$iu.negate($c)")
Review comment:
fixed
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570196840
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116023/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568647448
**[Test build #115685 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115685/testReport)** for PR 26995 at commit [`829cfe7`](https://github.com/apache/spark/commit/829cfe7a41f243cbef48746e13f2cc2b9cd6053e).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568668786
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115690/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570145827
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570149462
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362401180
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Average.scala
##########
@@ -81,7 +80,8 @@ case class Average(child: Expression) extends DeclarativeAggregate with Implicit
case _: DecimalType =>
DecimalPrecision.decimalAndDecimal(sum / count.cast(DecimalType.LongDecimal)).cast(resultType)
case CalendarIntervalType =>
- DivideInterval(sum.cast(resultType), count.cast(DoubleType))
+ val newCount = If(EqualTo(count, Literal(0L)), Literal(null, LongType), count)
Review comment:
I checked pgsql, avg on empty table returns null. So this is corrected.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569871119
**[Test build #115977 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115977/testReport)** for PR 26995 at commit [`92e2668`](https://github.com/apache/spark/commit/92e2668c11f4d26205690d972b28b3a2d92eddf5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569848473
**[Test build #115977 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115977/testReport)** for PR 26995 at commit [`92e2668`](https://github.com/apache/spark/commit/92e2668c11f4d26205690d972b28b3a2d92eddf5).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569663705
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115942/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361084511
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -37,6 +37,11 @@ case class UnaryMinus(child: Expression) extends UnaryExpression
with ExpectsInputTypes with NullIntolerant {
private val checkOverflow = SQLConf.get.ansiEnabled
+ override def nullable: Boolean = dataType match {
+ case CalendarIntervalType if !checkOverflow => true
+ case _ => super.nullable
Review comment:
nit: `child.nullable` is more clear here.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569653325
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570123502
**[Test build #116013 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116013/testReport)** for PR 26995 at commit [`aba10b2`](https://github.com/apache/spark/commit/aba10b22bc3928dd88cb20ae0db340413ec0b3b6).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570145543
**[Test build #116021 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116021/testReport)** for PR 26995 at commit [`988b51c`](https://github.com/apache/spark/commit/988b51c27fcc32ebfa8e8379ac4ae2913626c66d).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569264470
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569216429
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115847/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569218899
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568673988
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361085337
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -218,6 +252,11 @@ object BinaryArithmetic {
""")
case class Add(left: Expression, right: Expression) extends BinaryArithmetic {
+ override def nullable: Boolean = dataType match {
+ case CalendarIntervalType if !checkOverflow => true
Review comment:
shall we make the overflow behavior consistent? e.g. other numeric types follow the java overflow behavior and interval returns null for overflow, which is inconsistent.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569871347
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115977/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569264474
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20653/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569216427
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570139984
**[Test build #116013 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116013/testReport)** for PR 26995 at commit [`aba10b2`](https://github.com/apache/spark/commit/aba10b22bc3928dd88cb20ae0db340413ec0b3b6).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class MultiplyInterval(interval: Expression, num: Expression)`
* `case class DivideInterval(interval: Expression, num: Expression)`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570140081
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362391445
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -110,49 +110,51 @@ object ExtractIntervalPart {
}
}
-abstract class IntervalNumOperation(
- 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
Review comment:
https://github.com/apache/spark/blob/b6793816c16da1eea33dbb9d63e90a9b25394d45/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala#L362
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568738844
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568687587
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115703/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569218614
**[Test build #115851 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115851/testReport)** for PR 26995 at commit [`b679381`](https://github.com/apache/spark/commit/b6793816c16da1eea33dbb9d63e90a9b25394d45).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569624869
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568781338
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568763885
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115740/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361651024
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
hmm, then for multiply/divide overflow we return `null`s for other arithmetics we return overflowed values when ansi off? this seems weird too
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362402318
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -197,10 +198,22 @@ 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) {
Review comment:
BTW we can add a `val expr = MultiplyInterval(Literal(stringToInterval(interval)), Literal(num))` at the beginning, to avoid code duplication.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-572045251
@HeartSaVioR Thanks for pinging. I've had a slight look and seems like it may caused by `kafka-clients` 2.4 upgrade. Instead of `ZkUtils` now `KafkaZkClient` used to call `getAllBrokersInCluster` which is causing the issue:
```
Caused by: sbt.ForkMain$ForkError: org.apache.zookeeper.KeeperException$AuthFailedException: KeeperErrorCode = AuthFailed for /brokers/ids
at org.apache.zookeeper.KeeperException.create(KeeperException.java:130)
at org.apache.zookeeper.KeeperException.create(KeeperException.java:54)
at kafka.zookeeper.AsyncResponse.resultException(ZooKeeperClient.scala:554)
at kafka.zk.KafkaZkClient.getChildren(KafkaZkClient.scala:719)
at kafka.zk.KafkaZkClient.getSortedBrokerList(KafkaZkClient.scala:455)
at kafka.zk.KafkaZkClient.getAllBrokersInCluster(KafkaZkClient.scala:404)
at org.apache.spark.sql.kafka010.KafkaTestUtils.$anonfun$setup$3(KafkaTestUtils.scala:293)
at org.scalatest.concurrent.Eventually.makeAValiantAttempt$1(Eventually.scala:395)
at org.scalatest.concurrent.Eventually.tryTryAgain$1(Eventually.scala:409)
... 20 more
```
My first guess is that the new API behaves somehow different and/or it may contain some bugs.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569612173
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569626579
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568673994
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115692/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gatorsmile commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r373817637
##########
File path: sql/core/src/test/resources/sql-tests/results/interval.sql.out
##########
@@ -1101,3 +1102,45 @@ select interval '1 ' day
struct<INTERVAL '1 days':interval>
-- !query 113 output
1 days
+
+
+-- !query 114
+select -(a) from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 114 schema
+struct<(- a):interval>
+-- !query 114 output
+-178956970 years -8 months
+
+
+-- !query 115
+select a - b from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 115 schema
+struct<(a - b):interval>
+-- !query 115 output
+1 months
+
+
+-- !query 116
+select b + interval '1 month' from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 116 schema
+struct<(b + INTERVAL '1 months'):interval>
+-- !query 116 output
+-178956970 years -8 months
+
+
+-- !query 117
+select a * 1.1 from values (interval '-2147483648 months', interval '2147483647 months') t(a, b)
+-- !query 117 schema
+struct<>
+-- !query 117 output
+java.lang.ArithmeticException
+integer overflow
Review comment:
It sounds like the option 2 looks good to me.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568682043
**[Test build #115703 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115703/testReport)** for PR 26995 at commit [`7293377`](https://github.com/apache/spark/commit/729337756261a26a2dc376ede9db05a52f238c76).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568731503
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568744562
**[Test build #115737 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115737/testReport)** for PR 26995 at commit [`ba44c5a`](https://github.com/apache/spark/commit/ba44c5a7a5cee3e46464103fb31e36aaa2a2c12d).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class DivideInterval(`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568763882
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569682488
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115950/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362401636
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -125,34 +125,22 @@ abstract class IntervalNumOperation(
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
- }
+ operation(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
}
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- nullSafeCodeGen(ctx, ev, (interval, num) => {
- val iu = IntervalUtils.getClass.getName.stripSuffix("$")
- s"""
- try {
- ${ev.value} = $iu.$operationName($interval, $num);
- } catch (java.lang.ArithmeticException e) {
- ${ev.isNull} = true;
- }
- """
- })
+ val iu = IntervalUtils.getClass.getName.stripSuffix("$")
+ defineCodeGen(ctx, ev, (interval, num) => s"$iu.$operationName($interval, $num)")
}
- override def prettyName: String = operationName + "_interval"
+ override def prettyName: String = operationName.stripSuffix("Exact") + "_interval"
Review comment:
`.stripSuffix("Exact")` is not needed anymore.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568668780
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570145828
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20813/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569252293
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568660002
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20487/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568802546
**[Test build #115746 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115746/testReport)** for PR 26995 at commit [`7671d83`](https://github.com/apache/spark/commit/7671d83bb1ccc67fc2beef4a3a26fa44e06da8e7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568734480
**[Test build #115735 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115735/testReport)** for PR 26995 at commit [`f100d88`](https://github.com/apache/spark/commit/f100d8825684885ccb9518da3d252a38fdd51517).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `abstract class IntervalNumOperation(interval: Expression, num: Expression)`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362402737
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -211,14 +224,26 @@ 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, "integer overflow", 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)), expected)
Review comment:
ditto
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361084666
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -75,12 +80,29 @@ case class UnaryMinus(child: Expression) extends UnaryExpression
"""})
case _: CalendarIntervalType =>
val iu = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$iu.negate($c)")
+ nullSafeCodeGen(ctx, ev, interval => s"""
+ try {
+ ${ev.value} = $iu.negate($interval);
+ } catch (ArithmeticException e) {
+ if ($checkOverflow) {
+ throw new ArithmeticException("-($interval) caused interval overflow.");
+ } else {
+ ${ev.isNull} = true;
+ }
+ }
+ """)
}
protected override def nullSafeEval(input: Any): Any = dataType match {
- case CalendarIntervalType => IntervalUtils.negate(input.asInstanceOf[CalendarInterval])
- case _ => numeric.negate(input)
+ case CalendarIntervalType =>
+ try {
+ IntervalUtils.negate(input.asInstanceOf[CalendarInterval])
Review comment:
nit: indentation
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568734497
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115735/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568647472
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115685/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569306895
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568744587
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115737/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568659999
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569252293
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570197549
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570199030
**[Test build #116038 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116038/testReport)** for PR 26995 at commit [`f80f0f3`](https://github.com/apache/spark/commit/f80f0f37c0e26eba797ee9ab047e7df3ec4ffb1f).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361660797
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
We add the overflow flag so that we can avoid breaking backward compatibility. We don't have this concern for interval multiple.
For the overflow behavior, I think it's caused by `Double.toLong`. Is there an exact version of it? if not we may need to check `double > Long.Max` first.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569682140
**[Test build #115950 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115950/testReport)** for PR 26995 at commit [`5651959`](https://github.com/apache/spark/commit/56519596e4ebc886fae5be302bd56bab14514b21).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569218902
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20644/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361638521
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
It's not truncation that only drops the fraction part. I think it's hard to define "safe" for this `fromDoubles` method. Maybe we don't need a safe version for interval multiple (and we never have)
how about
```
def multiplyExact(interval: CalendarInterval, num: Double): CalendarInterval = {
fromDoubles(num * interval.months, num * interval.days, num * interval.microseconds)
}
def multiple(interval: CalendarInterval, num: Double): CalendarInterval = multiplyExact(interval, num)
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568744584
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361649973
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -112,49 +113,96 @@ object ExtractIntervalPart {
}
}
-abstract class IntervalNumOperation(
- interval: Expression,
- num: Expression,
- operation: (CalendarInterval, Double) => CalendarInterval,
- operationName: String)
+abstract class IntervalNumOperation(interval: Expression, num: Expression)
extends BinaryExpression with ImplicitCastInputTypes with Serializable {
+
+ protected val checkOverflow: Boolean = SQLConf.get.ansiEnabled
+
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
+}
+
+case class MultiplyInterval(interval: Expression, num: Expression)
+ extends IntervalNumOperation(interval, num) {
+
+ override def prettyName: String = "multiply_interval"
override def nullSafeEval(interval: Any, num: Any): Any = {
try {
- operation(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ if (checkOverflow) {
+ multiplyExact(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ } else {
+ multiply(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ }
} catch {
- case _: java.lang.ArithmeticException => null
+ case _: ArithmeticException if !checkOverflow => null
}
}
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (interval, num) => {
val iu = IntervalUtils.getClass.getName.stripSuffix("$")
+ val operationName = if (checkOverflow) "multiplyExact" else "multiply"
s"""
try {
${ev.value} = $iu.$operationName($interval, $num);
- } catch (java.lang.ArithmeticException e) {
- ${ev.isNull} = true;
+ } catch (ArithmeticException e) {
+ if ($checkOverflow) {
+ throw e;
+ } else {
+ ${ev.isNull} = true;
+ }
}
"""
})
}
-
- override def prettyName: String = operationName + "_interval"
}
-case class MultiplyInterval(interval: Expression, num: Expression)
- extends IntervalNumOperation(interval, num, multiply, "multiply")
-
case class DivideInterval(interval: Expression, num: Expression)
- extends IntervalNumOperation(interval, num, divide, "divide")
+ extends IntervalNumOperation(interval, num) {
+
+ override def prettyName: String = "divide_interval"
+
+ override def nullSafeEval(interval: Any, num: Any): Any = {
+ try {
+ if (num == 0) return null
+ if (checkOverflow) {
+ divideExact(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ } else {
+ divide(interval.asInstanceOf[CalendarInterval], num.asInstanceOf[Double])
+ }
+ } catch {
+ case _: ArithmeticException if !checkOverflow => null
Review comment:
ahh, I c..
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361084982
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -75,12 +80,29 @@ case class UnaryMinus(child: Expression) extends UnaryExpression
"""})
case _: CalendarIntervalType =>
val iu = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$iu.negate($c)")
+ nullSafeCodeGen(ctx, ev, interval => s"""
+ try {
+ ${ev.value} = $iu.negate($interval);
Review comment:
shall we have 2 methods `negate` and `safeNegate`? It's good to keep the generated code small.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568802703
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115746/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361088588
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -218,6 +252,11 @@ object BinaryArithmetic {
""")
case class Add(left: Expression, right: Expression) extends BinaryArithmetic {
+ override def nullable: Boolean = dataType match {
+ case CalendarIntervalType if !checkOverflow => true
Review comment:
Yes, the current behavior of master is separating a) decimal(which is null for overflow) from b) other types. This pr (so far)is just adding intervals to group a). We may reach an agreement on which way to follow first.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568742916
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20534/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568682314
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568734493
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn edited a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
yaooqinn edited a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568663921
> Why did you target interval only in this pr? If we change the overflow behaivour for arighmetic operatrions, I think we need to carefully handle it cuz it has impacts on the existing behaivours.
Because the numeric types are handled in arithmetics properly in both ANSI or non-ANSI mode based on my check. This PR try to fix the interval to follow what the numeric ones do. Thus, the behavior impacts are limited to the interval operations as the pr description says.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570149445
**[Test build #116021 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116021/testReport)** for PR 26995 at commit [`988b51c`](https://github.com/apache/spark/commit/988b51c27fcc32ebfa8e8379ac4ae2913626c66d).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570145828
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20813/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361591860
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,45 +415,141 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
+ Int.MaxValue
+ } else if (monthInLong < Int.MinValue) {
+ Int.MinValue
+ } else {
+ monthInLong.toInt
+ }
+ val days = daysWithFraction + DAYS_PER_MONTH * (monthsWithFraction - truncatedMonths)
+ val dayInLong = days.toLong
+ val truncatedDays = if (dayInLong > Int.MaxValue) {
+ Int.MaxValue
+ } else if (monthInLong < Int.MinValue) {
+ Int.MinValue
+ } else {
+ dayInLong.toInt
+ }
+ val micros = microsWithFraction + MICROS_PER_DAY * (days - truncatedDays)
+ new CalendarInterval(truncatedMonths, truncatedDays.toInt, micros.round)
+ }
+
/**
* Unary minus, return the negated the calendar interval value.
*
* @param interval the interval to be negated
* @return a new calendar interval instance with all it parameters negated from the origin one.
+ * @throws ArithmeticException if the result overflows any field value
*/
def negate(interval: CalendarInterval): CalendarInterval = {
+ val months = Math.negateExact(interval.months)
Review comment:
ah, seems better to follow the java naming conversion: `negate` and `negateExact`.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569210266
**[Test build #115847 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115847/testReport)** for PR 26995 at commit [`b679381`](https://github.com/apache/spark/commit/b6793816c16da1eea33dbb9d63e90a9b25394d45).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570123663
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20805/
Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-570123502
**[Test build #116013 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116013/testReport)** for PR 26995 at commit [`aba10b2`](https://github.com/apache/spark/commit/aba10b22bc3928dd88cb20ae0db340413ec0b3b6).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r362402157
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -197,10 +198,22 @@ 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) {
Review comment:
We don't need the `checkException` flag
```
if (expected == null) {
checkExceptionInExpression
} else {
...
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-568644295
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569252206
**[Test build #115851 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115851/testReport)** for PR 26995 at commit [`b679381`](https://github.com/apache/spark/commit/b6793816c16da1eea33dbb9d63e90a9b25394d45).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #26995:
[SPARK-30341][SQL] Overflow check for interval arithmetic operations
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26995: [SPARK-30341][SQL] Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#issuecomment-569264470
Merged build finished. Test PASSed.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org