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 2020/09/27 05:59:43 UTC
[GitHub] [spark] luluorta opened a new pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
luluorta opened a new pull request #29882:
URL: https://github.com/apache/spark/pull/29882
### What changes were proposed in this pull request?
When an division by zero occurs performing an divide-like operation (Divide, IntegralDivide, Remainder or Pmod), we are returning an incorrect value. Instead, we should throw an exception, as stated in the SQL standard.
### How was this patch tested?
added UT + existing UTs (improved)
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717034813
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717913213
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34967/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718731323
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-706024164
> isn't this covered by ansi mode?
@hvanhovell currently ansi mode only covers the case of `CalendarIntervalType`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718543197
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35005/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717899487
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34967/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717034822
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130317/
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718467693
**[Test build #130401 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130401/testReport)** for PR 29882 at commit [`11188c3`](https://github.com/apache/spark/commit/11188c356113b651435508b9137b5c084d4d990f).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716971453
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34914/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717032670
**[Test build #130312 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130312/testReport)** for PR 29882 at commit [`14c45e9`](https://github.com/apache/spark/commit/14c45e9d029e1165053cd1cbec5573fa9df508b2).
* 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718543274
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699590812
**[Test build #129142 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129142/testReport)** for PR 29882 at commit [`1d425f0`](https://github.com/apache/spark/commit/1d425f09fbd802c72ae69f0b9bddbf295f3401f5).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717048147
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708713764
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34369/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513232098
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -370,12 +417,13 @@ trait DivModLike extends BinaryArithmetic {
${eval2.code}
boolean ${ev.isNull} = false;
$javaType ${ev.value} = ${CodeGenerator.defaultValue(dataType)};
- if (${eval2.isNull} || $isZero) {
+ if (${eval2.isNull}$nullOnErrorCondition) {
${ev.isNull} = true;
} else {
${eval1.code}
if (${eval1.isNull}) {
${ev.isNull} = true;
+ $failOnErrorBranch
Review comment:
We should also change `eval` to match this.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -370,12 +417,13 @@ trait DivModLike extends BinaryArithmetic {
${eval2.code}
boolean ${ev.isNull} = false;
$javaType ${ev.value} = ${CodeGenerator.defaultValue(dataType)};
- if (${eval2.isNull} || $isZero) {
+ if (${eval2.isNull}$nullOnErrorCondition) {
${ev.isNull} = true;
} else {
${eval1.code}
if (${eval1.isNull}) {
${ev.isNull} = true;
+ $failOnErrorBranch
Review comment:
We should also change `eval` method to match 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716976751
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34915/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r502252042
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -320,24 +320,31 @@ trait DivModLike extends BinaryArithmetic {
override def nullable: Boolean = true
- final override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ private def divByZero: Any = {
+ if (checkOverflow) {
Review comment:
I think `nullOnError` is inaccurate for overflow behavior of non-ansi mode, cause it just returns corrupted result rather than null. Maybe a separated flag for `DivModLike` and `Pmod` is better.
Considering that there is also a `DivideInterval.checkOverflow`, shall we open a new PR to fix the flag issue?
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716971465
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513558976
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -560,6 +618,17 @@ case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic {
} else {
s"${eval2.value} == 0"
}
+ val divByZero = if (failOnError) {
Review comment:
This is only used in the `if (!left.nullable && !right.nullable)` branch, we can move it to that branch.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717037578
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34919/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513559156
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -560,6 +618,17 @@ case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic {
} else {
s"${eval2.value} == 0"
}
+ val divByZero = if (failOnError) {
+ "throw new ArithmeticException(\"divide by zero\");"
+ } else {
+ s"${ev.isNull} = true;"
+ }
+ val nullOnErrorCondition = if (failOnError) "" else s" || $isZero"
Review comment:
ditto, move it to the else branch.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513230225
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -560,6 +629,24 @@ case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic {
} else {
s"${eval2.value} == 0"
}
+ val divByZero = if (failOnError) {
Review comment:
nit: `nullSafeDivByZero`
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r514027971
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
##########
@@ -500,4 +524,53 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper
checkEvaluation(e6, 0.toByte)
}
}
+
+ private def testDecimalAndLongType(testFunc: (Int => Any) => Unit): Unit = {
+ testFunc(_.toLong)
+ testFunc(Decimal(_))
+ }
+
+ test("SPARK-33008: division by zero on divide-like operations returns incorrect result") {
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
+ testDecimalAndDoubleType { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(Divide(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Divide(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Divide(one, zero), "divide by zero")
+ }
+
+ testDecimalAndLongType { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(IntegralDivide(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(IntegralDivide(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](
+ IntegralDivide(one, zero), "divide by zero")
+ }
+
+ testNumericDataTypes { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(Remainder(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Remainder(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Remainder(one, zero), "divide by zero")
+
+ checkEvaluation(Pmod(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Pmod(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Pmod(one, zero), "divide by zero")
+ }
+
+ checkExceptionInExpression[ArithmeticException](
+ Pmod(Literal(-7), Literal(0)), "divide by zero")
Review comment:
Yea, I think it's covered by `testNumericDataTypes ...` above, dividing 0 with different 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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708867183
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717935139
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708866496
**[Test build #129763 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129763/testReport)** for PR 29882 at commit [`2f95fbd`](https://github.com/apache/spark/commit/2f95fbd89679f72d8a6604e2af8501b0752f483f).
* 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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717003726
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130310/
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716956210
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34912/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716993401
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716957289
**[Test build #130313 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130313/testReport)** for PR 29882 at commit [`4b1fac4`](https://github.com/apache/spark/commit/4b1fac4c44ffa7acc72a93a363ed6706daf58094).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717901620
**[Test build #130368 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130368/testReport)** for PR 29882 at commit [`5d332af`](https://github.com/apache/spark/commit/5d332af6c21b8624703d9cad51121201b8bc853f).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718091218
**[Test build #130368 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130368/testReport)** for PR 29882 at commit [`5d332af`](https://github.com/apache/spark/commit/5d332af6c21b8624703d9cad51121201b8bc853f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `abstract class BinaryArithmetic extends BinaryOperator with NullIntolerant `
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r504449033
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -320,24 +320,31 @@ trait DivModLike extends BinaryArithmetic {
override def nullable: Boolean = true
- final override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ private def divByZero: Any = {
+ if (checkOverflow) {
Review comment:
done
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513563441
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
##########
@@ -500,4 +524,53 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper
checkEvaluation(e6, 0.toByte)
}
}
+
+ private def testDecimalAndLongType(testFunc: (Int => Any) => Unit): Unit = {
+ testFunc(_.toLong)
+ testFunc(Decimal(_))
+ }
+
+ test("SPARK-33008: division by zero on divide-like operations returns incorrect result") {
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
+ testDecimalAndDoubleType { convert =>
Review comment:
we can simplify the test
```
val operators: Seq[(Expression, Expression) => Expression] = Seq(Divide(_, _), ...)
oeprators.foreach { operator =>
testDecimalAndDoubleType ...
}
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717003715
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699595758
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/129142/
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699590812
**[Test build #129142 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129142/testReport)** for PR 29882 at commit [`1d425f0`](https://github.com/apache/spark/commit/1d425f09fbd802c72ae69f0b9bddbf295f3401f5).
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718880099
@luluorta can you leave a comment in the JIRA ticket so that I can assign it to you?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bart-samwel commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
bart-samwel commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r533355327
##########
File path: sql/core/src/test/resources/sql-tests/inputs/postgreSQL/select_having.sql
##########
@@ -49,6 +49,7 @@ SELECT 1 AS one FROM test_having HAVING a > 1;
SELECT 1 AS one FROM test_having HAVING 1 > 2;
SELECT 1 AS one FROM test_having HAVING 1 < 2;
+-- [SPARK-33008] Spark SQL throws an exception
Review comment:
Interesting! Would you mind filing a ticket for that?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718511232
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35005/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513229769
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -370,12 +417,13 @@ trait DivModLike extends BinaryArithmetic {
${eval2.code}
boolean ${ev.isNull} = false;
$javaType ${ev.value} = ${CodeGenerator.defaultValue(dataType)};
- if (${eval2.isNull} || $isZero) {
+ if (${eval2.isNull}$nullOnErrorCondition) {
${ev.isNull} = true;
} else {
${eval1.code}
if (${eval1.isNull}) {
${ev.isNull} = true;
+ $failOnErrorBranch
Review comment:
nit: 2 space 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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r504486782
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -320,24 +320,31 @@ trait DivModLike extends BinaryArithmetic {
override def nullable: Boolean = true
- final override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ private def divByZero: Any = {
Review comment:
it's only called once, we can inline it.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718731323
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513237676
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Canonicalize.scala
##########
@@ -70,8 +70,10 @@ object Canonicalize {
/** Rearrange expressions that are commutative or associative. */
private def expressionReorder(e: Expression): Expression = e match {
- case a: Add => orderCommutative(a, { case Add(l, r) => Seq(l, r) }).reduce(Add)
- case m: Multiply => orderCommutative(m, { case Multiply(l, r) => Seq(l, r) }).reduce(Multiply)
+ case a @ Add(_, _, c) =>
Review comment:
This needs further updates as we can't reorder consecutive `Add`s with different `failOnError` flags. It's OK for now as the `Add`s in a query must have the same flag. Can we add a TODO 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708867183
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r503142060
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -320,24 +320,31 @@ trait DivModLike extends BinaryArithmetic {
override def nullable: Boolean = true
- final override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ private def divByZero: Any = {
+ if (checkOverflow) {
Review comment:
You are right. How about we flip it as `failOnError`?
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r533845521
##########
File path: sql/core/src/test/resources/sql-tests/inputs/postgreSQL/case.sql
##########
@@ -65,11 +65,11 @@ SELECT '7' AS `None`,
CASE WHEN rand() < 0 THEN 1
END AS `NULL on no matches`;
+-- [SPARK-33008] Spark SQL throws an exception
Review comment:
We should use a new JIRA instead of the current one. Our result is incorrect.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717922777
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34971/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-706024164
> isn't this covered by ansi mode?
@hvanhovell currently ansi mode only covers the case of `CalendarIntervalType`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699595363
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716964537
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717033455
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699595361
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33758/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718092566
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717935110
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34971/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708663318
**[Test build #129763 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129763/testReport)** for PR 29882 at commit [`2f95fbd`](https://github.com/apache/spark/commit/2f95fbd89679f72d8a6604e2af8501b0752f483f).
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716950296
**[Test build #130312 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130312/testReport)** for PR 29882 at commit [`14c45e9`](https://github.com/apache/spark/commit/14c45e9d029e1165053cd1cbec5573fa9df508b2).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716976768
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717048124
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34919/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717901620
**[Test build #130368 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130368/testReport)** for PR 29882 at commit [`5d332af`](https://github.com/apache/spark/commit/5d332af6c21b8624703d9cad51121201b8bc853f).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716964525
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34912/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717880163
**[Test build #130364 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130364/testReport)** for PR 29882 at commit [`6ff1af9`](https://github.com/apache/spark/commit/6ff1af90ed76e54a1b5169ffd90d3dc1e8f93de0).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r502252042
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -320,24 +320,31 @@ trait DivModLike extends BinaryArithmetic {
override def nullable: Boolean = true
- final override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ private def divByZero: Any = {
+ if (checkOverflow) {
Review comment:
I think `nullOnError` is inaccurate for overflow behavior of non-ansi mode, cause it just returns corrupted result rather than null. Maybe a separated flag for `DivModLike` and `Pmod` is better.
Considering that there is also a `DivideInterval.checkOverflow`, shall we open a new PR to fix the flag issue?
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717013900
**[Test build #130317 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130317/testReport)** for PR 29882 at commit [`df9b54f`](https://github.com/apache/spark/commit/df9b54fa7a78bfb8f760e54fcc3e402c0a0bb490).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699595705
**[Test build #129142 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129142/testReport)** for PR 29882 at commit [`1d425f0`](https://github.com/apache/spark/commit/1d425f09fbd802c72ae69f0b9bddbf295f3401f5).
* This patch **fails due to an unknown error code, -9**.
* 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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513564618
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
##########
@@ -500,4 +524,53 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper
checkEvaluation(e6, 0.toByte)
}
}
+
+ private def testDecimalAndLongType(testFunc: (Int => Any) => Unit): Unit = {
+ testFunc(_.toLong)
+ testFunc(Decimal(_))
+ }
+
+ test("SPARK-33008: division by zero on divide-like operations returns incorrect result") {
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
+ testDecimalAndDoubleType { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(Divide(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Divide(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Divide(one, zero), "divide by zero")
+ }
+
+ testDecimalAndLongType { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(IntegralDivide(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(IntegralDivide(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](
+ IntegralDivide(one, zero), "divide by zero")
+ }
+
+ testNumericDataTypes { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(Remainder(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Remainder(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Remainder(one, zero), "divide by zero")
+
+ checkEvaluation(Pmod(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Pmod(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Pmod(one, zero), "divide by zero")
+ }
+
+ checkExceptionInExpression[ArithmeticException](
+ Pmod(Literal(-7), Literal(0)), "divide by zero")
Review comment:
does it provide more test coverage?
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513236699
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -149,9 +153,8 @@ case class Abs(child: Expression)
protected override def nullSafeEval(input: Any): Any = numeric.abs(input)
}
-abstract class BinaryArithmetic extends BinaryOperator with NullIntolerant {
-
- protected val checkOverflow = SQLConf.get.ansiEnabled
+abstract class BinaryArithmetic(failOnError: Boolean)
Review comment:
We can make it a `val` to simplify class extension:
```
abstract class BinaryArithmetic ... {
val failOnError: Boolean
}
case class Divide(..., failOnError: Boolean = ...) extends DivModLike {...}
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bart-samwel commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
bart-samwel commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r531589627
##########
File path: sql/core/src/test/resources/sql-tests/inputs/postgreSQL/select_having.sql
##########
@@ -49,6 +49,7 @@ SELECT 1 AS one FROM test_having HAVING a > 1;
SELECT 1 AS one FROM test_having HAVING 1 > 2;
SELECT 1 AS one FROM test_having HAVING 1 < 2;
+-- [SPARK-33008] Spark SQL throws an exception
Review comment:
I think this test case is wrong -- the next line says that it's to prove that we aren't scanning the table, but the condition `1 < 2` actually _does_ force us to scan the table. I think this test case should have used `HAVING 1 > 2`. Could you correct the test case and remove the comment?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717013900
**[Test build #130317 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130317/testReport)** for PR 29882 at commit [`df9b54f`](https://github.com/apache/spark/commit/df9b54fa7a78bfb8f760e54fcc3e402c0a0bb490).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r512763289
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Average.scala
##########
@@ -77,11 +77,16 @@ case class Average(child: Expression) extends DeclarativeAggregate with Implicit
)
// If all input are nulls, count will be 0 and we will get null after the division.
- override lazy val evaluateExpression = child.dataType match {
- case _: DecimalType =>
- DecimalPrecision.decimalAndDecimal(sum / count.cast(DecimalType.LongDecimal)).cast(resultType)
- case _ =>
- sum.cast(resultType) / count.cast(resultType)
+ // We can't directly use `Divide` as it throws an exception under ansi mode.
Review comment:
done
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716957289
**[Test build #130313 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130313/testReport)** for PR 29882 at commit [`4b1fac4`](https://github.com/apache/spark/commit/4b1fac4c44ffa7acc72a93a363ed6706daf58094).
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718543274
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718024028
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130364/
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708720450
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34369/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699594016
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33758/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717048147
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r504448776
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Average.scala
##########
@@ -77,11 +77,15 @@ case class Average(child: Expression) extends DeclarativeAggregate with Implicit
)
// If all input are nulls, count will be 0 and we will get null after the division.
- override lazy val evaluateExpression = child.dataType match {
- case _: DecimalType =>
- DecimalPrecision.decimalAndDecimal(sum / count.cast(DecimalType.LongDecimal)).cast(resultType)
- case _ =>
- sum.cast(resultType) / count.cast(resultType)
+ override lazy val evaluateExpression = {
+ val eval = child.dataType match {
+ case _: DecimalType =>
+ DecimalPrecision.decimalAndDecimal(sum / count.cast(DecimalType.LongDecimal))
+ .cast(resultType)
+ case _ =>
+ sum.cast(resultType) / count.cast(resultType)
+ }
+ If(EqualTo(count, Literal(0L)), Literal(null, resultType), eval)
Review comment:
done
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717913234
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708720463
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717913234
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #29882:
URL: https://github.com/apache/spark/pull/29882
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699595363
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r504485906
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Average.scala
##########
@@ -77,11 +77,16 @@ case class Average(child: Expression) extends DeclarativeAggregate with Implicit
)
// If all input are nulls, count will be 0 and we will get null after the division.
- override lazy val evaluateExpression = child.dataType match {
- case _: DecimalType =>
- DecimalPrecision.decimalAndDecimal(sum / count.cast(DecimalType.LongDecimal)).cast(resultType)
- case _ =>
- sum.cast(resultType) / count.cast(resultType)
+ // We can't directly use `Divide` as it throws an exception under ansi mode.
Review comment:
can we put `failOnError` as a constructor parameter in `Divide`, like what @leanken did to the statistical aggregate functions? Then here we just need to write `Divide(sum.cast(resultType), count.cast(resultType), failOnError = 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r533036738
##########
File path: sql/core/src/test/resources/sql-tests/inputs/postgreSQL/select_having.sql
##########
@@ -49,6 +49,7 @@ SELECT 1 AS one FROM test_having HAVING a > 1;
SELECT 1 AS one FROM test_having HAVING 1 > 2;
SELECT 1 AS one FROM test_having HAVING 1 < 2;
+-- [SPARK-33008] Spark SQL throws an exception
Review comment:
IIUC, the `HAVING` condition here is performed on the globally grouped result set. I think this query is to prove the row count of the global group does not affect the output of an always-true `HAVING` clause.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699595756
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513229177
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -313,21 +331,31 @@ case class Multiply(left: Expression, right: Expression) extends BinaryArithmeti
override def exactMathMethod: Option[String] = Some("multiplyExact")
}
-// Common base trait for Divide and Remainder, since these two classes are almost identical
-trait DivModLike extends BinaryArithmetic {
+// Common base class for Divide and Remainder, since these two classes are almost identical
+abstract class DivModLike(failOnError: Boolean) extends BinaryArithmetic(failOnError) {
protected def decimalToDataTypeCodeGen(decimalResult: String): String = decimalResult
override def nullable: Boolean = true
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
final override def eval(input: InternalRow): Any = {
+ // evaluate right first as we have a chance to skip left if right is 0
val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ if (input2 == null || (!failOnError && isZero(input2))) {
null
} else {
val input1 = left.eval(input)
if (input1 == null) {
null
+ } else if (isZero(input2)) {
+ // `failOnError` is always true here
+ throw new ArithmeticException("divide by zero")
+ null
Review comment:
no need a return value after `throw`
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -313,21 +331,31 @@ case class Multiply(left: Expression, right: Expression) extends BinaryArithmeti
override def exactMathMethod: Option[String] = Some("multiplyExact")
}
-// Common base trait for Divide and Remainder, since these two classes are almost identical
-trait DivModLike extends BinaryArithmetic {
+// Common base class for Divide and Remainder, since these two classes are almost identical
+abstract class DivModLike(failOnError: Boolean) extends BinaryArithmetic(failOnError) {
protected def decimalToDataTypeCodeGen(decimalResult: String): String = decimalResult
override def nullable: Boolean = true
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
final override def eval(input: InternalRow): Any = {
+ // evaluate right first as we have a chance to skip left if right is 0
val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ if (input2 == null || (!failOnError && isZero(input2))) {
null
} else {
val input1 = left.eval(input)
if (input1 == null) {
null
+ } else if (isZero(input2)) {
+ // `failOnError` is always true here
+ throw new ArithmeticException("divide by zero")
+ null
Review comment:
don't need a return value after `throw`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
leanken commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699959207
@cloud-fan could you please help take a look at this PR，many thanks
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718024013
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718879219
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-700649548
@luluorta isn't this covered by ansi 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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717935139
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716971465
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r514078505
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -560,6 +618,17 @@ case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic {
} else {
s"${eval2.value} == 0"
}
+ val divByZero = if (failOnError) {
+ "throw new ArithmeticException(\"divide by zero\");"
+ } else {
+ s"${ev.isNull} = true;"
+ }
+ val nullOnErrorCondition = if (failOnError) "" else s" || $isZero"
Review comment:
done
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718467693
**[Test build #130401 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130401/testReport)** for PR 29882 at commit [`11188c3`](https://github.com/apache/spark/commit/11188c356113b651435508b9137b5c084d4d990f).
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708663318
**[Test build #129763 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129763/testReport)** for PR 29882 at commit [`2f95fbd`](https://github.com/apache/spark/commit/2f95fbd89679f72d8a6604e2af8501b0752f483f).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r512763548
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -320,24 +320,31 @@ trait DivModLike extends BinaryArithmetic {
override def nullable: Boolean = true
- final override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ private def divByZero: Any = {
Review comment:
done
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -360,22 +372,24 @@ trait DivModLike extends BinaryArithmetic {
boolean ${ev.isNull} = false;
$javaType ${ev.value} = ${CodeGenerator.defaultValue(dataType)};
if ($isZero) {
- ${ev.isNull} = true;
+ $divByZero
} else {
${eval1.code}
${ev.value} = $operation;
}""")
} else {
ev.copy(code = code"""
- ${eval2.code}
Review comment:
done
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -530,24 +544,31 @@ case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic {
override def nullable: Boolean = true
- override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ private def divByZero: Any = {
Review comment:
done
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718024013
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r514008045
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
##########
@@ -500,4 +524,53 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper
checkEvaluation(e6, 0.toByte)
}
}
+
+ private def testDecimalAndLongType(testFunc: (Int => Any) => Unit): Unit = {
+ testFunc(_.toLong)
+ testFunc(Decimal(_))
+ }
+
+ test("SPARK-33008: division by zero on divide-like operations returns incorrect result") {
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
+ testDecimalAndDoubleType { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(Divide(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Divide(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Divide(one, zero), "divide by zero")
+ }
+
+ testDecimalAndLongType { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(IntegralDivide(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(IntegralDivide(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](
+ IntegralDivide(one, zero), "divide by zero")
+ }
+
+ testNumericDataTypes { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(Remainder(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Remainder(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Remainder(one, zero), "divide by zero")
+
+ checkEvaluation(Pmod(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Pmod(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Pmod(one, zero), "divide by zero")
+ }
+
+ checkExceptionInExpression[ArithmeticException](
+ Pmod(Literal(-7), Literal(0)), "divide by zero")
Review comment:
I copied it from `ArithmeticExpressionSuite.test("pmod")`, shall we remove them all?
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716993401
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-700660733
similar to `BinaryArithmetic.checkOverflow`, we should add a new flag in `DivModLike` and `Pmod`, to control this behavior.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r533058089
##########
File path: sql/core/src/test/resources/sql-tests/inputs/postgreSQL/select_having.sql
##########
@@ -49,6 +49,7 @@ SELECT 1 AS one FROM test_having HAVING a > 1;
SELECT 1 AS one FROM test_having HAVING 1 > 2;
SELECT 1 AS one FROM test_having HAVING 1 < 2;
+-- [SPARK-33008] Spark SQL throws an exception
Review comment:
I found that postgres returns correct results without errors for both `1 < 2` and `1 > 2`. While Spark SQL always scans the table and throws exceptions cause its optimizer does not take this case into account.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716964537
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717003382
**[Test build #130310 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130310/testReport)** for PR 29882 at commit [`8d021f2`](https://github.com/apache/spark/commit/8d021f2161c4868cd7a098a11ad56e8c278967ec).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class UnaryMinus(`
* `abstract class BinaryArithmetic(failOnError: Boolean)`
* `case class Add(`
* `case class Subtract(`
* `case class Multiply(`
* `abstract class DivModLike(failOnError: Boolean) extends BinaryArithmetic(failOnError) `
* `case class Divide(`
* `case class Remainder(`
* `case class Pmod(`
* `case class BitwiseAnd(left: Expression, right: Expression) extends BinaryArithmetic(false) `
* `case class BitwiseOr(left: Expression, right: Expression) extends BinaryArithmetic(false) `
* `case class BitwiseXor(left: Expression, right: Expression) extends BinaryArithmetic(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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717034813
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
leanken commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r495886838
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Average.scala
##########
@@ -77,11 +77,15 @@ case class Average(child: Expression) extends DeclarativeAggregate with Implicit
)
// If all input are nulls, count will be 0 and we will get null after the division.
- override lazy val evaluateExpression = child.dataType match {
- case _: DecimalType =>
- DecimalPrecision.decimalAndDecimal(sum / count.cast(DecimalType.LongDecimal)).cast(resultType)
- case _ =>
- sum.cast(resultType) / count.cast(resultType)
+ override lazy val evaluateExpression = {
+ val eval = child.dataType match {
+ case _: DecimalType =>
+ DecimalPrecision.decimalAndDecimal(sum / count.cast(DecimalType.LongDecimal))
+ .cast(resultType)
+ case _ =>
+ sum.cast(resultType) / count.cast(resultType)
+ }
+ If(EqualTo(count, Literal(0L)), Literal(null, resultType), eval)
Review comment:
add comment to explain why we need the If translate here.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -530,24 +544,31 @@ case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic {
override def nullable: Boolean = true
- override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
Review comment:
dup code？ could you refine 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718092566
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-699595756
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r504489335
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -360,22 +372,24 @@ trait DivModLike extends BinaryArithmetic {
boolean ${ev.isNull} = false;
$javaType ${ev.value} = ${CodeGenerator.defaultValue(dataType)};
if ($isZero) {
- ${ev.isNull} = true;
+ $divByZero
} else {
${eval1.code}
${ev.value} = $operation;
}""")
} else {
ev.copy(code = code"""
- ${eval2.code}
Review comment:
we should run `eval2` first, as we have a chance to skip `eval1` if `eval2` is 0.
This is also why we didn't use `nullSafeEval` before, to control the evaluation order.
It would be great if you can help to write this down as code comments, so that other people won't make the same mistake in the future.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718730277
**[Test build #130401 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130401/testReport)** for PR 29882 at commit [`11188c3`](https://github.com/apache/spark/commit/11188c356113b651435508b9137b5c084d4d990f).
* 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] luluorta commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
luluorta commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r514078345
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
##########
@@ -500,4 +524,53 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper
checkEvaluation(e6, 0.toByte)
}
}
+
+ private def testDecimalAndLongType(testFunc: (Int => Any) => Unit): Unit = {
+ testFunc(_.toLong)
+ testFunc(Decimal(_))
+ }
+
+ test("SPARK-33008: division by zero on divide-like operations returns incorrect result") {
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
+ testDecimalAndDoubleType { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(Divide(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Divide(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Divide(one, zero), "divide by zero")
+ }
+
+ testDecimalAndLongType { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(IntegralDivide(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(IntegralDivide(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](
+ IntegralDivide(one, zero), "divide by zero")
+ }
+
+ testNumericDataTypes { convert =>
+ val one = Literal(convert(1))
+ val zero = Literal(convert(0))
+ checkEvaluation(Remainder(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Remainder(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Remainder(one, zero), "divide by zero")
+
+ checkEvaluation(Pmod(Literal.create(null, one.dataType), zero), null)
+ checkEvaluation(Pmod(one, Literal.create(null, zero.dataType)), null)
+ checkExceptionInExpression[ArithmeticException](Pmod(one, zero), "divide by zero")
+ }
+
+ checkExceptionInExpression[ArithmeticException](
+ Pmod(Literal(-7), Literal(0)), "divide by zero")
Review comment:
done
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
##########
@@ -500,4 +524,53 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper
checkEvaluation(e6, 0.toByte)
}
}
+
+ private def testDecimalAndLongType(testFunc: (Int => Any) => Unit): Unit = {
+ testFunc(_.toLong)
+ testFunc(Decimal(_))
+ }
+
+ test("SPARK-33008: division by zero on divide-like operations returns incorrect result") {
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
+ testDecimalAndDoubleType { convert =>
Review comment:
done
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r504489812
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -530,24 +544,31 @@ case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic {
override def nullable: Boolean = true
- override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ private def divByZero: Any = {
Review comment:
similar comments apply to this file as well.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716993407
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130313/
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717033455
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513557495
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -320,15 +339,22 @@ trait DivModLike extends BinaryArithmetic {
override def nullable: Boolean = true
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
final override def eval(input: InternalRow): Any = {
+ // evaluate right first as we have a chance to skip left if right is 0
val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ if (input2 == null || (!failOnError && isZero(input2))) {
null
} else {
val input1 = left.eval(input)
if (input1 == null) {
null
} else {
+ if (isZero(input2)) throw new ArithmeticException("divide by zero")
Review comment:
nit: add a comment `when we reach here, failOnError must bet true.`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716993220
**[Test build #130313 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130313/testReport)** for PR 29882 at commit [`4b1fac4`](https://github.com/apache/spark/commit/4b1fac4c44ffa7acc72a93a363ed6706daf58094).
* 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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716964540
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34912/
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717880163
**[Test build #130364 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130364/testReport)** for PR 29882 at commit [`6ff1af9`](https://github.com/apache/spark/commit/6ff1af90ed76e54a1b5169ffd90d3dc1e8f93de0).
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513231931
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -370,12 +417,13 @@ trait DivModLike extends BinaryArithmetic {
${eval2.code}
boolean ${ev.isNull} = false;
$javaType ${ev.value} = ${CodeGenerator.defaultValue(dataType)};
- if (${eval2.isNull} || $isZero) {
+ if (${eval2.isNull}$nullOnErrorCondition) {
${ev.isNull} = true;
} else {
${eval1.code}
if (${eval1.isNull}) {
${ev.isNull} = true;
+ $failOnErrorBranch
Review comment:
This is hard to read. A simpler way is
```
if (${eval1.isNull}) {
...
} else {
if ($isZero) throw new ArithmeticException("divide by zero"); // add if `failOnError=true`
${ev.value} = $operation;
}
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708720463
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716943464
**[Test build #130310 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130310/testReport)** for PR 29882 at commit [`8d021f2`](https://github.com/apache/spark/commit/8d021f2161c4868cd7a098a11ad56e8c278967ec).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716964755
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34914/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-708720466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34369/
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716970441
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34915/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717033467
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130312/
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716950296
**[Test build #130312 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130312/testReport)** for PR 29882 at commit [`14c45e9`](https://github.com/apache/spark/commit/14c45e9d029e1165053cd1cbec5573fa9df508b2).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-718022918
**[Test build #130364 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130364/testReport)** for PR 29882 at commit [`6ff1af9`](https://github.com/apache/spark/commit/6ff1af90ed76e54a1b5169ffd90d3dc1e8f93de0).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `abstract class BinaryArithmetic extends BinaryOperator with NullIntolerant with Serializable `
* `trait DivModLike extends BinaryArithmetic `
* `case class BitwiseAnd(left: Expression, right: Expression) extends BinaryArithmetic `
* `case class BitwiseOr(left: Expression, right: Expression) extends BinaryArithmetic `
* `case class BitwiseXor(left: Expression, right: Expression) extends BinaryArithmetic `
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513558248
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -530,15 +581,22 @@ case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic {
override def nullable: Boolean = true
- override def eval(input: InternalRow): Any = {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ final override def eval(input: InternalRow): Any = {
+ // evaluate right first as we have a chance to skip left if right is 0
val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ if (input2 == null || (!failOnError && isZero(input2))) {
null
} else {
val input1 = left.eval(input)
if (input1 == null) {
null
} else {
+ if (isZero(input2)) throw new ArithmeticException("divide by zero")
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r513559516
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -560,6 +618,17 @@ case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic {
} else {
s"${eval2.value} == 0"
}
+ val divByZero = if (failOnError) {
+ "throw new ArithmeticException(\"divide by zero\");"
+ } else {
+ s"${ev.isNull} = true;"
+ }
+ val nullOnErrorCondition = if (failOnError) "" else s" || $isZero"
Review comment:
same to the DivModLike
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717003715
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716976768
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-717034645
**[Test build #130317 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130317/testReport)** for PR 29882 at commit [`df9b54f`](https://github.com/apache/spark/commit/df9b54fa7a78bfb8f760e54fcc3e402c0a0bb490).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class BrokenColumnarAdd(`
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29882:
URL: https://github.com/apache/spark/pull/29882#issuecomment-716971469
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34914/
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
---------------------------------------------------------------------
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 #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r496666430
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -320,24 +320,31 @@ trait DivModLike extends BinaryArithmetic {
override def nullable: Boolean = true
- final override def eval(input: InternalRow): Any = {
- val input2 = right.eval(input)
- if (input2 == null || input2 == 0) {
+ private lazy val isZero: Any => Boolean = right.dataType match {
+ case _: DecimalType => x => x.asInstanceOf[Decimal].isZero
+ case _ => x => x == 0
+ }
+
+ private def divByZero: Any = {
+ if (checkOverflow) {
Review comment:
After some more thoughts, I think we don't need to control overflow behavior and divide 0 behavior separatedly. Let's just rename the flag to `nullOnError`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on a change in pull request #29882: [SPARK-33008][SQL] Division by zero on divide-like operations returns incorrect result
Posted by GitBox <gi...@apache.org>.
leanken commented on a change in pull request #29882:
URL: https://github.com/apache/spark/pull/29882#discussion_r495893654
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
##########
@@ -360,22 +372,24 @@ trait DivModLike extends BinaryArithmetic {
boolean ${ev.isNull} = false;
$javaType ${ev.value} = ${CodeGenerator.defaultValue(dataType)};
if ($isZero) {
- ${ev.isNull} = true;
+ $divByZero
Review comment:
make sure test covered with codegen on， FYI
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org