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/20 15:55:54 UTC

[GitHub] [spark] MaxGekk opened a new pull request #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

MaxGekk opened a new pull request #32262:
URL: https://github.com/apache/spark/pull/32262


   ### What changes were proposed in this pull request?
   In the PR, I propose to override the `sql` and `toString` methods of the expressions that implement operators over ANSI intervals (`YearMonthIntervalType`/`DayTimeIntervalType`), and replace internal expression class names by operators like `*`, `/` and `-`.
   
   ### Why are the changes needed?
   Proposed methods should make the textual representation of such operators more readable, and potentially parsable by Spark SQL parser. 
   
   ### Does this PR introduce _any_ user-facing change?
   Yes. This can influence on column names.
   
   
   ### How was this patch tested?
   By running existing test suites for interval and datetime expressions, and re-generating the `*.sql` tests:
   ```
   $ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z interval.sql"
   $ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z datetime.sql"
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


   **[Test build #137693 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137693/testReport)** for PR 32262 at commit [`5dced3e`](https://github.com/apache/spark/commit/5dced3ee9ed03a24aa2b0e15ef3a3c6db975c734).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


   **[Test build #137701 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137701/testReport)** for PR 32262 at commit [`9bdfe66`](https://github.com/apache/spark/commit/9bdfe66b61b74f4d5c83984eef1b07c6dd665628).
    * 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


   **[Test build #137693 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137693/testReport)** for PR 32262 at commit [`5dced3e`](https://github.com/apache/spark/commit/5dced3ee9ed03a24aa2b0e15ef3a3c6db975c734).
    * 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] gengliangwang commented on a change in pull request #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2556,6 +2556,9 @@ case class SubtractTimestamps(
         s"new org.apache.spark.unsafe.types.CalendarInterval(0, 0, $end - $start)")
   }
 
+  override def toString: String = s"($left - $right)"
+  override def sql: String = s"(${left.sql} - ${right.sql})"

Review comment:
       Quick question, why `SubtractTimestamps` extends `BinaryExpression` instead of `BinaryOperator`? This reminds me the similar code in `BinaryOperator`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


   **[Test build #137701 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137701/testReport)** for PR 32262 at commit [`9bdfe66`](https://github.com/apache/spark/commit/9bdfe66b61b74f4d5c83984eef1b07c6dd665628).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2556,6 +2556,9 @@ case class SubtractTimestamps(
         s"new org.apache.spark.unsafe.types.CalendarInterval(0, 0, $end - $start)")
   }
 
+  override def toString: String = s"($left - $right)"
+  override def sql: String = s"(${left.sql} - ${right.sql})"

Review comment:
       I don't have good answer to your question. Probably, we could refactor SubtractDates and SubtractTimestamps.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


   **[Test build #137693 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137693/testReport)** for PR 32262 at commit [`5dced3e`](https://github.com/apache/spark/commit/5dced3ee9ed03a24aa2b0e15ef3a3c6db975c734).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2556,6 +2556,9 @@ case class SubtractTimestamps(
         s"new org.apache.spark.unsafe.types.CalendarInterval(0, 0, $end - $start)")
   }
 
+  override def toString: String = s"($left - $right)"
+  override def sql: String = s"(${left.sql} - ${right.sql})"

Review comment:
       Here is the PR https://github.com/apache/spark/pull/32267. Please, review 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] MaxGekk closed pull request #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 pull request #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


   GA passed. Merging to master.
   Thank you, @gengliangwang and @cloud-fan for your 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



---------------------------------------------------------------------
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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


   **[Test build #137701 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137701/testReport)** for PR 32262 at commit [`9bdfe66`](https://github.com/apache/spark/commit/9bdfe66b61b74f4d5c83984eef1b07c6dd665628).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32262: [SPARK-35153][SQL] Make textual representation of ANSI interval operators more readable

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


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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