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 2021/09/01 08:55:34 UTC
[GitHub] [spark] beliefer opened a new pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
beliefer opened a new pull request #33889:
URL: https://github.com/apache/spark/pull/33889
### What changes were proposed in this pull request?
`DivideYMInterval` not consider the ansi mode, which leads some issue:
First, the behavior is not consistent with `Divide`.
Second, the function `try_divide` will not correct.
### Why are the changes needed?
Make consistent behavior and fix bug.
### Does this PR introduce _any_ user-facing change?
'Yes'. This PR fixes bug of `try_divide`.
### How was this patch tested?
New tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943133435
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48721/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937553216
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/143941/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on a change in pull request #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r728673143
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -612,6 +617,13 @@ case class DivideYMInterval(
override def inputTypes: Seq[AbstractDataType] = Seq(YearMonthIntervalType, NumericType)
override def dataType: DataType = YearMonthIntervalType()
+ @transient
+ private lazy val divideByZeroCheck: Any => Unit = right.dataType match {
Review comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910090715
**[Test build #142915 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142915/testReport)** for PR 33889 at commit [`631da9e`](https://github.com/apache/spark/commit/631da9e2e24ce2534fad22ce183ce23803aa2604).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910130655
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47417/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911424584
@MaxGekk
`DivideYMInterval` not consider the ansi mode, which leads some issue:
- `Divide` supports both ansi and non-ansi mode. `DivideYMInterval` only supports ansi mode. If `DivideYMInterval` does not need to support non-ansi mode, then it is very contradictory with the second issue.
- If we use `Divide` with non-ansi mode, when `Analyzer` replacing `Divide` with `DivideYMInterval`, current code ignored the non-ansi mode . The behavior looks so strange.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912222350
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47451/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910182922
**[Test build #142916 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142916/testReport)** for PR 33889 at commit [`218824c`](https://github.com/apache/spark/commit/218824c5f17f568b4f4e7fbdbfe384c3eb67293a).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912424420
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47475/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910183175
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142916/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943017904
> @beliefer any updates?
I'm so sorry for update late.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912404523
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910257676
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47423/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912210960
> Does `DivideDTInterval` have the same issue. If so, fix it in the PR, please. Don't need to spill the changes across two PRs.
I have created another PR. https://github.com/apache/spark/pull/33891
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912459696
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142965/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910224331
**[Test build #142920 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142920/testReport)** for PR 33889 at commit [`0325bfc`](https://github.com/apache/spark/commit/0325bfcc04dd64d2784acfaba19faea088081aef).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937553216
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/143941/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937675751
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48440/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910140621
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47418/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943266810
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144241/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on a change in pull request #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r728673233
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -598,6 +598,11 @@ trait IntervalDivide {
}
}
}
+
+ def divideByZeroCheckCodegen(expr: Expression, value: String): String = expr.dataType match {
Review comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943133435
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48721/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912259597
Can we do it in one PR? It looks overkill to always create 2 small PRs for interval-related changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910183175
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142916/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912312753
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47464/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] MaxGekk commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911255758
> ... to make other functions to compute internal types in ANSI style.
My question is why the functions/expressions that work on ANSI interval should support non-ANSI mode. They are not legacy, what's the reason?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943266810
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144241/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912404523
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47477/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937675751
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48440/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701633790
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -615,15 +615,19 @@ case class DivideYMInterval(
@transient
private lazy val evalFunc: (Int, Any) => Any = right.dataType match {
case LongType => (months: Int, num) =>
+ if (num == 0) throw QueryExecutionErrors.divideByZeroError()
Review comment:
performance-wise, I think try-catch is better for the common case (no error).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912311587
**[Test build #142950 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142950/testReport)** for PR 33889 at commit [`a2a90a0`](https://github.com/apache/spark/commit/a2a90a0277a30a04b136dc05af0b2516c3607a7f).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937505100
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912364164
**[Test build #142976 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142976/testReport)** for PR 33889 at commit [`7df29e5`](https://github.com/apache/spark/commit/7df29e5120f2473579f65e6ddab89cd878ca7ca0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943203412
@cloud-fan @MaxGekk @gengliangwang @HyukjinKwon Thank you for review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910268293
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47423/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer edited a comment on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
beliefer edited a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911424584
@MaxGekk
`DivideYMInterval` not consider the ansi mode, which leads some issue:
- `Divide` supports both ansi and non-ansi mode. `DivideYMInterval` only supports ansi mode. If `DivideYMInterval` does not need to support non-ansi mode, then it is very contradictory with the second issue.
- If we use `Divide` with non-ansi mode, when `Analyzer` replacing `Divide` with `DivideYMInterval`, current code ignored the non-ansi mode . The behavior looks so strange.
Or I think, if ansi interval encounters a `failOnError`, it should throw an unsupported exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912404474
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47477/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912532954
**[Test build #142974 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142974/testReport)** for PR 33889 at commit [`b00d8d8`](https://github.com/apache/spark/commit/b00d8d878984c44cda916598f7c42b87e5a634df).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912307819
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47464/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912313035
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911477038
@MaxGekk in ansi mode, `select 2 / 0` will output
```
org.apache.spark.SparkArithmeticException
divide by zero
```
but `select interval '2' year / 0` will output
```
java.lang.ArithmeticException
/ by zero
```
The behavior looks not inconsistent. I just think select interval '2' year / 0` should output the same exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911251702
I don't feel strongly but just to clarify I think this PR more aims to make other functions to compute internal types in ANSI style. ANSI mode is technically not the compliance of ANSI but more like ANSI-like or ANSI-style IIRC.
WDYT @gengliangwang and @cloud-fan on this support?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912313035
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912287235
**[Test build #142965 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142965/testReport)** for PR 33889 at commit [`823048a`](https://github.com/apache/spark/commit/823048a53d544e4beae38bb85d0e1c42b35140d3).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r702724178
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -612,6 +617,13 @@ case class DivideYMInterval(
override def inputTypes: Seq[AbstractDataType] = Seq(YearMonthIntervalType, NumericType)
override def dataType: DataType = YearMonthIntervalType()
+ @transient
+ private lazy val divideByZeroCheck: Any => Unit = right.dataType match {
Review comment:
can we move this to `IntervalDivide` 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912536113
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142976/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937553216
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-938487107
@beliefer any updates?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] gengliangwang commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911403254
@beliefer could you clarify the exact inconsistency behavior in the PR description?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912534490
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142974/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943265304
**[Test build #144241 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144241/testReport)** for PR 33889 at commit [`d202787`](https://github.com/apache/spark/commit/d202787af21472fa4278e9c4b6161790cbf76b06).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910193574
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142915/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912202941
**[Test build #142950 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142950/testReport)** for PR 33889 at commit [`a2a90a0`](https://github.com/apache/spark/commit/a2a90a0277a30a04b136dc05af0b2516c3607a7f).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] MaxGekk commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911484488
> but select interval '2' year / 0 will output
> java.lang.ArithmeticException
Then let's throw `org.apache.spark.SparkArithmeticException` independently from the SQL config (ANSI or non-ANSI).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] MaxGekk commented on a change in pull request #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701272912
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -615,15 +615,19 @@ case class DivideYMInterval(
@transient
private lazy val evalFunc: (Int, Any) => Any = right.dataType match {
case LongType => (months: Int, num) =>
+ if (num == 0) throw QueryExecutionErrors.divideByZeroError()
Review comment:
We don't need to double check proactively that `num` is 0. `LongMath.divide` will do that for you. Just catch the Java exception, and replace it by Spark's one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910378250
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142920/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912222328
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47451/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912287235
**[Test build #142965 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142965/testReport)** for PR 33889 at commit [`823048a`](https://github.com/apache/spark/commit/823048a53d544e4beae38bb85d0e1c42b35140d3).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943133393
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48721/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912352245
**[Test build #142974 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142974/testReport)** for PR 33889 at commit [`b00d8d8`](https://github.com/apache/spark/commit/b00d8d878984c44cda916598f7c42b87e5a634df).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on a change in pull request #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701537986
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -412,15 +412,21 @@ class IntervalExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
Seq(
- (Period.ofMonths(1), 0) -> "/ by zero",
- (Period.ofMonths(Int.MinValue), 0d) -> "input is infinite or NaN",
+ (Period.ofMonths(1), 0) -> "divide by zero",
+ (Period.ofMonths(Int.MinValue), 0d) -> "divide by zero",
(Period.ofMonths(-100), Float.NaN) -> "input is infinite or NaN"
).foreach { case ((period, num), expectedErrMsg) =>
checkExceptionInExpression[ArithmeticException](
DivideYMInterval(Literal(period), Literal(num)),
expectedErrMsg)
}
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
Review comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910147660
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47418/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910090715
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701633790
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -615,15 +615,19 @@ case class DivideYMInterval(
@transient
private lazy val evalFunc: (Int, Any) => Any = right.dataType match {
case LongType => (months: Int, num) =>
+ if (num == 0) throw QueryExecutionErrors.divideByZeroError()
Review comment:
performance-wise, I think try-catch is better for the common case (no error).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] gengliangwang commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911408114
OK, I got it.
There is similar discussion in https://github.com/apache/spark/pull/33751#issuecomment-899614009
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937609896
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48440/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911285000
@MaxGekk @HyukjinKwon The function `try_divide` with non-ansi mode and `Analyzer` replace `try_divide` with `Divide` and `Analyzer` replace `Divide` with `DivideYMInterval` later. so it will not correct.
Please refer to https://github.com/apache/spark/blob/9c5bcac61ee56fbb271e890cc33f9a983612c5b0/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/TryEval.scala#L116
and
https://github.com/apache/spark/blob/9c5bcac61ee56fbb271e890cc33f9a983612c5b0/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L428
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701063406
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala
##########
@@ -412,15 +412,21 @@ class IntervalExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
Seq(
- (Period.ofMonths(1), 0) -> "/ by zero",
- (Period.ofMonths(Int.MinValue), 0d) -> "input is infinite or NaN",
+ (Period.ofMonths(1), 0) -> "divide by zero",
+ (Period.ofMonths(Int.MinValue), 0d) -> "divide by zero",
(Period.ofMonths(-100), Float.NaN) -> "input is infinite or NaN"
).foreach { case ((period, num), expectedErrMsg) =>
checkExceptionInExpression[ArithmeticException](
DivideYMInterval(Literal(period), Literal(num)),
expectedErrMsg)
}
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
Review comment:
interval divide does not depend on the ansi mode, we don't need to add a UT for 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer edited a comment on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
beliefer edited a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911477038
@MaxGekk in ansi mode, `select 2 / 0` will output
```
org.apache.spark.SparkArithmeticException
divide by zero
```
but `select interval '2' year / 0` will output
```
java.lang.ArithmeticException
/ by zero
```
The behavior looks not inconsistent. I just think `select interval '2' year / 0`` should output the same exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701697934
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -641,6 +649,11 @@ case class DivideYMInterval(
val javaType = CodeGenerator.javaType(dataType)
val months = left.genCode(ctx)
val num = right.genCode(ctx)
+ val checkDivideByZero =
Review comment:
and we can move these util functions to `IntervalDivide`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943025597
**[Test build #144241 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144241/testReport)** for PR 33889 at commit [`d202787`](https://github.com/apache/spark/commit/d202787af21472fa4278e9c4b6161790cbf76b06).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912534490
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142974/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937666136
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48440/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910266601
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47423/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910104726
**[Test build #142916 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142916/testReport)** for PR 33889 at commit [`218824c`](https://github.com/apache/spark/commit/218824c5f17f568b4f4e7fbdbfe384c3eb67293a).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910268293
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47423/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912352245
**[Test build #142974 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142974/testReport)** for PR 33889 at commit [`b00d8d8`](https://github.com/apache/spark/commit/b00d8d878984c44cda916598f7c42b87e5a634df).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912262256
> Can we do it in one PR? It looks overkill to always create 2 small PRs for interval-related changes.
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] MaxGekk commented on a change in pull request #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r703052973
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -612,6 +617,13 @@ case class DivideYMInterval(
override def inputTypes: Seq[AbstractDataType] = Seq(YearMonthIntervalType, NumericType)
override def dataType: DataType = YearMonthIntervalType()
+ @transient
+ private lazy val divideByZeroCheck: Any => Unit = right.dataType match {
Review comment:
I agree to Wenchen, let's move it to the common trait.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937553216
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer edited a comment on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
beliefer edited a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911477038
@MaxGekk in ansi mode, `select 2 / 0` will output
```
org.apache.spark.SparkArithmeticException
divide by zero
```
but `select interval '2' year / 0` will output
```
java.lang.ArithmeticException
/ by zero
```
The behavior looks not inconsistent. I just think `select interval '2' year / 0` should output the same exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on a change in pull request #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701539060
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -615,15 +615,19 @@ case class DivideYMInterval(
@transient
private lazy val evalFunc: (Int, Any) => Any = right.dataType match {
case LongType => (months: Int, num) =>
+ if (num == 0) throw QueryExecutionErrors.divideByZeroError()
Review comment:
`try ... catch ...` looks less elegant than direct judgement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943025597
**[Test build #144241 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144241/testReport)** for PR 33889 at commit [`d202787`](https://github.com/apache/spark/commit/d202787af21472fa4278e9c4b6161790cbf76b06).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943161622
thanks, merging to master/3.2!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-937505100
**[Test build #143941 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143941/testReport)** for PR 33889 at commit [`7df29e5`](https://github.com/apache/spark/commit/7df29e5120f2473579f65e6ddab89cd878ca7ca0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912451599
**[Test build #142965 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142965/testReport)** for PR 33889 at commit [`823048a`](https://github.com/apache/spark/commit/823048a53d544e4beae38bb85d0e1c42b35140d3).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911661928
I think `try_divide` is OK. It will try catch any exception and return null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912222350
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47451/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910147631
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47418/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] MaxGekk edited a comment on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
MaxGekk edited a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911459861
> ... , current code ignored the non-ansi mode . The behavior looks so strange.
I cannot agree with that. ANSI intervals are new feature. We don't have any legacy user code which could require to support non-ANSI behavior. Everywhere in the new expressions, we have already implemented strong (ANSI) mode. Now, you propose to add many new branches for non-ansi implementations. Should be a good reason for overcomplicating the code besides of "consistency" from your point of view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912459696
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142965/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912386464
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47475/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912536113
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142976/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] MaxGekk commented on a change in pull request #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r703054116
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -598,6 +598,11 @@ trait IntervalDivide {
}
}
}
+
+ def divideByZeroCheckCodegen(expr: Expression, value: String): String = expr.dataType match {
Review comment:
nit: Expression is too general as the first parameter, you could pass `DataType`. Up 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer edited a comment on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
beliefer edited a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911424584
@MaxGekk
`DivideYMInterval` not consider the ansi mode, which leads some issue:
- `Divide` supports both ansi and non-ansi mode. `DivideYMInterval` only supports ansi mode. If `DivideYMInterval` does not need to support non-ansi mode, then it is very contradictory with the second issue.
- If we use `Divide` with non-ansi mode, when `Analyzer` replacing `Divide` with `DivideYMInterval`, current code ignored the non-ansi mode . The behavior looks so strange.
Or I think, if ansi interval encounters `failOnError`, it should throw an unsupported exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910377266
**[Test build #142920 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142920/testReport)** for PR 33889 at commit [`0325bfc`](https://github.com/apache/spark/commit/0325bfcc04dd64d2784acfaba19faea088081aef).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] MaxGekk commented on a change in pull request #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701590460
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -615,15 +615,19 @@ case class DivideYMInterval(
@transient
private lazy val evalFunc: (Int, Any) => Any = right.dataType match {
case LongType => (months: Int, num) =>
+ if (num == 0) throw QueryExecutionErrors.divideByZeroError()
Review comment:
hmm, I cannot say that duplicating checks per every case looks so elegant too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910123990
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47417/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on a change in pull request #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701736293
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -641,6 +649,11 @@ case class DivideYMInterval(
val javaType = CodeGenerator.javaType(dataType)
val months = left.genCode(ctx)
val num = right.genCode(ctx)
+ val checkDivideByZero =
Review comment:
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910193231
**[Test build #142915 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142915/testReport)** for PR 33889 at commit [`631da9e`](https://github.com/apache/spark/commit/631da9e2e24ce2534fad22ce183ce23803aa2604).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701693454
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -612,6 +612,13 @@ case class DivideYMInterval(
override def inputTypes: Seq[AbstractDataType] = Seq(YearMonthIntervalType, NumericType)
override def dataType: DataType = YearMonthIntervalType()
+ @transient
+ private lazy val checkFunc: (Any) => Unit = right.dataType match {
Review comment:
nit: `... val divideByZeroCheck: Any => Unit = ...`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912202941
**[Test build #142950 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142950/testReport)** for PR 33889 at commit [`a2a90a0`](https://github.com/apache/spark/commit/a2a90a0277a30a04b136dc05af0b2516c3607a7f).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910193574
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142915/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910135983
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47417/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910135983
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47417/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912534700
**[Test build #142976 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142976/testReport)** for PR 33889 at commit [`7df29e5`](https://github.com/apache/spark/commit/7df29e5120f2473579f65e6ddab89cd878ca7ca0).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910224331
**[Test build #142920 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142920/testReport)** for PR 33889 at commit [`0325bfc`](https://github.com/apache/spark/commit/0325bfcc04dd64d2784acfaba19faea088081aef).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910378250
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142920/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #33889:
URL: https://github.com/apache/spark/pull/33889
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-943088325
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48721/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912426041
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47475/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912218948
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47451/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-910147660
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47418/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-912364164
**[Test build #142976 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142976/testReport)** for PR 33889 at commit [`7df29e5`](https://github.com/apache/spark/commit/7df29e5120f2473579f65e6ddab89cd878ca7ca0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] beliefer commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911240647
ping @MaxGekk @cloud-fan @gengliangwang
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33889: [SPARK-36632][SQL] DivideYMInterval and DivideDTInterval should throw the same exception when divide by zero.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33889:
URL: https://github.com/apache/spark/pull/33889#discussion_r701695385
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala
##########
@@ -641,6 +649,11 @@ case class DivideYMInterval(
val javaType = CodeGenerator.javaType(dataType)
val months = left.genCode(ctx)
val num = right.genCode(ctx)
+ val checkDivideByZero =
Review comment:
we can add
```
private def divideByZeroCheckCodegen(value: String): String = right.dataType match {
case _: DecimalType => "if ($value.isZero()) throw ..."
case _ => "if ($value == 0) throw ..."
}
```
to avoid duplicate code 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] MaxGekk commented on pull request #33889: [SPARK-36632][SQL] DivideYMInterval should consider ansi mode.
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #33889:
URL: https://github.com/apache/spark/pull/33889#issuecomment-911459861
> ... , current code ignored the non-ansi mode . The behavior looks so strange.
I cannot agree with that. ANSI intervals are new feature. We don't have any legacy user code which could require to support non-ANSI behavior. Everywhere in the new expressions, we have already implemented strong (ANSI mode). Now, you propose to add many new branches for non-ansi implementations. Should be a good reason for overcomplicating the code besides of "consistency" from your point of view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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