You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/04/22 13:23:36 UTC

[GitHub] [spark] AngersZhuuuu opened a new pull request #32296: [SPARK-35187][SQL] Failure on minimal interval literal

AngersZhuuuu opened a new pull request #32296:
URL: https://github.com/apache/spark/pull/32296


   ### What changes were proposed in this pull request?
   If the sign '-' inside of interval string, everything is fine after https://github.com/apache/spark/commit/bb5459fb26b9d0d57eadee8b10b7488607eeaeb2:
   ```
   spark-sql> SELECT INTERVAL '-178956970-8' YEAR TO MONTH;
   -178956970-8
   ```
   but the sign outside of interval string is not handled properly:
   ```
   spark-sql> SELECT INTERVAL -'178956970-8' YEAR TO MONTH;
   Error in query:
   Error parsing interval year-month string: integer overflow(line 1, pos 16)
   
   == SQL ==
   SELECT INTERVAL -'178956970-8' YEAR TO MONTH
   ----------------^^^
   ```
   This pr fix this issue
   
   ### Why are the changes needed?
   Fix bug
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   ### How was this patch tested?
   Added UT


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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


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


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

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



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

[GitHub] [spark] SparkQA commented on pull request #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


   **[Test build #137810 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137810/testReport)** for PR 32296 at commit [`b41738f`](https://github.com/apache/spark/commit/b41738f2c74087349de90e2a009cdaddc9d05151).


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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


   **[Test build #137810 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137810/testReport)** for PR 32296 at commit [`b41738f`](https://github.com/apache/spark/commit/b41738f2c74087349de90e2a009cdaddc9d05151).


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

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



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

[GitHub] [spark] AngersZhuuuu commented on pull request #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


   FYI @MaxGekk 


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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Fix failure on the minimal interval literal

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


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


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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


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


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

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



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

[GitHub] [spark] AngersZhuuuu commented on a change in pull request #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2395,13 +2395,22 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
    */
   override def visitUnitToUnitInterval(ctx: UnitToUnitIntervalContext): CalendarInterval = {
     withOrigin(ctx) {
-      val value = Option(ctx.intervalValue.STRING).map(string).getOrElse {
+      val value = Option(ctx.intervalValue.STRING).map(string).map { interval =>
+        val sign = interval.startsWith("-")
+        val minus = ctx.intervalValue().MINUS() != null
+        (sign, minus) match {

Review comment:
       Done

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2395,13 +2395,22 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
    */
   override def visitUnitToUnitInterval(ctx: UnitToUnitIntervalContext): CalendarInterval = {
     withOrigin(ctx) {
-      val value = Option(ctx.intervalValue.STRING).map(string).getOrElse {
+      val value = Option(ctx.intervalValue.STRING).map(string).map { interval =>
+        val sign = interval.startsWith("-")
+        val minus = ctx.intervalValue().MINUS() != null
+        (sign, minus) match {
+          case (true, true) => interval.replaceFirst("-", "")
+          case (false, true) => s"-$interval"
+          case (_, false) => interval
+        }
+      }.getOrElse {
         throw QueryParsingErrors.invalidFromToUnitValueError(ctx.intervalValue)
       }
+      println(value)

Review comment:
       Done




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

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



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

[GitHub] [spark] AmplabJenkins commented on pull request #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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






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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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






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

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



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

[GitHub] [spark] MaxGekk closed pull request #32296: [SPARK-35187][SQL] Fix failure on the minimal interval literal

Posted by GitBox <gi...@apache.org>.
MaxGekk closed pull request #32296:
URL: https://github.com/apache/spark/pull/32296


   


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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


   Kubernetes integration test unable to build dist.
   
   exiting with code: 1
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42340/
   


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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


   **[Test build #137808 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137808/testReport)** for PR 32296 at commit [`03095a1`](https://github.com/apache/spark/commit/03095a1b5f2853d4a736ad10f2859cbefcc9d24e).


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

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



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

[GitHub] [spark] MaxGekk commented on a change in pull request #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2395,13 +2395,22 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
    */
   override def visitUnitToUnitInterval(ctx: UnitToUnitIntervalContext): CalendarInterval = {
     withOrigin(ctx) {
-      val value = Option(ctx.intervalValue.STRING).map(string).getOrElse {
+      val value = Option(ctx.intervalValue.STRING).map(string).map { interval =>
+        val sign = interval.startsWith("-")
+        val minus = ctx.intervalValue().MINUS() != null
+        (sign, minus) match {
+          case (true, true) => interval.replaceFirst("-", "")
+          case (false, true) => s"-$interval"
+          case (_, false) => interval
+        }
+      }.getOrElse {
         throw QueryParsingErrors.invalidFromToUnitValueError(ctx.intervalValue)
       }
+      println(value)

Review comment:
       Could you remove println




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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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






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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


   **[Test build #137808 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137808/testReport)** for PR 32296 at commit [`03095a1`](https://github.com/apache/spark/commit/03095a1b5f2853d4a736ad10f2859cbefcc9d24e).


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

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



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

[GitHub] [spark] MaxGekk commented on a change in pull request #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2395,13 +2395,22 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
    */
   override def visitUnitToUnitInterval(ctx: UnitToUnitIntervalContext): CalendarInterval = {
     withOrigin(ctx) {
-      val value = Option(ctx.intervalValue.STRING).map(string).getOrElse {
+      val value = Option(ctx.intervalValue.STRING).map(string).map { interval =>
+        val sign = interval.startsWith("-")
+        val minus = ctx.intervalValue().MINUS() != null
+        (sign, minus) match {

Review comment:
       nit: I would change the order since minus comes before sign




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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Fix failure on the minimal interval literal

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


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


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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


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


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

For queries about this service, please contact Infrastructure at:
users@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 #32296: [SPARK-35187][SQL] Failure on minimal interval literal

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


   **[Test build #137808 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137808/testReport)** for PR 32296 at commit [`03095a1`](https://github.com/apache/spark/commit/03095a1b5f2853d4a736ad10f2859cbefcc9d24e).
    * This patch **fails Scala style 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