You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/12/16 15:40:13 UTC
[GitHub] [spark] chongguang opened a new pull request #30807: [WIP][SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
chongguang opened a new pull request #30807:
URL: https://github.com/apache/spark/pull/30807
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r551147396
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
Review comment:
thx!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549859856
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
Review comment:
We do need this otherwise unit tests will fail for exemple this one: `sql/testOnly *SQLQueryTestSuite -- -z datetime.sql`
Similar PRs implemented this as well, foe exemple https://github.com/apache/spark/pull/30297/files#diff-da163d97a5f0fc534aad719c4a39eca97116f25bfc05b7d8941b342a3ed96036R1930
I can't really well explain why it is necessary here. Someone can share some idea here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752257570
**[Test build #133508 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133508/testReport)** for PR 30807 at commit [`fad1733`](https://github.com/apache/spark/commit/fad1733a13968400960b4abc0430a1a9fd3bad24).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r551172351
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,73 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-754419033
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133629/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-749021270
Hi @leanken, from my side this PR is ready for review. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30807:
URL: https://github.com/apache/spark/pull/30807
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752763466
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133544/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752225366
**[Test build #133505 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133505/testReport)** for PR 30807 at commit [`15cbe10`](https://github.com/apache/spark/commit/15cbe10507bd7706bfb3dd574a49bcee948a37f3).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752243464
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38094/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-754399718
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38218/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-754399718
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38218/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752018692
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38070/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752266548
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38097/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r550193992
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
+ dayOfWeekTerm: String,
+ sd: String,
+ dowS: String): String = {
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+
+ if (failOnError) {
+ s"""
+ |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |""".stripMargin
+ } else {
+ s"""
+ |int $dayOfWeekTerm;
+ |try {
+ | $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ | ${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |} catch (IllegalArgumentException e) {
+ | ${ev.isNull} = true;
+ |}
+ |""".stripMargin
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
} else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
- s"""
- |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);
- """.stripMargin
+ nextDayGenCode(ev, dayOfWeekTerm, sd, dowS)
Review comment:
One drawback is that we lose the stacktrace which points to `getDayOfWeekFromString`. I think the perf doesn't matter for the failure case and keeping stacktrace seems better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549662644
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1195,27 +1208,46 @@ case class NextDay(startDate: Expression, dayOfWeek: Expression)
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
+ val failOnErrorTerm = ctx.freshName("failOnError")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
+ } else if (DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ val errMsg = input.replace(
+ UTF8String.fromString("""""""), UTF8String.fromString("""\""""))
s"""
- |${ev.isNull} = true;
- """.stripMargin
+ |boolean $failOnErrorTerm = $failOnError;
+ |if ($failOnErrorTerm) {
Review comment:
Hi @cloud-fan ,
I thought about it but didn't find a solution. Here is what I tried:
Instead of doing:
```
s"""
|boolean $failOnErrorTerm = $failOnError;
|if ($failOnErrorTerm) {
| throw new IllegalArgumentException("Illegal input for dayOfWeek: $errMsg");
|} else {
| ${ev.isNull} = true;
|}
|""".stripMargin
```
I was doing this in the beginning:
```
if (failOnError) {
val errMsg = input.replace(
UTF8String.fromString("""""""), UTF8String.fromString("""\""""))
s"""throw new IllegalArgumentException("Illegal input for dayOfWeek: $errMsg");"""
} else {
s"""${ev.isNull} = true;"""
}
```
however the generated code would be:
```
/* 033 */ public java.lang.Object apply(java.lang.Object _i) {
/* 034 */ InternalRow i = (InternalRow) _i;
/* 035 */
/* 036 */
/* 037 */ boolean isNull_0 = true;
/* 038 */ int value_0 = -1;
/* 039 */
/* 040 */
/* 041 */
/* 042 */ isNull_0 = false; // resultCode could change nullability.
/* 043 */ throw new IllegalArgumentException("Illegal input for dayOfWeek: xx");
/* 044 */ isNull_3 = isNull_0;
/* 045 */ value_3 = value_0;
/* 046 */
/* 047 */ // copy all the results into MutableRow
/* 048 */
/* 049 */ if (!isNull_3) {
/* 050 */ mutableRow.setInt(0, value_3);
/* 051 */ } else {
/* 052 */ mutableRow.setNullAt(0);
/* 053 */ }
/* 054 */
/* 055 */ return mutableRow;
/* 056 */ }
```
Which can not be compiled because:
`org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 44, Column 1: failed to compile: org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 44, Column 1: Statement is unreachable`
What's why I put this if-else here. And the generated code becomes:
```
/* 043 */
/* 044 */ boolean failOnError_0 = false;
/* 045 */ if (failOnError_0) {
/* 046 */ throw new IllegalArgumentException("Illegal input for dayOfWeek: xx");
/* 047 */ } else {
/* 048 */ isNull_0 = true;
/* 049 */ }
/* 050 */ isNull_3 = isNull_0;
/* 051 */ value_3 = value_0;
/* 052 */
```
I looked at other expressions where the `throw exception` is always in a try-cache block. However the `DateTimeUtils.getDayOfWeekFromString` function here returns -1 instead of exception for non-valid inputs.
Do you have an idea how to implement without putting `failOnError` into the generated code?
Thanks in advance
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-813242498
ANSI mode is a new feature and we only backport bug fixes to previous branches.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] utkarsh39 edited a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
utkarsh39 edited a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-812604039
@chongguang @cloud-fan Can we port this to `3.1`? I see a lot of expressions support ANSI mode in `3.1` ([reference](https://github.com/apache/spark/blob/branch-3.1/docs/sql-ref-ansi-compliance.md#sql-functions)), so maybe we should also add ANSI support for `NextDay` expression in `3.1`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752234700
**[Test build #133499 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133499/testReport)** for PR 30807 at commit [`372472d`](https://github.com/apache/spark/commit/372472db4dcb2b13820f1e0e775c7fbd1a99ed0d).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752742758
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133538/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752257570
**[Test build #133508 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133508/testReport)** for PR 30807 at commit [`fad1733`](https://github.com/apache/spark/commit/fad1733a13968400960b4abc0430a1a9fd3bad24).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752066614
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133481/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752243480
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38094/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752780600
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38135/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-751662805
Hi guys, could you have a look at this PR?
cc @cloud-fan @leanken
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549757471
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
##########
@@ -684,7 +685,10 @@ object DateTimeUtils {
case "TH" | "THU" | "THURSDAY" => THURSDAY
case "FR" | "FRI" | "FRIDAY" => FRIDAY
case "SA" | "SAT" | "SATURDAY" => SATURDAY
- case _ => -1
+ case _ =>
+ val invalidInput = string.replace(UTF8String.fromString("""""""),
Review comment:
I think `"\""` is more readable than `"""""""`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752018692
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38070/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-754419033
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133629/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549666327
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1195,27 +1208,46 @@ case class NextDay(startDate: Expression, dayOfWeek: Expression)
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
+ val failOnErrorTerm = ctx.freshName("failOnError")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
+ } else if (DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ val errMsg = input.replace(
+ UTF8String.fromString("""""""), UTF8String.fromString("""\""""))
s"""
- |${ev.isNull} = true;
- """.stripMargin
+ |boolean $failOnErrorTerm = $failOnError;
+ |if ($failOnErrorTerm) {
+ | throw new IllegalArgumentException("Illegal input for dayOfWeek: $errMsg");
+ |} else {
+ | ${ev.isNull} = true;
+ |}
+ |""".stripMargin
} else {
val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
- s"""
- |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);
- """.stripMargin
+ s"${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);"
}
} else {
+ val errMsg = dowS.replace(""""""", """\"""")
+ val illegalInputBranch =
+ s"""
+ |boolean $failOnErrorTerm = $failOnError;
+ |if ($failOnErrorTerm) {
Review comment:
Hi @cloud-fan I did the same thing here but it can be done like this:
```
val illegalInputBranch = if (failOnError) {
val errMsg = dowS.replace(""""""", """\"""")
s"""throw new IllegalArgumentException("Illegal input for dayOfWeek: $errMsg");"""
} else {
s"""${ev.isNull} = true;"""
}
s"""
|int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
|if ($dayOfWeekTerm == -1) {
| $illegalInputBranch
|} else {
| ${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
|}
|""".stripMargin
```
It is ok here because the exception is already in an if-else block.
Do you want me to make the change?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752773733
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38135/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r550355049
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
+ dayOfWeekTerm: String,
+ sd: String,
+ dowS: String): String = {
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+
+ if (failOnError) {
+ s"""
+ |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |""".stripMargin
+ } else {
+ s"""
+ |int $dayOfWeekTerm;
+ |try {
+ | $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ | ${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |} catch (IllegalArgumentException e) {
+ | ${ev.isNull} = true;
+ |}
+ |""".stripMargin
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
} else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
- s"""
- |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);
- """.stripMargin
+ nextDayGenCode(ev, dayOfWeekTerm, sd, dowS)
Review comment:
And btw, for this kind of perf regression, do we have automated 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.
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] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549826843
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
##########
@@ -684,7 +685,10 @@ object DateTimeUtils {
case "TH" | "THU" | "THURSDAY" => THURSDAY
case "FR" | "FRI" | "FRIDAY" => FRIDAY
case "SA" | "SAT" | "SATURDAY" => SATURDAY
- case _ => -1
+ case _ =>
+ val invalidInput = string.replace(UTF8String.fromString("""""""),
Review comment:
Finally I think we can use the input directly
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549859856
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
Review comment:
We do need this otherwise unit tests will fail for exemple this one: `sql/testOnly *SQLQueryTestSuite -- -z datetime.sql`
I can't really well explain why it is necessary here. Someone can share some idea here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752742758
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133538/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752272324
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38097/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549939414
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
+ dayOfWeekTerm: String,
+ sd: String,
+ dowS: String): String = {
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+
+ if (failOnError) {
+ s"""
+ |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |""".stripMargin
+ } else {
+ s"""
+ |int $dayOfWeekTerm;
+ |try {
+ | $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ | ${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |} catch (IllegalArgumentException e) {
+ | ${ev.isNull} = true;
+ |}
+ |""".stripMargin
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
} else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
- s"""
- |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);
- """.stripMargin
+ nextDayGenCode(ev, dayOfWeekTerm, sd, dowS)
Review comment:
It's a perf regression that now we need to repeatedly evaluate the `getDayOfWeekFromString`. How about
```
try {
val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
s"${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);"
} catch {
case _: IllegalArgumentException => nextDayGenCode(ev, dayOfWeekTerm, sd, dowS)
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-746550920
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752660173
**[Test build #133538 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133538/testReport)** for PR 30807 at commit [`2365994`](https://github.com/apache/spark/commit/2365994f094ecdea4f9659e2816c03a1d3a4269d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752300732
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133508/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549858054
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
Review comment:
done
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(ev: ExprCode,
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752760107
**[Test build #133544 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133544/testReport)** for PR 30807 at commit [`c55ae65`](https://github.com/apache/spark/commit/c55ae6513104297f57058ad0d3d321dfdaee5117).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752066614
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133481/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752225366
**[Test build #133505 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133505/testReport)** for PR 30807 at commit [`15cbe10`](https://github.com/apache/spark/commit/15cbe10507bd7706bfb3dd574a49bcee948a37f3).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752740169
**[Test build #133538 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133538/testReport)** for PR 30807 at commit [`2365994`](https://github.com/apache/spark/commit/2365994f094ecdea4f9659e2816c03a1d3a4269d).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549609445
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1195,27 +1208,46 @@ case class NextDay(startDate: Expression, dayOfWeek: Expression)
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
+ val failOnErrorTerm = ctx.freshName("failOnError")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
+ } else if (DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ val errMsg = input.replace(
+ UTF8String.fromString("""""""), UTF8String.fromString("""\""""))
s"""
- |${ev.isNull} = true;
- """.stripMargin
+ |boolean $failOnErrorTerm = $failOnError;
+ |if ($failOnErrorTerm) {
Review comment:
can we follow other expressions? We don't need to generate `if-else` in the code, as we know the value of `failOnError` before generating the code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549835354
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
Review comment:
nit: indentation looks wrong here.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(ev: ExprCode,
Review comment:
ditto
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException if !failOnError => null
Review comment:
nit: can replace variable `e` with underscore since it's unused
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
Review comment:
nit: is this useful given that `failOnError` already has a default value?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752660173
**[Test build #133538 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133538/testReport)** for PR 30807 at commit [`2365994`](https://github.com/apache/spark/commit/2365994f094ecdea4f9659e2816c03a1d3a4269d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r551157960
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,73 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
Review comment:
ah, I see. Then can we make it a `def`? Then we don't need to include it when serializing the expression.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r550223300
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
+ dayOfWeekTerm: String,
+ sd: String,
+ dowS: String): String = {
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+
+ if (failOnError) {
+ s"""
+ |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |""".stripMargin
+ } else {
+ s"""
+ |int $dayOfWeekTerm;
+ |try {
+ | $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ | ${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |} catch (IllegalArgumentException e) {
+ | ${ev.isNull} = true;
+ |}
+ |""".stripMargin
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
} else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
- s"""
- |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);
- """.stripMargin
+ nextDayGenCode(ev, dayOfWeekTerm, sd, dowS)
Review comment:
ok I see!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r551138222
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,73 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
Review comment:
nit: we can define it as a local variable inside `nextDayGenCode`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752166885
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38088/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-754320092
**[Test build #133629 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133629/testReport)** for PR 30807 at commit [`e458546`](https://github.com/apache/spark/commit/e458546a7ddbe66a2f69c6d264961334d6d400a2).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752300192
**[Test build #133508 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133508/testReport)** for PR 30807 at commit [`fad1733`](https://github.com/apache/spark/commit/fad1733a13968400960b4abc0430a1a9fd3bad24).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class NextDay(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752138468
**[Test build #133499 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133499/testReport)** for PR 30807 at commit [`372472d`](https://github.com/apache/spark/commit/372472db4dcb2b13820f1e0e775c7fbd1a99ed0d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-753645951
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752763445
**[Test build #133544 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133544/testReport)** for PR 30807 at commit [`c55ae65`](https://github.com/apache/spark/commit/c55ae6513104297f57058ad0d3d321dfdaee5117).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class NextDay(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-751989101
**[Test build #133481 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133481/testReport)** for PR 30807 at commit [`09fc7e8`](https://github.com/apache/spark/commit/09fc7e820aac7a57d57debb2d2870f6ae7aef00f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752763466
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133544/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] utkarsh39 commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
utkarsh39 commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-812604039
@chongguang @cloud-fan Can we port this to `3.1`? I see a lot of expressions support ANSI mode in `3.1` ([reference](https://github.com/apache/spark/blob/branch-3.1/docs/sql-ref-ansi-compliance.md#sql-functions), so maybe we should also add ANSI support for `NextDay` expression in `3.1`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549757676
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,51 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException => if (failOnError) throw e else null
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
- val dayOfWeekTerm = ctx.freshName("dayOfWeek")
- if (dayOfWeek.foldable) {
- val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
- } else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
+ val input = dayOfWeek.eval().asInstanceOf[UTF8String]
Review comment:
We can only do this if `dayOfWeek` is foldable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-754403584
GA passed, merging to master, thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549664984
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1195,27 +1208,46 @@ case class NextDay(startDate: Expression, dayOfWeek: Expression)
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
+ val failOnErrorTerm = ctx.freshName("failOnError")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
+ } else if (DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ val errMsg = input.replace(
+ UTF8String.fromString("""""""), UTF8String.fromString("""\""""))
s"""
- |${ev.isNull} = true;
- """.stripMargin
+ |boolean $failOnErrorTerm = $failOnError;
+ |if ($failOnErrorTerm) {
Review comment:
Maybe we can go modify the function `DateTimeUtils.getDayOfWeekFromString` to make it throw an exception instead of returning -1 and do some try-catch in `NextDay` ?
From a spark point of view it is ok because the `getDayOfWeekFromString` is only used in the `NextDay` expression. But since the `getDayOfWeekFromString` function is not private, maybe users of spark are using it as well. And in that case the change would be breaking for them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752272309
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38097/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549826991
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,51 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException => if (failOnError) throw e else null
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
- val dayOfWeekTerm = ctx.freshName("dayOfWeek")
- if (dayOfWeek.foldable) {
- val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
- } else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
+ val input = dayOfWeek.eval().asInstanceOf[UTF8String]
Review comment:
good point. That actually failed a unit test. Corrected.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,51 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException => if (failOnError) throw e else null
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
- val dayOfWeekTerm = ctx.freshName("dayOfWeek")
- if (dayOfWeek.foldable) {
- val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
- } else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
+ val input = dayOfWeek.eval().asInstanceOf[UTF8String]
+ (dayOfWeek.foldable, input) match {
+ case (true, null) => s"""${ev.isNull} = true;"""
+ case _ =>
+ val dayOfWeekTerm = ctx.freshName("dayOfWeek")
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+ val illegalInputBranch = if (failOnError) { s"""throw e;""" } else {
+ s"""${ev.isNull} = true;"""
+ }
s"""
- |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);
- """.stripMargin
- }
- } else {
- s"""
- |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
- |if ($dayOfWeekTerm == -1) {
- | ${ev.isNull} = true;
- |} else {
- | ${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
- |}
- """.stripMargin
+ |int $dayOfWeekTerm;
+ |try {
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752293385
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133505/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r550223030
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
+ dayOfWeekTerm: String,
+ sd: String,
+ dowS: String): String = {
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+
+ if (failOnError) {
+ s"""
+ |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |""".stripMargin
+ } else {
+ s"""
+ |int $dayOfWeekTerm;
Review comment:
done
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549938599
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
Review comment:
nit: 4 spaces indentation for parameters.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752300732
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133508/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549938493
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
Review comment:
The `FunctionRegistry` requires a standard class constructor, not the `apply` method of case class.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549939519
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
+ dayOfWeekTerm: String,
+ sd: String,
+ dowS: String): String = {
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+
+ if (failOnError) {
+ s"""
+ |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |""".stripMargin
+ } else {
+ s"""
+ |int $dayOfWeekTerm;
Review comment:
can we declare `dayOfWeekTerm` inside the `try-catch`? Seems we don't access `dayOfWeekTerm` out side of the `try-catch`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752239614
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133499/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752672252
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38127/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549826671
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,51 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException => if (failOnError) throw e else null
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-754416745
**[Test build #133629 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133629/testReport)** for PR 30807 at commit [`e458546`](https://github.com/apache/spark/commit/e458546a7ddbe66a2f69c6d264961334d6d400a2).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class NextDay(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752243480
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38094/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752272324
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38097/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r551138554
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
+ dayOfWeekTerm: String,
+ sd: String,
+ dowS: String): String = {
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+
+ if (failOnError) {
+ s"""
+ |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |""".stripMargin
+ } else {
+ s"""
+ |int $dayOfWeekTerm;
+ |try {
+ | $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ | ${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |} catch (IllegalArgumentException e) {
+ | ${ev.isNull} = true;
+ |}
+ |""".stripMargin
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
} else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
- s"""
- |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);
- """.stripMargin
+ nextDayGenCode(ev, dayOfWeekTerm, sd, dowS)
Review comment:
no, people need to catch it manually...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752760107
**[Test build #133544 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133544/testReport)** for PR 30807 at commit [`c55ae65`](https://github.com/apache/spark/commit/c55ae6513104297f57058ad0d3d321dfdaee5117).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752674171
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38127/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-751988459
ok to test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752293136
**[Test build #133505 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133505/testReport)** for PR 30807 at commit [`15cbe10`](https://github.com/apache/spark/commit/15cbe10507bd7706bfb3dd574a49bcee948a37f3).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752673753
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38127/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752138468
**[Test build #133499 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133499/testReport)** for PR 30807 at commit [`372472d`](https://github.com/apache/spark/commit/372472db4dcb2b13820f1e0e775c7fbd1a99ed0d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752239614
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133499/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752166902
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38088/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752236227
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38094/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752780600
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38135/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-753399755
Hi @cloud-fan, all your comments have been taken into account. Do you see other points? Can we merge this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752166902
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38088/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752293385
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133505/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-812614241
Hi @utkarsh39 , just for your information this pull request is a sub-task of https://issues.apache.org/jira/browse/SPARK-33275 ticket which is still open. I don't know how the apache spark project manages the release of tasks and sub-tasks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-751989101
**[Test build #133481 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133481/testReport)** for PR 30807 at commit [`09fc7e8`](https://github.com/apache/spark/commit/09fc7e820aac7a57d57debb2d2870f6ae7aef00f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752780452
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38135/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30807: [WIP][SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-746550920
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-753645951
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r551147173
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,73 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
Review comment:
It is also used outside of `nextDayGenCode`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752062256
**[Test build #133481 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133481/testReport)** for PR 30807 at commit [`09fc7e8`](https://github.com/apache/spark/commit/09fc7e820aac7a57d57debb2d2870f6ae7aef00f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class NextDay(startDate: Expression,`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752674171
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38127/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549758013
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,51 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException => if (failOnError) throw e else null
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
- val dayOfWeekTerm = ctx.freshName("dayOfWeek")
- if (dayOfWeek.foldable) {
- val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
- } else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
+ val input = dayOfWeek.eval().asInstanceOf[UTF8String]
+ (dayOfWeek.foldable, input) match {
+ case (true, null) => s"""${ev.isNull} = true;"""
+ case _ =>
+ val dayOfWeekTerm = ctx.freshName("dayOfWeek")
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+ val illegalInputBranch = if (failOnError) { s"""throw e;""" } else {
+ s"""${ev.isNull} = true;"""
+ }
s"""
- |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);
- """.stripMargin
- }
- } else {
- s"""
- |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
- |if ($dayOfWeekTerm == -1) {
- | ${ev.isNull} = true;
- |} else {
- | ${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
- |}
- """.stripMargin
+ |int $dayOfWeekTerm;
+ |try {
Review comment:
We don't need to generate `try-catch` if `failOnError = true`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549757178
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,51 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException => if (failOnError) throw e else null
Review comment:
nit: `case e: IllegalArgumentException if !failOnError => null`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549698833
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1195,27 +1208,46 @@ case class NextDay(startDate: Expression, dayOfWeek: Expression)
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
+ val failOnErrorTerm = ctx.freshName("failOnError")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
+ } else if (DateTimeUtils.getDayOfWeekFromString(input) == -1) {
+ val errMsg = input.replace(
+ UTF8String.fromString("""""""), UTF8String.fromString("""\""""))
s"""
- |${ev.isNull} = true;
- """.stripMargin
+ |boolean $failOnErrorTerm = $failOnError;
+ |if ($failOnErrorTerm) {
Review comment:
it's private as it's in the `catalyst` package. Your approach SGTM.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-754320092
**[Test build #133629 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133629/testReport)** for PR 30807 at commit [`e458546`](https://github.com/apache/spark/commit/e458546a7ddbe66a2f69c6d264961334d6d400a2).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r550075012
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,69 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(
+ startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case _: IllegalArgumentException if !failOnError => null
+ }
+ }
+
+ private def nextDayGenCode(
+ ev: ExprCode,
+ dayOfWeekTerm: String,
+ sd: String,
+ dowS: String): String = {
+ val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
+
+ if (failOnError) {
+ s"""
+ |int $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |""".stripMargin
+ } else {
+ s"""
+ |int $dayOfWeekTerm;
+ |try {
+ | $dayOfWeekTerm = $dateTimeUtilClass.getDayOfWeekFromString($dowS);
+ | ${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekTerm);
+ |} catch (IllegalArgumentException e) {
+ | ${ev.isNull} = true;
+ |}
+ |""".stripMargin
}
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, (sd, dowS) => {
- val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
val dayOfWeekTerm = ctx.freshName("dayOfWeek")
if (dayOfWeek.foldable) {
val input = dayOfWeek.eval().asInstanceOf[UTF8String]
- if ((input eq null) || DateTimeUtils.getDayOfWeekFromString(input) == -1) {
- s"""
- |${ev.isNull} = true;
- """.stripMargin
+ if (input eq null) {
+ s"""${ev.isNull} = true;"""
} else {
- val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
- s"""
- |${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);
- """.stripMargin
+ nextDayGenCode(ev, dayOfWeekTerm, sd, dowS)
Review comment:
Thx for pointing out.
How about the following which avoids re-evaluating `getDayOfWeekFromString` in case of exception?
```
try {
val dayOfWeekValue = DateTimeUtils.getDayOfWeekFromString(input)
val dateTimeUtilClass = DateTimeUtils.getClass.getName.stripSuffix("$")
s"${ev.value} = $dateTimeUtilClass.getNextDateForDayOfWeek($sd, $dayOfWeekValue);"
} catch {
case e: IllegalArgumentException => if (failOnError) {
val errMsg = e.getMessage.replace("\"", """\"""")
s"""
|if (true) {
| throw new IllegalArgumentException("$errMsg");
|}
|""".stripMargin
} else {
s"""${ev.isNull} = true;"""
}
}
```
The `if (true)` here seems a little strange but I just saw it in other parts of code. So I'm thinking it's ok?
For exemple here: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala#L91
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30807:
URL: https://github.com/apache/spark/pull/30807#issuecomment-752153783
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38088/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] chongguang commented on a change in pull request #30807: [SPARK-33794][SQL]NextDay expression throw runtime IllegalArgumentException when receiving invalid input under ANSI mode
Posted by GitBox <gi...@apache.org>.
chongguang commented on a change in pull request #30807:
URL: https://github.com/apache/spark/pull/30807#discussion_r549858152
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1170,52 +1175,67 @@ case class LastDay(startDate: Expression)
group = "datetime_funcs",
since = "1.5.0")
// scalastyle:on line.size.limit
-case class NextDay(startDate: Expression, dayOfWeek: Expression)
+case class NextDay(startDate: Expression,
+ dayOfWeek: Expression,
+ failOnError: Boolean = SQLConf.get.ansiEnabled)
extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
override def left: Expression = startDate
override def right: Expression = dayOfWeek
+ def this(left: Expression, right: Expression) = this(left, right, SQLConf.get.ansiEnabled)
+
override def inputTypes: Seq[AbstractDataType] = Seq(DateType, StringType)
override def dataType: DataType = DateType
override def nullable: Boolean = true
override def nullSafeEval(start: Any, dayOfW: Any): Any = {
- val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
- if (dow == -1) {
- null
- } else {
+ try {
+ val dow = DateTimeUtils.getDayOfWeekFromString(dayOfW.asInstanceOf[UTF8String])
val sd = start.asInstanceOf[Int]
DateTimeUtils.getNextDateForDayOfWeek(sd, dow)
+ } catch {
+ case e: IllegalArgumentException if !failOnError => null
Review comment:
done, thx
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org