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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
