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/23 08:55:59 UTC
[GitHub] [spark] beliefer opened a new pull request #32311: Accept ANSI intervals by the Sequence expression
beliefer opened a new pull request #32311:
URL: https://github.com/apache/spark/pull/32311
### What changes were proposed in this pull request?
This PR makes `Sequence` expression supports ANSI intervals as step expression.
If the start and stop expression is `TimestampType,` then the step expression could select year-month or day-time interval.
If the start and stop expression is `DateType,` then the step expression must be year-month.
### Why are the changes needed?
Extends the function of `Sequence` expression.
### Does this PR introduce _any_ user-facing change?
'Yes'. Users could use ANSI intervals as step expression for `Sequence` expression.
### How was this patch tested?
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] SparkQA removed a comment on pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826476646
**[Test build #137932 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137932/testReport)** for PR 32311 at commit [`47853e8`](https://github.com/apache/spark/commit/47853e8610a0bf37d377b552cdf6739b3cbeee36).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825556608
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42383/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk closed pull request #32311:
URL: https://github.com/apache/spark/pull/32311
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826492466
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42453/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825561363
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42388/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825561311
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826289677
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42435/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826299378
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137919/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826298870
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42436/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619312636
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2723,28 +2745,94 @@ object Sequence {
}
}
+ private class PeriodSequenceImpl[T: ClassTag]
+ (dt: IntegralType, scale: Long, fromLong: Long => T, zoneId: ZoneId)
+ (implicit num: Integral[T]) extends InternalSequenceBase(dt, scale, fromLong, zoneId) {
Review comment:
I am not sure it is good idea to use `timestampAddInterval()` in `InternalSequenceBase.eval` for adding months to dates. I guess `DateTimeUtils.dateAddMonths()` and `DateTimeUtils.timestampAddInterval` can return different result, especially taking into account that `dateAddMonths()` **does not depend** on the current time zone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826286842
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42435/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826299103
**[Test build #137919 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137919/testReport)** for PR 32311 at commit [`cb361d1`](https://github.com/apache/spark/commit/cb361d17f28ba39c9287ead41d6e3f4d80b40215).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825561363
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42388/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826290119
**[Test build #137914 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137914/testReport)** for PR 32311 at commit [`75562fe`](https://github.com/apache/spark/commit/75562fe02180252499658f713c1f9abb6f04ad44).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826290119
**[Test build #137914 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137914/testReport)** for PR 32311 at commit [`75562fe`](https://github.com/apache/spark/commit/75562fe02180252499658f713c1f9abb6f04ad44).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619791586
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2484,8 +2484,8 @@ case class Flatten(child: Expression) extends UnaryExpression with NullIntoleran
The start and stop expressions must resolve to the same type.
If start and stop expressions resolve to the 'date' or 'timestamp' type
- then the step expression must resolve to the 'interval' type, otherwise to the same type
- as the start and stop expressions.
+ then the step expression must resolve to the 'interval' or 'year-month' or 'day-time' type,
Review comment:
nit:
'year-month' -> 'year-month interval'
'day-time' -> 'day-time interval'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825555803
**[Test build #137858 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137858/testReport)** for PR 32311 at commit [`89a34bc`](https://github.com/apache/spark/commit/89a34bc10513f78dc1b7da06eecb2a26f121905a).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826281705
**[Test build #137913 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137913/testReport)** for PR 32311 at commit [`1d38d02`](https://github.com/apache/spark/commit/1d38d0213861fedcaaf439746e5a4c5e289da408).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619281179
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -744,8 +745,8 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
checkEvaluation(new Sequence(
Literal(Timestamp.valueOf("2018-01-01 00:00:00")),
- Literal(Timestamp.valueOf("2018-01-02 00:00:01")),
Review comment:
Why did you change the test for `CalendarInterval`? It seems it checks a specific case.
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -760,6 +761,15 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
Timestamp.valueOf("2018-01-01 12:00:00"),
Timestamp.valueOf("2018-01-01 00:00:00")))
+ checkEvaluation(new Sequence(
Review comment:
nit: The test is already big enough. Does it make sense to put new checks to a separate test and prepend JIRA id?
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -919,6 +1039,23 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
Date.valueOf("2020-11-01"),
Date.valueOf("2022-04-01")))
+ checkEvaluation(new Sequence(
+ Literal(Date.valueOf("2018-01-01")),
+ Literal(Date.valueOf("2023-01-01")),
+ Literal(Period.of(1, 5, 0))),
+ Seq(
+ Date.valueOf("2018-01-01"),
+ Date.valueOf("2019-06-01"),
+ Date.valueOf("2020-11-01"),
+ Date.valueOf("2022-04-01")))
+
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("2018-01-01")),
+ Literal(Date.valueOf("2018-01-05")),
+ Literal(Period.ofDays(2))),
+ EmptyRow, "sequence step must be a day interval if start and end values are dates")
Review comment:
`a day interval` -> `a day-time interval`. Please, call `DayTimeIntervalType.typeName()` here. Since the types are not stable yet, we can rename the types in the near future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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] beliefer commented on a change in pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619758753
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -744,8 +745,8 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
checkEvaluation(new Sequence(
Literal(Timestamp.valueOf("2018-01-01 00:00:00")),
- Literal(Timestamp.valueOf("2018-01-02 00:00:01")),
Review comment:
I'm dazzled.
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
Good found.
It seems we should change the behavior of `CalenderInterval` so as it could avoid such assumption.
Or the assumption looks like is a bug for the `CalenderInterval`.
If so , could we fix the bug in another PR?
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
In further, microsPerDay just used to estimated length of the sequences. We just need to improve the exception message so as avoid output the assume value.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2723,28 +2745,94 @@ object Sequence {
}
}
+ private class PeriodSequenceImpl[T: ClassTag]
+ (dt: IntegralType, scale: Long, fromLong: Long => T, zoneId: ZoneId)
+ (implicit num: Integral[T]) extends InternalSequenceBase(dt, scale, fromLong, zoneId) {
Review comment:
In fact, the current implement uses `DateTimeUtils.timestampAddInterval` and it's behavior seems good.
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
Good found.
~~It seems we should change the behavior of `CalenderInterval` so as it could avoid such assumption.~~
~~Or the assumption looks like is a bug for the `CalenderInterval`.~~
~~If so , could we fix the bug in another PR?~~
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
In further, `microsPerDay` just used to estimated length of the sequences. We just need to improve the exception message so as avoid output the assume value.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2484,8 +2484,8 @@ case class Flatten(child: Expression) extends UnaryExpression with NullIntoleran
The start and stop expressions must resolve to the same type.
If start and stop expressions resolve to the 'date' or 'timestamp' type
- then the step expression must resolve to the 'interval' type, otherwise to the same type
- as the start and stop expressions.
+ then the step expression must resolve to the 'interval' or 'year-month' or 'day-time' type,
Review comment:
OK
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2561,29 +2568,50 @@ case class Sequence(
TypeCheckResult.TypeCheckSuccess
} else {
TypeCheckResult.TypeCheckFailure(
- s"$prettyName only supports integral, timestamp or date types")
+ s"""
+ |$prettyName uses the wrong parameter type. The parameter type must conform to:
+ |1. The start and stop expressions must resolve to the same type.
+ |2. If start and stop expressions resolve to the 'date' or 'timestamp' type
+ |then the step expression must resolve to the 'interval' or 'year-month' or
Review comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826281705
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826281938
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826299378
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137919/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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] beliefer commented on a change in pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619758753
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -744,8 +745,8 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
checkEvaluation(new Sequence(
Literal(Timestamp.valueOf("2018-01-01 00:00:00")),
- Literal(Timestamp.valueOf("2018-01-02 00:00:01")),
Review comment:
I'm dazzled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826281938
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137913/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826330437
**[Test build #137914 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137914/testReport)** for PR 32311 at commit [`75562fe`](https://github.com/apache/spark/commit/75562fe02180252499658f713c1f9abb6f04ad44).
* 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826332644
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137914/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825708169
**[Test build #137858 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137858/testReport)** for PR 32311 at commit [`89a34bc`](https://github.com/apache/spark/commit/89a34bc10513f78dc1b7da06eecb2a26f121905a).
* 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825556631
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42383/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825726636
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137858/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825726636
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137858/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826307357
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42440/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826298870
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42436/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826299103
**[Test build #137919 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137919/testReport)** for PR 32311 at commit [`cb361d1`](https://github.com/apache/spark/commit/cb361d17f28ba39c9287ead41d6e3f4d80b40215).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826299372
**[Test build #137919 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137919/testReport)** for PR 32311 at commit [`cb361d1`](https://github.com/apache/spark/commit/cb361d17f28ba39c9287ead41d6e3f4d80b40215).
* 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
[GitHub] [spark] SparkQA commented on pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825521215
**[Test build #137853 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137853/testReport)** for PR 32311 at commit [`1e96d54`](https://github.com/apache/spark/commit/1e96d549853f21859edd41433666eb6e37e8096e).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619780862
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
ok. I see.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2723,28 +2745,94 @@ object Sequence {
}
}
+ private class PeriodSequenceImpl[T: ClassTag]
+ (dt: IntegralType, scale: Long, fromLong: Long => T, zoneId: ZoneId)
+ (implicit num: Integral[T]) extends InternalSequenceBase(dt, scale, fromLong, zoneId) {
Review comment:
ok. Let's use `timestampAddInterval` since we don't have an example that could demonstrate any issues caused by `timestampAddInterval()`.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2484,8 +2484,8 @@ case class Flatten(child: Expression) extends UnaryExpression with NullIntoleran
The start and stop expressions must resolve to the same type.
If start and stop expressions resolve to the 'date' or 'timestamp' type
- then the step expression must resolve to the 'interval' type, otherwise to the same type
- as the start and stop expressions.
+ then the step expression must resolve to the 'interval' or 'year-month' or 'day-time' type,
Review comment:
nit:
'year-month' -> 'year-month interval'
'day-time' -> 'day-time interval'
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2561,29 +2568,50 @@ case class Sequence(
TypeCheckResult.TypeCheckSuccess
} else {
TypeCheckResult.TypeCheckFailure(
- s"$prettyName only supports integral, timestamp or date types")
+ s"""
+ |$prettyName uses the wrong parameter type. The parameter type must conform to:
+ |1. The start and stop expressions must resolve to the same type.
+ |2. If start and stop expressions resolve to the 'date' or 'timestamp' type
+ |then the step expression must resolve to the 'interval' or 'year-month' or
Review comment:
Could you call `YearMonthIntervalType.typeName`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619791845
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2561,29 +2568,50 @@ case class Sequence(
TypeCheckResult.TypeCheckSuccess
} else {
TypeCheckResult.TypeCheckFailure(
- s"$prettyName only supports integral, timestamp or date types")
+ s"""
+ |$prettyName uses the wrong parameter type. The parameter type must conform to:
+ |1. The start and stop expressions must resolve to the same type.
+ |2. If start and stop expressions resolve to the 'date' or 'timestamp' type
+ |then the step expression must resolve to the 'interval' or 'year-month' or
Review comment:
Could you call `YearMonthIntervalType.typeName`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826621931
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137932/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826281938
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826332644
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137914/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826281928
**[Test build #137913 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137913/testReport)** for PR 32311 at commit [`1d38d02`](https://github.com/apache/spark/commit/1d38d0213861fedcaaf439746e5a4c5e289da408).
* 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
[GitHub] [spark] SparkQA commented on pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825555803
**[Test build #137858 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137858/testReport)** for PR 32311 at commit [`89a34bc`](https://github.com/apache/spark/commit/89a34bc10513f78dc1b7da06eecb2a26f121905a).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826476646
**[Test build #137932 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137932/testReport)** for PR 32311 at commit [`47853e8`](https://github.com/apache/spark/commit/47853e8610a0bf37d377b552cdf6739b3cbeee36).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826307357
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42440/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826281938
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826281705
**[Test build #137913 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137913/testReport)** for PR 32311 at commit [`1d38d02`](https://github.com/apache/spark/commit/1d38d0213861fedcaaf439746e5a4c5e289da408).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826296104
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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] beliefer commented on a change in pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619780784
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2723,28 +2745,94 @@ object Sequence {
}
}
+ private class PeriodSequenceImpl[T: ClassTag]
+ (dt: IntegralType, scale: Long, fromLong: Long => T, zoneId: ZoneId)
+ (implicit num: Integral[T]) extends InternalSequenceBase(dt, scale, fromLong, zoneId) {
Review comment:
In fact, the current implement uses `DateTimeUtils.timestampAddInterval` and it's behavior seems good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619300327
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
hmm, 28 because we assume 28 days per month there https://github.com/apache/spark/blob/9af338cd685bce26abbc2dd4d077bde5068157b1/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala#L2742 ?
By introducing new interval types, we tried to avoid such assumption.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619781065
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2723,28 +2745,94 @@ object Sequence {
}
}
+ private class PeriodSequenceImpl[T: ClassTag]
+ (dt: IntegralType, scale: Long, fromLong: Long => T, zoneId: ZoneId)
+ (implicit num: Integral[T]) extends InternalSequenceBase(dt, scale, fromLong, zoneId) {
Review comment:
ok. Let's use `timestampAddInterval` since we don't have an example that could demonstrate any issues caused by `timestampAddInterval()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825556631
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42383/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825521215
**[Test build #137853 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137853/testReport)** for PR 32311 at commit [`1e96d54`](https://github.com/apache/spark/commit/1e96d549853f21859edd41433666eb6e37e8096e).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825724105
The error message confuses slightly:
https://github.com/apache/spark/blob/9af338cd685bce26abbc2dd4d077bde5068157b1/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala#L2567
Type checking can fails because of unsupported type of steps even `start` and `stop` have one of: integral, timestamp or date.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825522166
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137853/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826601540
**[Test build #137932 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137932/testReport)** for PR 32311 at commit [`47853e8`](https://github.com/apache/spark/commit/47853e8610a0bf37d377b552cdf6739b3cbeee36).
* 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826492466
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42453/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826305843
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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] beliefer commented on a change in pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619770377
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
Good found.
~~It seems we should change the behavior of `CalenderInterval` so as it could avoid such assumption.~~
~~Or the assumption looks like is a bug for the `CalenderInterval`.~~
~~If so , could we fix the bug in another 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] beliefer commented on a change in pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619780608
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
In further, `microsPerDay` just used to estimated length of the sequences. We just need to improve the exception message so as avoid output the assume value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826492444
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826281705
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825522166
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137853/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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] beliefer commented on a change in pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619770377
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
Good found.
It seems we should change the behavior of `CalenderInterval` so as it could avoid such assumption.
Or the assumption looks like is a bug for the `CalenderInterval`.
If so , could we fix the bug in another 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] beliefer commented on a change in pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619780608
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
In further, microsPerDay just used to estimated length of the sequences. We just need to improve the exception message so as avoid output the assume value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-826571866
@MaxGekk Thanks for you 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] beliefer commented on a change in pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619947604
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2561,29 +2568,50 @@ case class Sequence(
TypeCheckResult.TypeCheckSuccess
} else {
TypeCheckResult.TypeCheckFailure(
- s"$prettyName only supports integral, timestamp or date types")
+ s"""
+ |$prettyName uses the wrong parameter type. The parameter type must conform to:
+ |1. The start and stop expressions must resolve to the same type.
+ |2. If start and stop expressions resolve to the 'date' or 'timestamp' type
+ |then the step expression must resolve to the 'interval' or 'year-month' or
Review comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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] beliefer commented on a change in pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619946808
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2484,8 +2484,8 @@ case class Flatten(child: Expression) extends UnaryExpression with NullIntoleran
The start and stop expressions must resolve to the same type.
If start and stop expressions resolve to the 'date' or 'timestamp' type
- then the step expression must resolve to the 'interval' type, otherwise to the same type
- as the start and stop expressions.
+ then the step expression must resolve to the 'interval' or 'year-month' or 'day-time' type,
Review comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub 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 #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32311:
URL: https://github.com/apache/spark/pull/32311#discussion_r619780862
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
##########
@@ -934,12 +1071,25 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
EmptyRow,
s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
+ checkExceptionInExpression[IllegalArgumentException](
+ new Sequence(
+ Literal(Date.valueOf("1970-01-01")),
+ Literal(Date.valueOf("1970-02-01")),
+ Literal(Period.ofMonths(-1))),
+ EmptyRow,
+ s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}")
Review comment:
ok. I see.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32311: [SPARK-35088][SQL] Accept ANSI intervals by the Sequence expression
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32311:
URL: https://github.com/apache/spark/pull/32311#issuecomment-825522154
**[Test build #137853 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137853/testReport)** for PR 32311 at commit [`1e96d54`](https://github.com/apache/spark/commit/1e96d549853f21859edd41433666eb6e37e8096e).
* 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