You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/09/02 10:02:50 UTC

[GitHub] [spark] yaooqinn opened a new pull request #29627: [SPARK-32752] Fix parsing aliased typed interval literal issue

yaooqinn opened a new pull request #29627:
URL: https://github.com/apache/spark/pull/29627


   
   ### What changes were proposed in this pull request?
   
   This PR fixes the issue for parsing aliased typed interval values
   
   e.g.  cases below should be all valid ones
   
   ```sql
   
            > select interval '1 day' abc;
   Error in query:
   Error parsing ' 1 day abc' to interval, unrecognized number 'abc'(line 1, pos 16)
   
   == SQL ==
   select interval '1 day' abc
   ----------------^^^
   ```
   
   ```
   spark-sql> select interval '1 day' as abc;
   Error in query:
   Error parsing ' 1 day as' to interval, unrecognized number 'as'(line 1, pos 16)
   
   == SQL ==
   select interval '1 day' as abc
   ----------------^^^
   ```
   
   Additionally, cases like the below one  will be fixed as invalid ones since I touched some test cases.
   ```
   select interval 'interval中文1 day';
   ```
   
   
   ### 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.
   -->
   
   bug fix
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   yes, the cases shown in part I. will be valid cases
   
   ### 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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686299366


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128235/
   Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482078440



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -573,7 +573,7 @@ object IntervalUtils {
     var pointPrefixed: Boolean = false
 
     def trimToNextState(b: Byte, next: ParseState): Unit = {
-      if (b <= ' ') {
+      if (Character.isWhitespace(b)) {

Review comment:
       This worths a new PR.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] github-actions[bot] commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-743928135


   We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
   If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] yaooqinn commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482103624



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -818,6 +818,7 @@ primaryExpression
 
 constant
     : NULL                                                                                     #nullLiteral
+    | INTERVAL STRING AS? identifier?                                                          #maybeAliasedIntervalLiteral

Review comment:
       Before this PR, these cases will be captured by grammar rule `multiUnitsInterval` but not `typedLiteral`, e.g. for interval '1 day' day, the value is `1 day` and the unit is day, which actually should be parsed as `((interval 1 day) as day)`
   
   In this PR, I add a new rule "INTERVAL STRING AS? identifier? " to handle interval typed literals(maybe aliasing) and 1-stingvalue-unit interval together and before the rule `multiUnitsInterval`.
   
   when the "AS" key is not null, the `STRING` must be the whole context of the interval whether it is valid or not, the identifier should be the alias name or else the "AS" itself as alias name.
   
   when the "AS" key is null
       1 and when the `STRING` contains any alphabet, we can assume that the  `STRING` is the whole context of the interval whether it is valid or not. the identifier part will be alias name if present
       2 and when the `STRING` contains no alphabet, e.g. '1' day, '-1' day, '\t1' day, so the identifier here should be the unit part whether it is valid or not.
   
   Also updated in PR desc.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685767210






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] yaooqinn commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482647285



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -818,6 +818,7 @@ primaryExpression
 
 constant
     : NULL                                                                                     #nullLiteral
+    | INTERVAL STRING AS? identifier?                                                          #maybeAliasedIntervalLiteral

Review comment:
       ```sql
   presto> select interval '1 day' day;
   Query 20200903_015817_00008_gec8v failed: Invalid INTERVAL DAY value: 1 day
   
   presto> select interval '1' day day;
         day
   ----------------
    1 00:00:00.000
   (1 row)
   
   Query 20200903_015821_00009_gec8v, FINISHED, 1 node
   Splits: 17 total, 17 done (100.00%)
   0:00 [0 rows, 0B] [0 rows/s, 0B/s]
   
   presto> select interval '1' day;
        _col0
   ----------------
    1 00:00:00.000
   (1 row)
   
   Query 20200903_015825_00010_gec8v, FINISHED, 1 node
   Splits: 17 total, 17 done (100.00%)
   0:00 [0 rows, 0B] [0 rows/s, 0B/s]
   
   presto>
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685766459


   **[Test build #128206 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128206/testReport)** for PR 29627 at commit [`464a09f`](https://github.com/apache/spark/commit/464a09f48249e01d2002e13767bd5c20a8f762e1).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686299302


   **[Test build #128235 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128235/testReport)** for PR 29627 at commit [`0ea121e`](https://github.com/apache/spark/commit/0ea121ee778243fb229ddbf70474c9dc37159e11).
    * This patch **fails due to an unknown error code, -9**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686299355


   Merged build finished. Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685543893


   **[Test build #128206 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128206/testReport)** for PR 29627 at commit [`464a09f`](https://github.com/apache/spark/commit/464a09f48249e01d2002e13767bd5c20a8f762e1).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] yaooqinn commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482084827



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -818,6 +818,7 @@ primaryExpression
 
 constant
     : NULL                                                                                     #nullLiteral
+    | INTERVAL STRING AS? identifier?                                                          #maybeAliasedIntervalLiteral

Review comment:
       OK, I will update pr description after ci passes




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686302082






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686000894


   **[Test build #128207 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128207/testReport)** for PR 29627 at commit [`baf2530`](https://github.com/apache/spark/commit/baf2530eb657aa52b0983a6870324a69de147118).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686288585






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686441159






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] github-actions[bot] closed pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #29627:
URL: https://github.com/apache/spark/pull/29627


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686439935


   **[Test build #128237 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128237/testReport)** for PR 29627 at commit [`0ea121e`](https://github.com/apache/spark/commit/0ea121ee778243fb229ddbf70474c9dc37159e11).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752] Fix parsing aliased typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685545659






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482197715



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -818,6 +818,7 @@ primaryExpression
 
 constant
     : NULL                                                                                     #nullLiteral
+    | INTERVAL STRING AS? identifier?                                                          #maybeAliasedIntervalLiteral

Review comment:
       can you check `interval '1 day' day` in other systems? To see if they also treat '1 day' as the number of days and fail.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686287831


   **[Test build #128235 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128235/testReport)** for PR 29627 at commit [`0ea121e`](https://github.com/apache/spark/commit/0ea121ee778243fb229ddbf70474c9dc37159e11).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686301476


   **[Test build #128237 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128237/testReport)** for PR 29627 at commit [`0ea121e`](https://github.com/apache/spark/commit/0ea121ee778243fb229ddbf70474c9dc37159e11).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686302082






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686185790






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686285255






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686000894


   **[Test build #128207 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128207/testReport)** for PR 29627 at commit [`baf2530`](https://github.com/apache/spark/commit/baf2530eb657aa52b0983a6870324a69de147118).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686285255






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] yaooqinn commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482087400



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -573,7 +573,7 @@ object IntervalUtils {
     var pointPrefixed: Boolean = false
 
     def trimToNextState(b: Byte, next: ParseState): Unit = {
-      if (b <= ' ') {
+      if (Character.isWhitespace(b)) {

Review comment:
       OK, got it




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] yaooqinn commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482647941



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -818,6 +818,7 @@ primaryExpression
 
 constant
     : NULL                                                                                     #nullLiteral
+    | INTERVAL STRING AS? identifier?                                                          #maybeAliasedIntervalLiteral

Review comment:
       presto does not support typed literal for intervals




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] yaooqinn commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482647064



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -818,6 +818,7 @@ primaryExpression
 
 constant
     : NULL                                                                                     #nullLiteral
+    | INTERVAL STRING AS? identifier?                                                          #maybeAliasedIntervalLiteral

Review comment:
       ```
   postgres=# select interval '1 day' day;
    interval
   ----------
    1 day
   (1 row)
   
   postgres=# select interval '1' day day;
   ERROR:  syntax error at or near "day"
   LINE 1: select interval '1' day day;
                                   ^
   postgres=# select interval '1' day;
    interval
   ----------
    1 day
   (1 row)
   ```
   
   
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] yaooqinn commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482103624



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -818,6 +818,7 @@ primaryExpression
 
 constant
     : NULL                                                                                     #nullLiteral
+    | INTERVAL STRING AS? identifier?                                                          #maybeAliasedIntervalLiteral

Review comment:
       Before this PR, these cases will be captured by grammar rule `multiUnitsInterval` but not `typedLiteral`, e.g. for interval '1 day' day, the value is `1 day` and the unit is day, which actually should be parsed as `((interval 1 day) as day)`
   
   In this PR, I add a new rule "INTERVAL STRING AS? identifier? " to handle interval typed literals and 1-value-unit interval together and before the rule `multiUnitsInterval`.
   
   when the "AS" key is not null, the `STRING` must be the whole context of the interval whether it is valid or not, the identifier should be the alias name or else the "AS" itself as alias name.
   
   when the "AS" key is null
       1 and when the `STRING` contains any alphabet, we can assume that the  `STRING` is the whole context of the interval whether it is valid or not. the identifier part will be alias name if present
       2 and when the `STRING` contains no alphabet, e.g. '1' day, '-1' day, '\t1' day, so the identifier here should be the unit part whether it is valid or not.
   
   Also updated in PR desc.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #29627: [SPARK-32752] Fix parsing aliased typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685543893


   **[Test build #128206 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128206/testReport)** for PR 29627 at commit [`464a09f`](https://github.com/apache/spark/commit/464a09f48249e01d2002e13767bd5c20a8f762e1).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685995636






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686185790






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #29627: [SPARK-32752] Fix parsing aliased typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685545659






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] yaooqinn commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686300151


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685995636






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685767228


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128206/
   Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686287831


   **[Test build #128235 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128235/testReport)** for PR 29627 at commit [`0ea121e`](https://github.com/apache/spark/commit/0ea121ee778243fb229ddbf70474c9dc37159e11).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-685767210


   Merged build finished. Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686185219


   **[Test build #128207 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128207/testReport)** for PR 29627 at commit [`baf2530`](https://github.com/apache/spark/commit/baf2530eb657aa52b0983a6870324a69de147118).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686299355






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686301476


   **[Test build #128237 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128237/testReport)** for PR 29627 at commit [`0ea121e`](https://github.com/apache/spark/commit/0ea121ee778243fb229ddbf70474c9dc37159e11).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482078271



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -818,6 +818,7 @@ primaryExpression
 
 constant
     : NULL                                                                                     #nullLiteral
+    | INTERVAL STRING AS? identifier?                                                          #maybeAliasedIntervalLiteral

Review comment:
       can you explain how the bug triggers?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] yaooqinn commented on a change in pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #29627:
URL: https://github.com/apache/spark/pull/29627#discussion_r482103624



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -818,6 +818,7 @@ primaryExpression
 
 constant
     : NULL                                                                                     #nullLiteral
+    | INTERVAL STRING AS? identifier?                                                          #maybeAliasedIntervalLiteral

Review comment:
       Before this PR, these cases will be captured by grammar rule `multiUnitsInterval` but not `typedLiteral`, e.g. for interval '1 day' day, the value is `1 day` and the unit is day, which actually should be parsed as `((interval 1 day) as day)`
   
   In this PR, I add a new rule "INTERVAL STRING AS? identifier? " to handle interval typed literals(maybe aliasing) and 1-value-unit interval together and before the rule `multiUnitsInterval`.
   
   when the "AS" key is not null, the `STRING` must be the whole context of the interval whether it is valid or not, the identifier should be the alias name or else the "AS" itself as alias name.
   
   when the "AS" key is null
       1 and when the `STRING` contains any alphabet, we can assume that the  `STRING` is the whole context of the interval whether it is valid or not. the identifier part will be alias name if present
       2 and when the `STRING` contains no alphabet, e.g. '1' day, '-1' day, '\t1' day, so the identifier here should be the unit part whether it is valid or not.
   
   Also updated in PR desc.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686288585






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #29627: [SPARK-32752][SQL]Fix parsing aliasing typed interval literal issue

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29627:
URL: https://github.com/apache/spark/pull/29627#issuecomment-686441159






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org