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/03/20 08:52:19 UTC
[GitHub] [spark] cloud-fan opened a new pull request #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
cloud-fan opened a new pull request #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965
<!--
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.
-->
https://github.com/apache/spark/pull/26412 introduced a behavior change that `date_add`/`date_sub` functions can't accept string and double values in the second parameter. This is reasonable as it's error-prone to cast string/double to int at runtime.
However, using string literals as function arguments is very common in SQL databases. To avoid breaking valid use cases that the string literal is indeed an integer, this PR proposes to add ansi_cast for string literal in date_add/date_sub functions. If the string value is not a valid integer, we fail at query compiling time because of constant folding.
### 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.
-->
avoid breaking changes
### Does this PR introduce any user-facing change?
<!--
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 no, write 'No'.
-->
Yes, now 3.0 can run `date_add('2011-11-11', '1')` like 2.4
### 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.
-->
new tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601693101
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601670401
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24811/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r396043033
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
No~ That's the behavior of this PR. This PR extends accidentally *expressions`, too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601738870
**[Test build #120099 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120099/testReport)** for PR 27965 at commit [`94e2fce`](https://github.com/apache/spark/commit/94e2fce05621ac90a3161cb2ae4a2205a2f1db0f).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601617469
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120086/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r396043033
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
No~ The both queries are the same. What I meant was it's the behavior of this PR; this PR extends **expressions**, too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601592961
cc @yaooqinn @maropu @HyukjinKwon
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395790680
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
This causes a behavior difference in arithmatic operations, too. Could you describe the following change in the PR description?
**2.4.5 and 3.0.0-preview2**
```
scala> sql("select (cast('2020-03-28' AS DATE) + '1')").show
org.apache.spark.sql.AnalysisException: cannot resolve '(CAST('2020-03-28' AS DATE) + CAST('1' AS DOUBLE))' due to data type mismatch: differing types in '(CAST('2020-03-28' AS DATE) + CAST('1' AS DOUBLE))' (date and double).; line 1 pos 8;
```
This PR.
```
scala> sql("select (cast('2020-03-28' AS DATE) + '1')").show
+-------------------------------------+
|date_add(CAST(2020-03-28 AS DATE), 1)|
+-------------------------------------+
| 2020-03-29|
+-------------------------------------+
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601594654
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601693117
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24815/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601696282
**[Test build #120099 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120099/testReport)** for PR 27965 at commit [`94e2fce`](https://github.com/apache/spark/commit/94e2fce05621ac90a3161cb2ae4a2205a2f1db0f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395587009
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
Review comment:
How about filing a jira for this TODO then leave a jira ID here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602718324
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395785650
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
This PR seems to have a side effect to change the behavior on a general arithmetic (`1 + date`) instead of `date_add/date_sub` function. Apache Spark 2.4.x doesn't allow those artithemetic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602718332
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120194/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601748700
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395599207
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
Review comment:
Ur, I wrote the design doc though, I didn't file a jira at that time. Probably, @gengliangwang migith have done so?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601739028
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395606758
##########
File path: docs/sql-migration-guide.md
##########
@@ -113,7 +113,7 @@ license: |
### UDFs and Built-in Functions
- - Since Spark 3.0, the `date_add` and `date_sub` functions only accepts int, smallint, tinyint as the 2nd argument, fractional and string types are not valid anymore, e.g. `date_add(cast('1964-05-23' as date), '12.34')` will cause `AnalysisException`. In Spark version 2.4 and earlier, if the 2nd argument is fractional or string value, it will be coerced to int value, and the result will be a date value of `1964-06-04`.
+ - Since Spark 3.0, the `date_add` and `date_sub` functions only accepts int, smallint, tinyint as the 2nd argument, fractional and string types are not valid anymore, e.g. `date_add(cast('1964-05-23' as date), '12.34')` will cause `AnalysisException`. Note that, string literals are still allowed, but Spark will throw Analysis Exception if the string is not a valid integer. In Spark version 2.4 and earlier, if the 2nd argument is fractional or string value, it will be coerced to int value, and the result will be a date value of `1964-06-04`.
Review comment:
`accepts` -> `accept`; `fractional and string types are not valid anymore` seems to be inconsistent with ` string literals are still allowed`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601617345
**[Test build #120086 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120086/testReport)** for PR 27965 at commit [`24df520`](https://github.com/apache/spark/commit/24df520e278be3cb77c8178b74ca7f04617a0aa5).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601594229
**[Test build #120086 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120086/testReport)** for PR 27965 at commit [`24df520`](https://github.com/apache/spark/commit/24df520e278be3cb77c8178b74ca7f04617a0aa5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602528921
**[Test build #120194 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120194/testReport)** for PR 27965 at commit [`bd6be16`](https://github.com/apache/spark/commit/bd6be1620d54e4e5ac65f6f9b930af9ccd604997).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601594664
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24803/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602526030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24907/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395524006
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
+ override protected def coerceTypes(plan: LogicalPlan): LogicalPlan = plan resolveExpressions {
+ // Skip nodes who's children have not been resolved yet.
+ case e if !e.childrenResolved => e
+ case DateAdd(l, r) if r.dataType == StringType && r.foldable =>
+ DateAdd(l, AnsiCast(r, IntegerType))
Review comment:
We need to depend on `AnsiCast` here, even though we can know cast errors in an analysis phase? IMHO an analysis exception with an explicit error message is better than the current number format exception: https://github.com/apache/spark/pull/27965/files#diff-79dd276be45ede6f34e24ad7005b0a7cR279
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602474547
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120184/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601817469
The failures seems to be relevant. Could you take a look?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602474535
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601657783
@yaooqinn the reason why almost all the mainstream databases accept string literals as SQL function arguments is because it's convenient and common. Some BI tools even generate SQL queries with string literals in function arguments regardless of the required type.
Spark is mostly compatible with this use case because our type coercion can implicitly cast string to other types. This is bad as it can lead to runtime null values for invalid string values, and we are trying to improve it under the ANSI mode.
#26412 breaks this use case. I'm OK to add back string type support completely, but I do agree the previous behavior was very confusing. This PR proposes a temporary workaround to support valid string literals only, and I believe we will have a holistic solution of ANSI type coercion in the next release, which handles string literals and invalid string values.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602442692
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601827559
cc @gatorsmile
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602474547
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120184/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601594229
**[Test build #120086 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120086/testReport)** for PR 27965 at commit [`24df520`](https://github.com/apache/spark/commit/24df520e278be3cb77c8178b74ca7f04617a0aa5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601594654
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601670395
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602526030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24907/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601739041
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120099/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602474435
**[Test build #120184 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120184/testReport)** for PR 27965 at commit [`879238c`](https://github.com/apache/spark/commit/879238c6b3ca264a012c8d065cdedf75f61ceb10).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602474535
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602442692
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601617469
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120086/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601748130
**[Test build #120095 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120095/testReport)** for PR 27965 at commit [`9caf98c`](https://github.com/apache/spark/commit/9caf98cdb76c1b3e3600c8674e0dd831072f0231).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601693117
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24815/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601748710
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120095/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602526009
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602442702
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24897/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602718332
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120194/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601693101
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602442702
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24897/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601739028
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395578249
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
+ override protected def coerceTypes(plan: LogicalPlan): LogicalPlan = plan resolveExpressions {
+ // Skip nodes who's children have not been resolved yet.
+ case e if !e.childrenResolved => e
+ case DateAdd(l, r) if r.dataType == StringType && r.foldable =>
+ DateAdd(l, AnsiCast(r, IntegerType))
Review comment:
> Any string->int casts should not be allowed when ansi=true.
Do you mean type coercion? Many databases can cast string to int and they are ANSI-compliant.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601748710
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120095/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395782473
##########
File path: docs/sql-migration-guide.md
##########
@@ -113,7 +113,7 @@ license: |
### UDFs and Built-in Functions
- - Since Spark 3.0, the `date_add` and `date_sub` functions only accepts int, smallint, tinyint as the 2nd argument, fractional and string types are not valid anymore, e.g. `date_add(cast('1964-05-23' as date), '12.34')` will cause `AnalysisException`. In Spark version 2.4 and earlier, if the 2nd argument is fractional or string value, it will be coerced to int value, and the result will be a date value of `1964-06-04`.
+ - Since Spark 3.0, the `date_add` and `date_sub` functions only accept int, smallint, tinyint as the 2nd argument, fractional and non-literal string are not valid anymore, e.g. `date_add(cast('1964-05-23' as date), 12.34)` will cause `AnalysisException`. Note that, string literals are still allowed, but Spark will throw Analysis Exception if the string content is not a valid integer. In Spark version 2.4 and earlier, if the 2nd argument is fractional or string value, it will be coerced to int value, and the result will be a date value of `1964-06-04`.
Review comment:
The new approach looks good.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602528921
**[Test build #120194 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120194/testReport)** for PR 27965 at commit [`bd6be16`](https://github.com/apache/spark/commit/bd6be1620d54e4e5ac65f6f9b930af9ccd604997).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395790680
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
This causes a behavior difference in arithmatic operations, too. Could you describe the following change in the PR description? New one looks reasonable to me.
**2.4.5 and 3.0.0-preview2**
```
scala> sql("select (cast('2020-03-28' AS DATE) + '1')").show
org.apache.spark.sql.AnalysisException: cannot resolve '(CAST('2020-03-28' AS DATE) + CAST('1' AS DOUBLE))' due to data type mismatch: differing types in '(CAST('2020-03-28' AS DATE) + CAST('1' AS DOUBLE))' (date and double).; line 1 pos 8;
```
This PR.
```
scala> sql("select (cast('2020-03-28' AS DATE) + '1')").show
+-------------------------------------+
|date_add(CAST(2020-03-28 AS DATE), 1)|
+-------------------------------------+
| 2020-03-29|
+-------------------------------------+
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601617461
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395585654
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
+ override protected def coerceTypes(plan: LogicalPlan): LogicalPlan = plan resolveExpressions {
+ // Skip nodes who's children have not been resolved yet.
+ case e if !e.childrenResolved => e
+ case DateAdd(l, r) if r.dataType == StringType && r.foldable =>
+ DateAdd(l, AnsiCast(r, IntegerType))
Review comment:
Ah, yes. I rechecked the references of implicit casts in some databases, as you said, the cast seems to be acceptable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601669871
**[Test build #120095 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120095/testReport)** for PR 27965 at commit [`9caf98c`](https://github.com/apache/spark/commit/9caf98cdb76c1b3e3600c8674e0dd831072f0231).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601594664
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24803/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601614952
I'm -1 for such kind of restore.
1. It restores only string literal cases, not the non-literal column cases, so it makes these functions more complicated to understand.
2. the string literals have to be int-castable, this makes the scope even smaller. So, I'm wondering why a user might prefer an integer value with extra single quotes to the integer itself.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602717106
**[Test build #120194 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120194/testReport)** for PR 27965 at commit [`bd6be16`](https://github.com/apache/spark/commit/bd6be1620d54e4e5ac65f6f9b930af9ccd604997).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r396043033
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
No~ That's the behavior of this PR. This PR accidentally extends **expressions**, too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601739041
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120099/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601615016
I think this issue is worth filing a new jira for better traceability.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395785650
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
This PR seems to have a side effect to change the behavior on a general arithmetic (`1 + date`) instead of `date_add/date_sub` function. Apache Spark 2.4.x doesn't allow those artithemetic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395585654
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
+ override protected def coerceTypes(plan: LogicalPlan): LogicalPlan = plan resolveExpressions {
+ // Skip nodes who's children have not been resolved yet.
+ case e if !e.childrenResolved => e
+ case DateAdd(l, r) if r.dataType == StringType && r.foldable =>
+ DateAdd(l, AnsiCast(r, IntegerType))
Review comment:
Ah, yes. I rechecked the references of implicit casts in some databases, as you said, the case seems to be acceptable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395542677
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
+ override protected def coerceTypes(plan: LogicalPlan): LogicalPlan = plan resolveExpressions {
+ // Skip nodes who's children have not been resolved yet.
+ case e if !e.childrenResolved => e
+ case DateAdd(l, r) if r.dataType == StringType && r.foldable =>
+ DateAdd(l, AnsiCast(r, IntegerType))
Review comment:
Also, is the behaviour independent of the ANSI mode? Any string->int casts should not be allowed when `ansi=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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395530639
##########
File path: docs/sql-migration-guide.md
##########
@@ -113,7 +113,7 @@ license: |
### UDFs and Built-in Functions
- - Since Spark 3.0, the `date_add` and `date_sub` functions only accepts int, smallint, tinyint as the 2nd argument, fractional and string types are not valid anymore, e.g. `date_add(cast('1964-05-23' as date), '12.34')` will cause `AnalysisException`. In Spark version 2.4 and earlier, if the 2nd argument is fractional or string value, it will be coerced to int value, and the result will be a date value of `1964-06-04`.
+ - Since Spark 3.0, the `date_add` and `date_sub` functions only accepts int, smallint, tinyint as the 2nd argument, fractional and string types (excluding string literals) are not valid anymore, e.g. `date_add(cast('1964-05-23' as date), '12.34')` will cause `AnalysisException`. In Spark version 2.4 and earlier, if the 2nd argument is fractional or string value, it will be coerced to int value, and the result will be a date value of `1964-06-04`.
Review comment:
In the current behaviour of this PR, `e.g. date_add(cast('1964-05-23' as date), '12.34') will cause AnalysisException` seems to be wrong.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-29774][SQL][followup] support string literal as the second argument
of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-29774][SQL][followup] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601617461
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601669871
**[Test build #120095 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120095/testReport)** for PR 27965 at commit [`9caf98c`](https://github.com/apache/spark/commit/9caf98cdb76c1b3e3600c8674e0dd831072f0231).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602442103
**[Test build #120184 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120184/testReport)** for PR 27965 at commit [`879238c`](https://github.com/apache/spark/commit/879238c6b3ca264a012c8d065cdedf75f61ceb10).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602442103
**[Test build #120184 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120184/testReport)** for PR 27965 at commit [`879238c`](https://github.com/apache/spark/commit/879238c6b3ca264a012c8d065cdedf75f61ceb10).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395595971
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
Review comment:
AFAIK we started the ANSI type coercion (you have a design doc right?). Did we create a JIRA ticket at that time?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601670401
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/24811/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-603002426
merging to master/3.0, thanks for review!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602439366
@dongjoon-hyun good catch! I've fixed it and added 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601696282
**[Test build #120099 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120099/testReport)** for PR 27965 at commit [`94e2fce`](https://github.com/apache/spark/commit/94e2fce05621ac90a3161cb2ae4a2205a2f1db0f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r396043033
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
No~ That's the behavior of this PR. This PR extends accidentally *expressions*, too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395927157
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
You forgot to write a function name in the second query above?
```
scala> sql("select (cast('2020-03-28' AS DATE) + '1')").show
^^^^
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602526009
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395587009
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
Review comment:
nit: how about filing a jira for this TODO then leave a jira ID here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r396043033
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
No~ That's the behavior of this PR. This PR accidentally extends *expressions*, too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-602718324
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601748700
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r396086134
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1045,34 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility as a temporary workaround.
+ * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
Review comment:
Oh, 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #27965:
[SPARK-31205][SQL] support string literal as the second argument of
date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#discussion_r395585654
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
##########
@@ -1043,6 +1044,22 @@ object TypeCoercion {
}
}
}
+
+ /**
+ * A special rule to support string literal as the second argument of date_add/date_sub functions,
+ * to keep backward compatibility.
+ * TODO: revisit the type coercion rules for string.
+ */
+ object StringLiteralCoercion extends TypeCoercionRule {
+ override protected def coerceTypes(plan: LogicalPlan): LogicalPlan = plan resolveExpressions {
+ // Skip nodes who's children have not been resolved yet.
+ case e if !e.childrenResolved => e
+ case DateAdd(l, r) if r.dataType == StringType && r.foldable =>
+ DateAdd(l, AnsiCast(r, IntegerType))
Review comment:
Ah, yes. I rechecked the reference of implicit casts in some databases, as you said, the case seems to be acceptable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27965: [SPARK-31205][SQL]
support string literal as the second argument of date_add/date_sub functions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27965: [SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions
URL: https://github.com/apache/spark/pull/27965#issuecomment-601670395
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org