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/11/05 11:27:52 UTC
[GitHub] [spark] Peng-Lei opened a new pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Peng-Lei opened a new pull request #34494:
URL: https://github.com/apache/spark/pull/34494
### What changes were proposed in this pull request?
Add cast `YearMonthIntervalType` to `NumericType`
requirement: `YearMonthIntervalType` just have one unit and `Numeric` is `IntegralType`
cast rule:
1. `YearMonthIntervalType(YEAR)` to `ByteType`: `value / 12`, it may cause overflow
eg: `cast 'INTERVAL 1 YEAR' to ByteTpe == 1.toByte`
2. `YearMonthIntervalType(YEAR)` to `ShortType`: `value / 12`, it may cause overflow
3. `YearMonthIntervalType(YEAR)` to `IntegerType`: `value / 12`
4. `YearMonthIntervalType(YEAR)` to `LongType`: `value / 12`
5. `YearMonthIntervalType(MONTH)` to `ByteType`: `value`, it may cause overflow
6. `YearMonthIntervalType(MONTH)` to `ShortType`: `value`, it may cause overflow
7. `YearMonthIntervalType(MONTH)` to `IntegerType`: `value`
8. `YearMonthIntervalType(MONTH)` to `LongType`: `value`
Add cast `NumericType` to `YearMonthIntervalType`
requirement: `YearMonthIntervalType` just have one unit and `Numeric` is `IntegralType`
cast rule:
1. `ByteType` to `YearMonthIntervalType(YEAR)`: `value * 12`
eg: `cast 1.toByte to YearMonthIntervalType(YEAR) == 'INTERVAL 1 YEAR'`
2. `ShortType` to `YearMonthIntervalType(YEAR)`: `value * 12`
3. `IntegerType` to `YearMonthIntervalType(YEAR)`: `value * 12`, it may cause overflow
4. `LongType` to `YearMonthIntervalType(YEAR)`: `value * 12`, it may cause overflow
5. `ByteType` to `YearMonthIntervalType(MONTH)`: `value`
6. `ShortType` to `YearMonthIntervalType(MONTH)`: `value`
7. `IntegerType` to `YearMonthIntervalType(MONTH)`: `value`
8. `LongType` to `YearMonthIntervalType(MONTH)`: `value`, it may cause overflow
### Why are the changes needed?
According to 2011 Standards
7) If TD is an interval and SD is exact numeric, then TD shall contain only a single <primary datetime field>.
8) If TD is exact numeric and SD is an interval, then SD shall contain only a single <primary datetime field>.
### Does this PR introduce _any_ user-facing change?
Yes, user can use cast function between YearMonthIntervalType to NumericType
### How was this patch tested?
add ut testcase
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-970020691
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49739/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-970179510
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145269/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749941998
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,50 @@ object IntervalUtils {
}
intervalString
}
+
+ def integralToYearMonthInterval(v: Long, endField: Byte): Int = {
+ if (v != v.toInt) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ endField match {
+ case YEAR =>
+ try {
+ Math.multiplyExact(v.toInt, MONTHS_PER_YEAR)
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ case MONTH => v.toInt
+ }
+ }
+
+ def yearMonthIntervalToInt(v: Int, endFiled: Byte): Int = {
+ endFiled match {
+ case YEAR => v / MONTHS_PER_YEAR
+ case MONTH => v
+ }
+ }
+
+ def integralToDayTimeInterval(v: Long, endField: Byte): Long = {
+ try {
+ endField match {
+ case DAY => Math.multiplyExact(v, MICROS_PER_DAY)
+ case HOUR => Math.multiplyExact(v, MICROS_PER_HOUR)
+ case MINUTE => Math.multiplyExact(v, MICROS_PER_MINUTE)
+ case SECOND => Math.multiplyExact(v, MICROS_PER_SECOND)
+ }
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, DT(endField).catalogString)
+ }
+ }
+
+ def dayTimeIntervalToLong(v: Long, endFiled: Byte): Long = {
+ endFiled match {
+ case DAY => v / MICROS_PER_DAY
+ case HOUR => v / MICROS_PER_HOUR
+ case MINUTE => v / MICROS_PER_MINUTE
+ case SECOND => v / MICROS_PER_SECOND
+ }
+ }
Review comment:
I thought about it. Finally, as you said, the corresponding convert function is defined even if it will be used infrequently. So the codegen just call the corresponding convert function that is better match and the overflow check is performed only when necessary for the corresponding convert function.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-969912548
**[Test build #145269 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145269/testReport)** for PR 34494 at commit [`686a122`](https://github.com/apache/spark/commit/686a122b3759e849d310ff4cca65fc8389d83a16).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971350201
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145310/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971540487
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145321/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-972395639
@cloud-fan Thank you very much for your very patient 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971442816
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49798/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-962049620
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144931/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966047461
**[Test build #145095 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145095/testReport)** for PR 34494 at commit [`004cc67`](https://github.com/apache/spark/commit/004cc67960b2f46e57bb92f2cced2c22caaf48a1).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966248468
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49573/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966212108
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49564/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966073837
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49559/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749059657
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -589,6 +594,9 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToDTInterval(s, it.startField, it.endField))
case _: DayTimeIntervalType => buildCast[Long](_, s =>
IntervalUtils.durationToMicros(IntervalUtils.microsToDuration(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
seems better to do
```
case x: IntegralType =>
assert(it.startField == it.endField)
...
```
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961830498
**[Test build #144931 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144931/testReport)** for PR 34494 at commit [`bf9de6c`](https://github.com/apache/spark/commit/bf9de6c5841faf74719ef7c732ce5b6b2e7e6001).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961905452
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r743613019
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +603,24 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
We can move the code to `IntervalUtils` and share the code between codegen and interpreted code paths.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750878715
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1580,6 +1647,59 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
}
}
+ private[this] def castDayTimeIntervalToIntegralTypeCode(
+ ctx: CodegenContext,
+ endFiled: Byte,
+ integralType: String,
+ catalogType: String): CastFunction = {
+ val util = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
+ integralType match {
Review comment:
I think the caller side should just pass `Int`, `Long`, etc. and here we can simply write `$util.dayTimeInterval$integralType`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971242245
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49781/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750947143
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,115 @@ object IntervalUtils {
}
intervalString
}
+
+ def intToYearMonthInterval(v: Int, endField: Byte): Int = {
+ endField match {
+ case YEAR =>
+ try {
+ Math.multiplyExact(v, MONTHS_PER_YEAR)
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ case MONTH => v
+ }
+ }
+
+ def longToYearMonthInterval(v: Long, endField: Byte): Int = {
+ val vInt = v.toInt
+ if (v != vInt) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ intToYearMonthInterval(vInt, endField)
+ }
+
+ def yearMonthIntervalToInt(v: Int, endFiled: Byte): Int = {
+ endFiled match {
+ case YEAR => v / MONTHS_PER_YEAR
+ case MONTH => v
+ }
+ }
+
+ def yearMonthIntervalToShort(v: Int, endFiled: Byte): Short = {
+ val vInt = yearMonthIntervalToInt(v, endFiled)
+ val vShort = vInt.toShort
+ if (vInt != vShort) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, ShortType.catalogString);
Review comment:
Seems more intuitive that print `v` with interval format. A small burden is the need to add a parameter `startField` to build the interval format. eg `INTERVAL 'v' startField TO endField`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971350201
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145310/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971400531
**[Test build #145327 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145327/testReport)** for PR 34494 at commit [`c974930`](https://github.com/apache/spark/commit/c974930a451538cae2a935d20afe644cb34ec596).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971539025
**[Test build #145321 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145321/testReport)** for PR 34494 at commit [`276673a`](https://github.com/apache/spark/commit/276673a993319c18bd2cf4e750396a24511b6696).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966336869
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145104/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-969124794
**[Test build #145235 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145235/testReport)** for PR 34494 at commit [`19f275a`](https://github.com/apache/spark/commit/19f275a50d026ff6e227b716045eba4b7b95ffae).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-969137581
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145235/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961821841
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966045384
**[Test build #145090 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145090/testReport)** for PR 34494 at commit [`faeffbc`](https://github.com/apache/spark/commit/faeffbc8041484c5c05d97aafb964ba8cef561cf).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961859117
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49402/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-962045894
**[Test build #144931 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144931/testReport)** for PR 34494 at commit [`bf9de6c`](https://github.com/apache/spark/commit/bf9de6c5841faf74719ef7c732ce5b6b2e7e6001).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r743613019
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +603,24 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
We can move the code to `IntervalUtils` and share the code between codegen and interpreted code paths.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750264114
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1580,6 +1675,59 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
}
}
+ private[this] def castDayTimeIntervalToIntegralTypeCode(
+ ctx: CodegenContext,
+ endFiled: Byte,
+ integralType: String,
+ catalogType: String): CastFunction = {
+ val util = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
+ integralType match {
Review comment:
is it just `$evPrim = $util.dayTimeIntervalTo$integralType($c, (byte)$endFiled);`?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971314950
**[Test build #145322 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145322/testReport)** for PR 34494 at commit [`2ac9d11`](https://github.com/apache/spark/commit/2ac9d11ae8db628442f6b8f7698f881012a711db).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968522387
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49685/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968842517
update as follow：
- IntegralType to DayTimeIntervalType
1. `byte` `short` => DayTimeIntervalType
no need check overflow.
2. `int` `long` => DayTimeIntervalType
need check overflow.
- DayTimeIntervalType to IntegralType
1. only `DayTimeIntervalType ` => `long`
no need check overflow.
2. others need check overflow.
- IntegralType to YearMonthIntervalType
1. `byte` `short` => YearMonthIntervalType
no overflow, no check
2. `int` `long` => YearMonthIntervalType
maybe overflow, need check
- YearMonthIntervalType to IntegralType
1. `YearMonthIntervalType` => `byte` or `short`
need check overflow.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749399322
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,50 @@ object IntervalUtils {
}
intervalString
}
+
+ def integralToYearMonthInterval(v: Long, endField: Byte): Int = {
+ if (v != v.toInt) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ endField match {
+ case YEAR =>
+ try {
+ Math.multiplyExact(v.toInt, MONTHS_PER_YEAR)
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ case MONTH => v.toInt
+ }
+ }
+
+ def yearMonthIntervalToInt(v: Int, endFiled: Byte): Int = {
+ endFiled match {
+ case YEAR => v / MONTHS_PER_YEAR
+ case MONTH => v
+ }
+ }
+
+ def integralToDayTimeInterval(v: Long, endField: Byte): Long = {
+ try {
+ endField match {
+ case DAY => Math.multiplyExact(v, MICROS_PER_DAY)
+ case HOUR => Math.multiplyExact(v, MICROS_PER_HOUR)
+ case MINUTE => Math.multiplyExact(v, MICROS_PER_MINUTE)
+ case SECOND => Math.multiplyExact(v, MICROS_PER_SECOND)
+ }
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, DT(endField).catalogString)
+ }
+ }
+
+ def dayTimeIntervalToLong(v: Long, endFiled: Byte): Long = {
+ endFiled match {
+ case DAY => v / MICROS_PER_DAY
+ case HOUR => v / MICROS_PER_HOUR
+ case MINUTE => v / MICROS_PER_MINUTE
+ case SECOND => v / MICROS_PER_SECOND
+ }
+ }
Review comment:
A general principle is to write more static code instead of doing codegen, which is easier to review and debug.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-970177982
**[Test build #145269 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145269/testReport)** for PR 34494 at commit [`686a122`](https://github.com/apache/spark/commit/686a122b3759e849d310ff4cca65fc8389d83a16).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971536097
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145322/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971400531
**[Test build #145327 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145327/testReport)** for PR 34494 at commit [`c974930`](https://github.com/apache/spark/commit/c974930a451538cae2a935d20afe644cb34ec596).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966249689
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145090/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r748037204
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -81,9 +82,13 @@ object Cast {
case (StringType, CalendarIntervalType) => true
case (StringType, _: DayTimeIntervalType) => true
case (StringType, _: YearMonthIntervalType) => true
+ case (_: IntegralType, DayTimeIntervalType(s, e)) if s == e => true
+ case (_: IntegralType, YearMonthIntervalType(s, e)) if s == e => true
case (_: DayTimeIntervalType, _: DayTimeIntervalType) => true
case (_: YearMonthIntervalType, _: YearMonthIntervalType) => true
+ case (DayTimeIntervalType(s, e), _: IntegralType) if s == e => true
+ case (YearMonthIntervalType(s, e), _: IntegralType) if s == e => true
Review comment:
> Since we can cast INTERVAL DAY TO SECOND to INTERVAL SECOND, I think it's a reasonable and convenient extension to > cast INTERVAL DAY TO SECOND to integral type directly.
So clever. You're absolutely right. I didn't think of this use case.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -81,9 +82,13 @@ object Cast {
case (StringType, CalendarIntervalType) => true
case (StringType, _: DayTimeIntervalType) => true
case (StringType, _: YearMonthIntervalType) => true
+ case (_: IntegralType, DayTimeIntervalType(s, e)) if s == e => true
+ case (_: IntegralType, YearMonthIntervalType(s, e)) if s == e => true
case (_: DayTimeIntervalType, _: DayTimeIntervalType) => true
case (_: YearMonthIntervalType, _: YearMonthIntervalType) => true
+ case (DayTimeIntervalType(s, e), _: IntegralType) if s == e => true
+ case (YearMonthIntervalType(s, e), _: IntegralType) if s == e => true
Review comment:
> Since we can cast INTERVAL DAY TO SECOND to INTERVAL SECOND, I think it's a reasonable and convenient extension to > cast INTERVAL DAY TO SECOND to integral type directly.
So clever. You're absolutely right. I didn't think of this use case.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966989418
Update. @cloud-fan @MaxGekk If you have time, I'd like to ask you to take a look at it again. Thank you very much.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-969137581
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145235/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750878448
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1580,6 +1647,59 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
}
}
+ private[this] def castDayTimeIntervalToIntegralTypeCode(
+ ctx: CodegenContext,
+ endFiled: Byte,
+ integralType: String,
+ catalogType: String): CastFunction = {
+ val util = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
+ integralType match {
+ case "long" =>
+ (c, evPrim, _) =>
+ code"""$evPrim = $util.dayTimeIntervalToLong($c, (byte)$endFiled);"""
+ case "int" =>
+ (c, evPrim, _) =>
+ code"""$evPrim = $util.dayTimeIntervalToInt($c, (byte)$endFiled);"""
+ case "short" =>
+ (c, evPrim, _) =>
+ code"""$evPrim = $util.dayTimeIntervalToShort($c, (byte)$endFiled);"""
+ case "byte" =>
+ (c, evPrim, _) =>
+ code"""$evPrim = $util.dayTimeIntervalToByte($c, (byte)$endFiled);"""
+ }
+ }
+
+ private[this] def castYearMonthIntervalToIntegralTypeCode(
+ ctx: CodegenContext,
+ endFiled: Byte,
+ integralType: String,
+ catalogType: String): CastFunction = {
Review comment:
this parameter is not used
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-970020691
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49739/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966988990
**[Test build #145156 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145156/testReport)** for PR 34494 at commit [`b624c2f`](https://github.com/apache/spark/commit/b624c2f9e080d06a0d1e05c33cd94552ee598189).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750264114
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1580,6 +1675,59 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
}
}
+ private[this] def castDayTimeIntervalToIntegralTypeCode(
+ ctx: CodegenContext,
+ endFiled: Byte,
+ integralType: String,
+ catalogType: String): CastFunction = {
+ val util = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
+ integralType match {
Review comment:
is it just `$evPrim = $util.dayTimeIntervalTo$integralType($c, (byte)$endFiled);`?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971242245
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49781/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971143052
**[Test build #145310 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145310/testReport)** for PR 34494 at commit [`20c8e9f`](https://github.com/apache/spark/commit/20c8e9f4f1d53c3b58b91422118bd44264f61fe5).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r747309723
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -589,6 +594,9 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToDTInterval(s, it.startField, it.endField))
case _: DayTimeIntervalType => buildCast[Long](_, s =>
IntervalUtils.durationToMicros(IntervalUtils.microsToDuration(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
+ b => IntervalUtils.longToDayTimeInterval(
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toLong(b), it.endField)
Review comment:
No overflow is possible that onvert byte/short/int/long to long. It's not like year-month intervals, because there is a case that long to int.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966255257
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145095/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-969972906
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49739/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r748004567
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +606,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
At now, if `it.startField != it.endField`. the exception is as follow like:
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-967055702
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49627/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961830498
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961905452
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749068269
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,50 @@ object IntervalUtils {
}
intervalString
}
+
+ def integralToYearMonthInterval(v: Long, endField: Byte): Int = {
+ if (v != v.toInt) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ endField match {
+ case YEAR =>
+ try {
+ Math.multiplyExact(v.toInt, MONTHS_PER_YEAR)
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ case MONTH => v.toInt
+ }
+ }
+
+ def yearMonthIntervalToInt(v: Int, endFiled: Byte): Int = {
+ endFiled match {
+ case YEAR => v / MONTHS_PER_YEAR
+ case MONTH => v
+ }
+ }
+
+ def integralToDayTimeInterval(v: Long, endField: Byte): Long = {
+ try {
+ endField match {
+ case DAY => Math.multiplyExact(v, MICROS_PER_DAY)
+ case HOUR => Math.multiplyExact(v, MICROS_PER_HOUR)
+ case MINUTE => Math.multiplyExact(v, MICROS_PER_MINUTE)
+ case SECOND => Math.multiplyExact(v, MICROS_PER_SECOND)
+ }
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, DT(endField).catalogString)
+ }
+ }
+
+ def dayTimeIntervalToLong(v: Long, endFiled: Byte): Long = {
+ endFiled match {
+ case DAY => v / MICROS_PER_DAY
+ case HOUR => v / MICROS_PER_HOUR
+ case MINUTE => v / MICROS_PER_MINUTE
+ case SECOND => v / MICROS_PER_SECOND
+ }
+ }
Review comment:
Thinking about it more, I think a better approach is
```
def intToYearMonthInterval...
def longToYearMonthInterval... (has extra overflow check and call intToYearMonthInterval at the end)
def longToDayTimeInterval...
def yearMonthIntervalToLong... = yearMonthIntervalToInt(...).toLong
def yearMonthIntervalToInt... (no overflow check)
def yearMonthIntervalToShort... (has overflow check and call yearMonthIntervalToInt at the beginning)
def yearMonthIntervalToByte... (ditto)
def dayTimeIntervalToLong... (no overflow check)
def dayTimeIntervalToInt... (has overflow check and call dayTimeIntervalToLong at the beginning)
def dayTimeIntervalToShort... (ditto)
def dayTimeIntervalToByte... (ditto)
```
The benefits are:
1. Codegen code path can call the best version to get the best performance (e.g. no `int -> long -> int` roundtrip)
2. interpreted code path can avoid duplicating the overflow checking logic.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749939106
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala
##########
@@ -580,4 +584,329 @@ class CastSuite extends CastSuiteBase {
test("SPARK-36286: invalid string cast to timestamp") {
checkEvaluation(cast(Literal("2015-03-18T"), TimestampType), null)
}
+
+ test("SPARK-36924: Cast DayTimeIntervalType to IntegralType") {
+ DataTypeTestUtils.dayTimeIntervalTypes.foreach { dt =>
+ val v1 = Literal.create(Duration.ZERO, dt)
+ checkEvaluation(cast(v1, ByteType), 0.toByte)
+ checkEvaluation(cast(v1, ShortType), 0.toShort)
+ checkEvaluation(cast(v1, IntegerType), 0)
+ checkEvaluation(cast(v1, LongType), 0L)
+
+ val num = SECONDS_PER_DAY + SECONDS_PER_HOUR + SECONDS_PER_MINUTE + 1
+ val v2 = Literal.create(Duration.ofSeconds(num), dt)
+ dt.endField match {
+ case DAY =>
+ checkEvaluation(cast(v2, ByteType), 1.toByte)
+ checkEvaluation(cast(v2, ShortType), 1.toShort)
+ checkEvaluation(cast(v2, IntegerType), 1)
+ checkEvaluation(cast(v2, LongType), 1L)
+ case HOUR =>
+ checkEvaluation(cast(v2, ByteType), 25.toByte)
+ checkEvaluation(cast(v2, ShortType), 25.toShort)
+ checkEvaluation(cast(v2, IntegerType), 25)
+ checkEvaluation(cast(v2, LongType), 25L)
+ case MINUTE =>
+ val e = intercept[ArithmeticException] {
+ cast(v2, ByteType).eval()
Review comment:
done
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala
##########
@@ -580,4 +584,329 @@ class CastSuite extends CastSuiteBase {
test("SPARK-36286: invalid string cast to timestamp") {
checkEvaluation(cast(Literal("2015-03-18T"), TimestampType), null)
}
+
+ test("SPARK-36924: Cast DayTimeIntervalType to IntegralType") {
+ DataTypeTestUtils.dayTimeIntervalTypes.foreach { dt =>
+ val v1 = Literal.create(Duration.ZERO, dt)
+ checkEvaluation(cast(v1, ByteType), 0.toByte)
+ checkEvaluation(cast(v1, ShortType), 0.toShort)
+ checkEvaluation(cast(v1, IntegerType), 0)
+ checkEvaluation(cast(v1, LongType), 0L)
+
+ val num = SECONDS_PER_DAY + SECONDS_PER_HOUR + SECONDS_PER_MINUTE + 1
+ val v2 = Literal.create(Duration.ofSeconds(num), dt)
+ dt.endField match {
+ case DAY =>
+ checkEvaluation(cast(v2, ByteType), 1.toByte)
+ checkEvaluation(cast(v2, ShortType), 1.toShort)
+ checkEvaluation(cast(v2, IntegerType), 1)
+ checkEvaluation(cast(v2, LongType), 1L)
+ case HOUR =>
+ checkEvaluation(cast(v2, ByteType), 25.toByte)
+ checkEvaluation(cast(v2, ShortType), 25.toShort)
+ checkEvaluation(cast(v2, IntegerType), 25)
+ checkEvaluation(cast(v2, LongType), 25L)
+ case MINUTE =>
+ val e = intercept[ArithmeticException] {
+ cast(v2, ByteType).eval()
+ }.getMessage
+ assert(e.contains(s"Casting 90060000000 to tinyint causes overflow"))
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971340589
**[Test build #145310 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145310/testReport)** for PR 34494 at commit [`20c8e9f`](https://github.com/apache/spark/commit/20c8e9f4f1d53c3b58b91422118bd44264f61fe5).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750878288
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1580,6 +1647,59 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
}
}
+ private[this] def castDayTimeIntervalToIntegralTypeCode(
+ ctx: CodegenContext,
+ endFiled: Byte,
+ integralType: String,
+ catalogType: String): CastFunction = {
Review comment:
this parameter is not used
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971377632
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49792/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961821841
@MaxGekk @cloud-fan Can you give me some comments. At now I just done the cast between `YearMonthInterval` and `Numeric`. Your guidance will help me during development of casting between `DayTimeIntervalType` and `Numeric`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971488537
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49798/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971644801
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145327/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961905452
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966242739
**[Test build #145090 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145090/testReport)** for PR 34494 at commit [`faeffbc`](https://github.com/apache/spark/commit/faeffbc8041484c5c05d97aafb964ba8cef561cf).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r747309723
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -589,6 +594,9 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToDTInterval(s, it.startField, it.endField))
case _: DayTimeIntervalType => buildCast[Long](_, s =>
IntervalUtils.durationToMicros(IntervalUtils.microsToDuration(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
+ b => IntervalUtils.longToDayTimeInterval(
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toLong(b), it.endField)
Review comment:
No overflow is possible that convert byte/short/int/long to long. It's not like year-month intervals, because there is a case that long to int.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-967202496
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145156/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-967202496
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145156/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966138038
**[Test build #145104 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145104/testReport)** for PR 34494 at commit [`b170f78`](https://github.com/apache/spark/commit/b170f7888225a0a3c52c468a2e0aed9bab360df4).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966255257
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145095/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966138038
**[Test build #145104 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145104/testReport)** for PR 34494 at commit [`b170f78`](https://github.com/apache/spark/commit/b170f7888225a0a3c52c468a2e0aed9bab360df4).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966336869
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145104/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971143052
**[Test build #145310 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145310/testReport)** for PR 34494 at commit [`20c8e9f`](https://github.com/apache/spark/commit/20c8e9f4f1d53c3b58b91422118bd44264f61fe5).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750879209
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,115 @@ object IntervalUtils {
}
intervalString
}
+
+ def intToYearMonthInterval(v: Int, endField: Byte): Int = {
+ endField match {
+ case YEAR =>
+ try {
+ Math.multiplyExact(v, MONTHS_PER_YEAR)
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ case MONTH => v
+ }
+ }
+
+ def longToYearMonthInterval(v: Long, endField: Byte): Int = {
+ val vInt = v.toInt
+ if (v != vInt) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ intToYearMonthInterval(vInt, endField)
+ }
+
+ def yearMonthIntervalToInt(v: Int, endFiled: Byte): Int = {
+ endFiled match {
+ case YEAR => v / MONTHS_PER_YEAR
+ case MONTH => v
+ }
+ }
+
+ def yearMonthIntervalToShort(v: Int, endFiled: Byte): Short = {
+ val vInt = yearMonthIntervalToInt(v, endFiled)
+ val vShort = vInt.toShort
+ if (vInt != vShort) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, ShortType.catalogString);
Review comment:
not a blocker, but it will be great to print `v` with interval format, not just an integer.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750877856
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1502,6 +1541,20 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
code"""
$evPrim = $util.durationToMicros($util.microsToDuration($c), (byte)${it.endField});
"""
+ case ByteType | ShortType | IntegerType =>
+ assert(it.startField == it.endField)
+ val util = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
+ (c, evPrim, _) =>
+ code"""
+ $evPrim = $util.intToDayTimeInterval($c, (byte)${it.endField});
+ """
+ case LongType =>
Review comment:
ditto, we can unify the two cases and write `if-else`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r748037204
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -81,9 +82,13 @@ object Cast {
case (StringType, CalendarIntervalType) => true
case (StringType, _: DayTimeIntervalType) => true
case (StringType, _: YearMonthIntervalType) => true
+ case (_: IntegralType, DayTimeIntervalType(s, e)) if s == e => true
+ case (_: IntegralType, YearMonthIntervalType(s, e)) if s == e => true
case (_: DayTimeIntervalType, _: DayTimeIntervalType) => true
case (_: YearMonthIntervalType, _: YearMonthIntervalType) => true
+ case (DayTimeIntervalType(s, e), _: IntegralType) if s == e => true
+ case (YearMonthIntervalType(s, e), _: IntegralType) if s == e => true
Review comment:
> Since we can cast INTERVAL DAY TO SECOND to INTERVAL SECOND, I think it's a reasonable and convenient extension to cast INTERVAL DAY TO SECOND to integral type directly.
So clever. You're absolutely right. I didn't think of this use case.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968960245
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49705/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-970179510
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145269/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968459385
**[Test build #145216 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145216/testReport)** for PR 34494 at commit [`514d275`](https://github.com/apache/spark/commit/514d2754c9ae0e58ea21c6fd6cbe96cb0f025213).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968459385
**[Test build #145216 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145216/testReport)** for PR 34494 at commit [`514d275`](https://github.com/apache/spark/commit/514d2754c9ae0e58ea21c6fd6cbe96cb0f025213).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971341276
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49792/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r751040338
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,120 @@ object IntervalUtils {
}
intervalString
}
+
+ def intToYearMonthInterval(v: Int, endField: Byte): Int = {
+ endField match {
+ case YEAR =>
+ try {
+ Math.multiplyExact(v, MONTHS_PER_YEAR)
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ case MONTH => v
+ }
+ }
+
+ def longToYearMonthInterval(v: Long, endField: Byte): Int = {
+ val vInt = v.toInt
+ if (v != vInt) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ intToYearMonthInterval(vInt, endField)
+ }
+
+ def yearMonthIntervalToInt(v: Int, startField: Byte, endField: Byte): Int = {
Review comment:
Here, startField is not used. but in order to unify code in codegen `$evPrim = $util.yearMonthIntervalTo$integralType($c, (byte)$startField, (byte)$endField);`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968594057
**[Test build #145216 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145216/testReport)** for PR 34494 at commit [`514d275`](https://github.com/apache/spark/commit/514d2754c9ae0e58ea21c6fd6cbe96cb0f025213).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966045384
**[Test build #145090 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145090/testReport)** for PR 34494 at commit [`faeffbc`](https://github.com/apache/spark/commit/faeffbc8041484c5c05d97aafb964ba8cef561cf).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966253790
**[Test build #145095 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145095/testReport)** for PR 34494 at commit [`004cc67`](https://github.com/apache/spark/commit/004cc67960b2f46e57bb92f2cced2c22caaf48a1).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r744351232
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +603,24 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
Ok, Thank you. I will do 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r743613019
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +603,24 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
We can move the code to `IntervalUtils` and share the code between codegen and interpreted code paths.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961905452
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961830498
**[Test build #144931 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144931/testReport)** for PR 34494 at commit [`bf9de6c`](https://github.com/apache/spark/commit/bf9de6c5841faf74719ef7c732ce5b6b2e7e6001).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961905452
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49402/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961830498
**[Test build #144931 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144931/testReport)** for PR 34494 at commit [`bf9de6c`](https://github.com/apache/spark/commit/bf9de6c5841faf74719ef7c732ce5b6b2e7e6001).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961830498
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r747276666
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -81,9 +82,13 @@ object Cast {
case (StringType, CalendarIntervalType) => true
case (StringType, _: DayTimeIntervalType) => true
case (StringType, _: YearMonthIntervalType) => true
+ case (_: IntegralType, DayTimeIntervalType(s, e)) if s == e => true
+ case (_: IntegralType, YearMonthIntervalType(s, e)) if s == e => true
Review comment:
PR's title says about the numeric type. Are you going to support other types in the PR like decimal?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +606,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
How does the error message look like when `t.startField != it.endField`. It would be nice if we say to users that according to the SQL standard the start and end fields should be the same.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -589,6 +594,9 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToDTInterval(s, it.startField, it.endField))
case _: DayTimeIntervalType => buildCast[Long](_, s =>
IntervalUtils.durationToMicros(IntervalUtils.microsToDuration(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
+ b => IntervalUtils.longToDayTimeInterval(
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toLong(b), it.endField)
Review comment:
Why don't you handle overflow here like for year-month intervals?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -688,6 +722,28 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
}
case x: NumericType =>
b => x.numeric.asInstanceOf[Numeric[Any]].toInt(b).toShort
+ case DayTimeIntervalType(s, e) if s == e =>
+ buildCast[Long](_, i => dayTimeIntervalToShort(i, e))
+ case YearMonthIntervalType(s, e) if s == e =>
+ buildCast[Int](_, i => yearMonthIntervalToShort(i, e))
+ }
+
+ private[this] def dayTimeIntervalToShort(v: Long, endFiled: Byte): Short = {
Review comment:
The same. Please, embed it.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -645,6 +666,19 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
b => x.exactNumeric.asInstanceOf[Numeric[Any]].toInt(b)
case x: NumericType =>
b => x.numeric.asInstanceOf[Numeric[Any]].toInt(b)
+ case DayTimeIntervalType(s, e) if s == e =>
+ buildCast[Long](_, i => dayTimeIntervalToInt(i, e))
+ case YearMonthIntervalType(s, e) if s == e =>
+ buildCast[Int](_, i => yearMonthIntervalToInt(i, e))
+ }
+
+ private[this] def dayTimeIntervalToInt(v: Long, endFiled: Byte): Int = {
Review comment:
Could you embed it as this function is used only once.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r747350973
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +606,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
Oh, Maybe I made a mistake that we no need check `it.startField == it.endField`, because it has called the `canCast` and return false if `it.startField != it.endField` when go here. So I think no need add error message here. Do you think so?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r748062082
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +606,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
+ b =>
+ val intValue = try {
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toInt(b)
Review comment:
+1
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968604434
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145216/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749068269
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,50 @@ object IntervalUtils {
}
intervalString
}
+
+ def integralToYearMonthInterval(v: Long, endField: Byte): Int = {
+ if (v != v.toInt) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ endField match {
+ case YEAR =>
+ try {
+ Math.multiplyExact(v.toInt, MONTHS_PER_YEAR)
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ case MONTH => v.toInt
+ }
+ }
+
+ def yearMonthIntervalToInt(v: Int, endFiled: Byte): Int = {
+ endFiled match {
+ case YEAR => v / MONTHS_PER_YEAR
+ case MONTH => v
+ }
+ }
+
+ def integralToDayTimeInterval(v: Long, endField: Byte): Long = {
+ try {
+ endField match {
+ case DAY => Math.multiplyExact(v, MICROS_PER_DAY)
+ case HOUR => Math.multiplyExact(v, MICROS_PER_HOUR)
+ case MINUTE => Math.multiplyExact(v, MICROS_PER_MINUTE)
+ case SECOND => Math.multiplyExact(v, MICROS_PER_SECOND)
+ }
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, DT(endField).catalogString)
+ }
+ }
+
+ def dayTimeIntervalToLong(v: Long, endFiled: Byte): Long = {
+ endFiled match {
+ case DAY => v / MICROS_PER_DAY
+ case HOUR => v / MICROS_PER_HOUR
+ case MINUTE => v / MICROS_PER_MINUTE
+ case SECOND => v / MICROS_PER_SECOND
+ }
+ }
Review comment:
Thinking about it more, I think a better approach is
```
def intToYearMonthInterval...
def longToYearMonthInterval... (has extra overflow check and call intToYearMonthInterval at the end)
def longToDayTimeInterval...
def yearMonthIntervalToInt... (no overflow check)
def yearMonthIntervalToShort... (has overflow check and call yearMonthIntervalToInt at the beginning)
def yearMonthIntervalToByte... (ditto)
def dayTimeIntervalToLong... (no overflow check)
def dayTimeIntervalToInt... (has overflow check and call dayTimeIntervalToLong at the beginning)
def dayTimeIntervalToShort... (ditto)
def dayTimeIntervalToByte... (ditto)
```
The benefits are:
1. Codegen code path can call the best version to get the best performance (e.g. no `int -> long -> int` roundtrip)
2. interpreted code path can avoid duplicating the overflow checking logic.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968862481
**[Test build #145235 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145235/testReport)** for PR 34494 at commit [`19f275a`](https://github.com/apache/spark/commit/19f275a50d026ff6e227b716045eba4b7b95ffae).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968941609
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49705/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750849610
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -589,6 +594,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToDTInterval(s, it.startField, it.endField))
case _: DayTimeIntervalType => buildCast[Long](_, s =>
IntervalUtils.durationToMicros(IntervalUtils.microsToDuration(s), it.endField))
+ case x: LongType =>
+ assert(it.startField == it.endField)
+ b => IntervalUtils.longToDayTimeInterval(
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toLong(b), it.endField)
+ case x: IntegralType =>
+ assert(it.startField == it.endField)
+ assert(x != LongType)
+ b => IntervalUtils.intToDayTimeInterval(
+ x.integral.asInstanceOf[Integral[Any]].toInt(b), it.endField)
Review comment:
Merge byte/short/int into one case branch to reduce the code and add `assert(x != LongType)` although I think it won't get here for LongType
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750956969
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -589,6 +594,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToDTInterval(s, it.startField, it.endField))
case _: DayTimeIntervalType => buildCast[Long](_, s =>
IntervalUtils.durationToMicros(IntervalUtils.microsToDuration(s), it.endField))
+ case x: LongType =>
+ assert(it.startField == it.endField)
+ b => IntervalUtils.longToDayTimeInterval(
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toLong(b), it.endField)
+ case x: IntegralType =>
+ assert(it.startField == it.endField)
+ assert(x != LongType)
+ b => IntervalUtils.intToDayTimeInterval(
+ x.integral.asInstanceOf[Integral[Any]].toInt(b), it.endField)
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-969940491
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49739/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750268691
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,131 @@ object IntervalUtils {
}
intervalString
}
+
+ def byteAndShortToYearMonthInterval(v: Int, endField: Byte): Int = {
Review comment:
I feel it's a bit overkill to add this method to avoid overflow check for byte and short, as these two types are rarely used and perf doesn't matter that much.
I think we can just call `intToYearMonthInterval` for byte and short.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971396841
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971312880
**[Test build #145321 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145321/testReport)** for PR 34494 at commit [`276673a`](https://github.com/apache/spark/commit/276673a993319c18bd2cf4e750396a24511b6696).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r747311835
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -645,6 +666,19 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
b => x.exactNumeric.asInstanceOf[Numeric[Any]].toInt(b)
case x: NumericType =>
b => x.numeric.asInstanceOf[Numeric[Any]].toInt(b)
+ case DayTimeIntervalType(s, e) if s == e =>
+ buildCast[Long](_, i => dayTimeIntervalToInt(i, e))
+ case YearMonthIntervalType(s, e) if s == e =>
+ buildCast[Int](_, i => yearMonthIntervalToInt(i, e))
+ }
+
+ private[this] def dayTimeIntervalToInt(v: Long, endFiled: Byte): Int = {
Review comment:
make sense.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r747307497
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -81,9 +82,13 @@ object Cast {
case (StringType, CalendarIntervalType) => true
case (StringType, _: DayTimeIntervalType) => true
case (StringType, _: YearMonthIntervalType) => true
+ case (_: IntegralType, DayTimeIntervalType(s, e)) if s == e => true
+ case (_: IntegralType, YearMonthIntervalType(s, e)) if s == e => true
Review comment:
No. According SQL standard. The conversion that between AN and Interval Type is not allowed. I will update title "NumericType => IntegralType"
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966047461
**[Test build #145095 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145095/testReport)** for PR 34494 at commit [`004cc67`](https://github.com/apache/spark/commit/004cc67960b2f46e57bb92f2cced2c22caaf48a1).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966288190
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49573/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966334978
**[Test build #145104 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145104/testReport)** for PR 34494 at commit [`b170f78`](https://github.com/apache/spark/commit/b170f7888225a0a3c52c468a2e0aed9bab360df4).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-967017066
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49627/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968862481
**[Test build #145235 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145235/testReport)** for PR 34494 at commit [`19f275a`](https://github.com/apache/spark/commit/19f275a50d026ff6e227b716045eba4b7b95ffae).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r748031987
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -81,9 +82,13 @@ object Cast {
case (StringType, CalendarIntervalType) => true
case (StringType, _: DayTimeIntervalType) => true
case (StringType, _: YearMonthIntervalType) => true
+ case (_: IntegralType, DayTimeIntervalType(s, e)) if s == e => true
Review comment:
Since we can cast `INTERVAL DAY TO SECOND` to `INTERVAL SECOND`, I think it's a reasonable and convenient extension to cast `INTERVAL DAY TO SECOND` to integral type directly. That said, we can allow casting any interval type to integral type. For casting integral to interval, we should still only allow single field.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749060247
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1502,6 +1564,12 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
code"""
$evPrim = $util.durationToMicros($util.microsToDuration($c), (byte)${it.endField});
"""
+ case ByteType | ShortType | IntegerType | LongType if it.startField == it.endField =>
Review comment:
ditto: use assert
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-969912548
**[Test build #145269 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145269/testReport)** for PR 34494 at commit [`686a122`](https://github.com/apache/spark/commit/686a122b3759e849d310ff4cca65fc8389d83a16).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749070604
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala
##########
@@ -580,4 +584,329 @@ class CastSuite extends CastSuiteBase {
test("SPARK-36286: invalid string cast to timestamp") {
checkEvaluation(cast(Literal("2015-03-18T"), TimestampType), null)
}
+
+ test("SPARK-36924: Cast DayTimeIntervalType to IntegralType") {
+ DataTypeTestUtils.dayTimeIntervalTypes.foreach { dt =>
+ val v1 = Literal.create(Duration.ZERO, dt)
+ checkEvaluation(cast(v1, ByteType), 0.toByte)
+ checkEvaluation(cast(v1, ShortType), 0.toShort)
+ checkEvaluation(cast(v1, IntegerType), 0)
+ checkEvaluation(cast(v1, LongType), 0L)
+
+ val num = SECONDS_PER_DAY + SECONDS_PER_HOUR + SECONDS_PER_MINUTE + 1
+ val v2 = Literal.create(Duration.ofSeconds(num), dt)
+ dt.endField match {
+ case DAY =>
+ checkEvaluation(cast(v2, ByteType), 1.toByte)
+ checkEvaluation(cast(v2, ShortType), 1.toShort)
+ checkEvaluation(cast(v2, IntegerType), 1)
+ checkEvaluation(cast(v2, LongType), 1L)
+ case HOUR =>
+ checkEvaluation(cast(v2, ByteType), 25.toByte)
+ checkEvaluation(cast(v2, ShortType), 25.toShort)
+ checkEvaluation(cast(v2, IntegerType), 25)
+ checkEvaluation(cast(v2, LongType), 25L)
+ case MINUTE =>
+ val e = intercept[ArithmeticException] {
+ cast(v2, ByteType).eval()
+ }.getMessage
+ assert(e.contains(s"Casting 90060000000 to tinyint causes overflow"))
Review comment:
shall we use interval value string instead of `90060000000`?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968960245
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49705/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750850100
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +612,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: LongType =>
+ assert(it.startField == it.endField)
+ b => IntervalUtils.longToYearMonthInterval(
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toLong(b), it.endField)
+ case x: IntegralType =>
Review comment:
ditto
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971534540
**[Test build #145322 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145322/testReport)** for PR 34494 at commit [`2ac9d11`](https://github.com/apache/spark/commit/2ac9d11ae8db628442f6b8f7698f881012a711db).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971312880
**[Test build #145321 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145321/testReport)** for PR 34494 at commit [`276673a`](https://github.com/apache/spark/commit/276673a993319c18bd2cf4e750396a24511b6696).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #34494:
URL: https://github.com/apache/spark/pull/34494
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971384588
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49793/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971644801
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145327/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961885542
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49402/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971381242
> not a blocker, but it will be great to print v with interval format, not just an integer.
Although it is not a blocker. but with interval format is friendly and clear. So I update the 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971488537
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49798/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971219289
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49781/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750959137
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1755,6 +1850,10 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
castFractionToIntegralTypeCode("long", LongType.catalogString)
case x: NumericType =>
(c, evPrim, evNull) => code"$evPrim = (long) $c;"
+ case x: DayTimeIntervalType =>
+ castDayTimeIntervalToIntegralTypeCode(x.endField, "Long")
+ case x: YearMonthIntervalType =>
+ castYearMonthIntervalToIntegralTypeCode(x.endField, "Int")
Review comment:
Here we use `Int` instead of `Long`, as we use `yearMonthIntervaltoInt().toLong` instead of define the function `yearMonthIntervaltoLong`. also I think no problem that assign int value to long result.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971478235
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49798/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971314950
**[Test build #145322 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145322/testReport)** for PR 34494 at commit [`2ac9d11`](https://github.com/apache/spark/commit/2ac9d11ae8db628442f6b8f7698f881012a711db).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971536097
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145322/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971540487
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145321/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966113108
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49559/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966203681
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49564/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966249689
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145090/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966288144
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49573/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r748032282
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -81,9 +82,13 @@ object Cast {
case (StringType, CalendarIntervalType) => true
case (StringType, _: DayTimeIntervalType) => true
case (StringType, _: YearMonthIntervalType) => true
+ case (_: IntegralType, DayTimeIntervalType(s, e)) if s == e => true
+ case (_: IntegralType, YearMonthIntervalType(s, e)) if s == e => true
case (_: DayTimeIntervalType, _: DayTimeIntervalType) => true
case (_: YearMonthIntervalType, _: YearMonthIntervalType) => true
+ case (DayTimeIntervalType(s, e), _: IntegralType) if s == e => true
+ case (YearMonthIntervalType(s, e), _: IntegralType) if s == e => true
Review comment:
Since we can cast `INTERVAL DAY TO SECOND` to `INTERVAL SECOND`, I think it's a reasonable and convenient extension to cast `INTERVAL DAY TO SECOND` to integral type directly. That said, we can allow casting any interval type to integral type. For casting integral to interval, we should still only allow single field.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +606,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
+ b =>
+ val intValue = try {
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toInt(b)
Review comment:
how about we let `intToYearMonthInterval` take long? then only `intToYearMonthInterval` need to do overflow check, not here.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +606,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
> So I think no need add error message here
+1
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750957056
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1580,6 +1647,59 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
}
}
+ private[this] def castDayTimeIntervalToIntegralTypeCode(
+ ctx: CodegenContext,
+ endFiled: Byte,
+ integralType: String,
+ catalogType: String): CastFunction = {
Review comment:
done
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -1580,6 +1647,59 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
}
}
+ private[this] def castDayTimeIntervalToIntegralTypeCode(
+ ctx: CodegenContext,
+ endFiled: Byte,
+ integralType: String,
+ catalogType: String): CastFunction = {
+ val util = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
+ integralType match {
+ case "long" =>
+ (c, evPrim, _) =>
+ code"""$evPrim = $util.dayTimeIntervalToLong($c, (byte)$endFiled);"""
+ case "int" =>
+ (c, evPrim, _) =>
+ code"""$evPrim = $util.dayTimeIntervalToInt($c, (byte)$endFiled);"""
+ case "short" =>
+ (c, evPrim, _) =>
+ code"""$evPrim = $util.dayTimeIntervalToShort($c, (byte)$endFiled);"""
+ case "byte" =>
+ (c, evPrim, _) =>
+ code"""$evPrim = $util.dayTimeIntervalToByte($c, (byte)$endFiled);"""
+ }
+ }
+
+ private[this] def castYearMonthIntervalToIntegralTypeCode(
+ ctx: CodegenContext,
+ endFiled: Byte,
+ integralType: String,
+ catalogType: String): CastFunction = {
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750877258
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -589,6 +594,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToDTInterval(s, it.startField, it.endField))
case _: DayTimeIntervalType => buildCast[Long](_, s =>
IntervalUtils.durationToMicros(IntervalUtils.microsToDuration(s), it.endField))
+ case x: LongType =>
+ assert(it.startField == it.endField)
+ b => IntervalUtils.longToDayTimeInterval(
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toLong(b), it.endField)
+ case x: IntegralType =>
+ assert(it.startField == it.endField)
+ assert(x != LongType)
+ b => IntervalUtils.intToDayTimeInterval(
+ x.integral.asInstanceOf[Integral[Any]].toInt(b), it.endField)
Review comment:
seems better to unify the code more
```
case x: IntegralType =>
assert(it.startField == it.endField)
if (x == LongType) ... else ...
```
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +612,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: LongType =>
+ assert(it.startField == it.endField)
+ b => IntervalUtils.longToYearMonthInterval(
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toLong(b), it.endField)
+ case x: IntegralType =>
+ assert(it.startField == it.endField)
+ assert(x != LongType)
+ b => IntervalUtils.intToYearMonthInterval(
+ x.integral.asInstanceOf[Integral[Any]].toInt(b), it.endField)
Review comment:
ditto
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971396841
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968901823
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49705/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r748045155
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +606,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
+ b =>
+ val intValue = try {
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toInt(b)
Review comment:
make sense. It's a little weird `intToYearMonthInterval` take a parameter of long type. So I want to change the method name: `intToYearMonthInterval` => `integralToYearMonthInterval` and `longToDayTimeInterval` => `integralToDayTimeInterval`. Do you agree?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-967064241
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49627/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-967064241
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49627/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749070031
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala
##########
@@ -580,4 +584,329 @@ class CastSuite extends CastSuiteBase {
test("SPARK-36286: invalid string cast to timestamp") {
checkEvaluation(cast(Literal("2015-03-18T"), TimestampType), null)
}
+
+ test("SPARK-36924: Cast DayTimeIntervalType to IntegralType") {
+ DataTypeTestUtils.dayTimeIntervalTypes.foreach { dt =>
+ val v1 = Literal.create(Duration.ZERO, dt)
+ checkEvaluation(cast(v1, ByteType), 0.toByte)
+ checkEvaluation(cast(v1, ShortType), 0.toShort)
+ checkEvaluation(cast(v1, IntegerType), 0)
+ checkEvaluation(cast(v1, LongType), 0L)
+
+ val num = SECONDS_PER_DAY + SECONDS_PER_HOUR + SECONDS_PER_MINUTE + 1
+ val v2 = Literal.create(Duration.ofSeconds(num), dt)
+ dt.endField match {
+ case DAY =>
+ checkEvaluation(cast(v2, ByteType), 1.toByte)
+ checkEvaluation(cast(v2, ShortType), 1.toShort)
+ checkEvaluation(cast(v2, IntegerType), 1)
+ checkEvaluation(cast(v2, LongType), 1L)
+ case HOUR =>
+ checkEvaluation(cast(v2, ByteType), 25.toByte)
+ checkEvaluation(cast(v2, ShortType), 25.toShort)
+ checkEvaluation(cast(v2, IntegerType), 25)
+ checkEvaluation(cast(v2, LongType), 25L)
+ case MINUTE =>
+ val e = intercept[ArithmeticException] {
+ cast(v2, ByteType).eval()
Review comment:
let's use `checkExceptionInExpression`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966100984
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49559/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968524916
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49685/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968524916
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49685/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749064084
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,50 @@ object IntervalUtils {
}
intervalString
}
+
+ def integralToYearMonthInterval(v: Long, endField: Byte): Int = {
+ if (v != v.toInt) {
Review comment:
nit: shall we call `v.toInt` only once?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968604434
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145216/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966988990
**[Test build #145156 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145156/testReport)** for PR 34494 at commit [`b624c2f`](https://github.com/apache/spark/commit/b624c2f9e080d06a0d1e05c33cd94552ee598189).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966113108
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49559/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r749091569
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,50 @@ object IntervalUtils {
}
intervalString
}
+
+ def integralToYearMonthInterval(v: Long, endField: Byte): Int = {
+ if (v != v.toInt) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ endField match {
+ case YEAR =>
+ try {
+ Math.multiplyExact(v.toInt, MONTHS_PER_YEAR)
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ case MONTH => v.toInt
+ }
+ }
+
+ def yearMonthIntervalToInt(v: Int, endFiled: Byte): Int = {
+ endFiled match {
+ case YEAR => v / MONTHS_PER_YEAR
+ case MONTH => v
+ }
+ }
+
+ def integralToDayTimeInterval(v: Long, endField: Byte): Long = {
+ try {
+ endField match {
+ case DAY => Math.multiplyExact(v, MICROS_PER_DAY)
+ case HOUR => Math.multiplyExact(v, MICROS_PER_HOUR)
+ case MINUTE => Math.multiplyExact(v, MICROS_PER_MINUTE)
+ case SECOND => Math.multiplyExact(v, MICROS_PER_SECOND)
+ }
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, DT(endField).catalogString)
+ }
+ }
+
+ def dayTimeIntervalToLong(v: Long, endFiled: Byte): Long = {
+ endFiled match {
+ case DAY => v / MICROS_PER_DAY
+ case HOUR => v / MICROS_PER_HOUR
+ case MINUTE => v / MICROS_PER_MINUTE
+ case SECOND => v / MICROS_PER_SECOND
+ }
+ }
Review comment:
>
> The benefits are:
>
> Codegen code path can call the best version to get the best performance (e.g. no int -> long -> int roundtrip)
> interpreted code path can avoid duplicating the overflow checking logic.
I agree with benefits you said. Let me thinking more about it. I want to embed function instead of define the function. because the funciton just be used only once except `intToYearMonthInterval` `longToDayTimeInterval` `yearMonthIntervalToInt` `dayTimeIntervalToLong`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966165904
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49564/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966288190
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49573/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966054924
@MaxGekk @cloud-fan If you have time, could you please review the code again?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-966212108
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49564/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-967200870
**[Test build #145156 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145156/testReport)** for PR 34494 at commit [`b624c2f`](https://github.com/apache/spark/commit/b624c2f9e080d06a0d1e05c33cd94552ee598189).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei edited a comment on pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei edited a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968842517
update as follow：
- IntegralType to DayTimeIntervalType
1. `byte` `short` => DayTimeIntervalType
no need check overflow.
2. `int` or `long` => `DayTimeIntervalType`
need check overflow.
- DayTimeIntervalType to IntegralType
1. only `DayTimeIntervalType ` => `long`
no need check overflow.
2. others need check overflow.
- IntegralType to YearMonthIntervalType
1. `byte` `short` => `YearMonthIntervalType`
no need check overflow.
2. `int` `long` => `YearMonthIntervalType`
need check `overflow`
- YearMonthIntervalType to IntegralType
1. `YearMonthIntervalType` => `byte` or `short`
need check overflow.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r748005095
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +606,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: IntegralType if it.startField == it.endField =>
Review comment:
At now, if `it.startField != it.endField` the exception is as follow:
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-968500557
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49685/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] Peng-Lei commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750850260
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -598,6 +612,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
IntervalUtils.castStringToYMInterval(s, it.startField, it.endField))
case _: YearMonthIntervalType => buildCast[Int](_, s =>
IntervalUtils.periodToMonths(IntervalUtils.monthsToPeriod(s), it.endField))
+ case x: LongType =>
+ assert(it.startField == it.endField)
+ b => IntervalUtils.longToYearMonthInterval(
+ x.exactNumeric.asInstanceOf[Numeric[Any]].toLong(b), it.endField)
+ case x: IntegralType =>
+ assert(it.startField == it.endField)
+ assert(x != LongType)
+ b => IntervalUtils.intToYearMonthInterval(
+ x.integral.asInstanceOf[Integral[Any]].toInt(b), it.endField)
Review comment:
ditto
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971351699
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49793/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971642508
**[Test build #145327 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145327/testReport)** for PR 34494 at commit [`c974930`](https://github.com/apache/spark/commit/c974930a451538cae2a935d20afe644cb34ec596).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971654574
The GA failure is unrelated. Merging to master, thanks!
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34494:
URL: https://github.com/apache/spark/pull/34494#discussion_r750317463
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1253,4 +1255,131 @@ object IntervalUtils {
}
intervalString
}
+
+ def byteAndShortToYearMonthInterval(v: Int, endField: Byte): Int = {
+ endField match {
+ case YEAR => v * MONTHS_PER_YEAR
+ case MONTH => v
+ }
+ }
+
+ def intToYearMonthInterval(v: Int, endField: Byte): Int = {
+ endField match {
+ case YEAR =>
+ try {
+ Math.multiplyExact(v, MONTHS_PER_YEAR)
+ } catch {
+ case _: ArithmeticException =>
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ case MONTH => v
+ }
+ }
+
+ def longToYearMonthInterval(v: Long, endField: Byte): Int = {
+ val vInt = v.toInt
+ if (v != vInt) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, YM(endField).catalogString)
+ }
+ intToYearMonthInterval(vInt, endField)
+ }
+
+ def yearMonthIntervalToInt(v: Int, endFiled: Byte): Int = {
+ endFiled match {
+ case YEAR => v / MONTHS_PER_YEAR
+ case MONTH => v
+ }
+ }
+
+ def yearMonthIntervalToShort(v: Int, endFiled: Byte): Short = {
+ val vInt = yearMonthIntervalToInt(v, endFiled)
+ val vShort = vInt.toShort
+ if (vInt != vShort) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, ShortType.catalogString);
+ }
+ vShort
+ }
+
+ def yearMonthIntervalToByte(v: Int, endFiled: Byte): Byte = {
+ val vInt = yearMonthIntervalToInt(v, endFiled)
+ val vByte = vInt.toByte
+ if (vInt != vByte) {
+ throw QueryExecutionErrors.castingCauseOverflowError(v, ByteType.catalogString);
+ }
+ vByte
+ }
+
+ def byteAndShortToDayTimeInterval(v: Int, endField: Byte): Long = {
Review comment:
ditto
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and IntegralType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-971201954
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49781/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961830498
**[Test build #144931 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144931/testReport)** for PR 34494 at commit [`bf9de6c`](https://github.com/apache/spark/commit/bf9de6c5841faf74719ef7c732ce5b6b2e7e6001).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34494: [SPARK-36924][SQL] CAST between ANSI intervals and numerics
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34494:
URL: https://github.com/apache/spark/pull/34494#issuecomment-961905452
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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