MaxGekk commented on a change in pull request #28534:
URL: https://github.com/apache/spark/pull/28534#discussion_r426594740
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -806,6 +806,17 @@ abstract class ToTimestamp
case NonFatal(_) => null
}
+ private lazy val scaleFactor = right.toString match {
Review comment:
Please, look at other places in the file how to handle foldable
expressions.
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -846,6 +857,15 @@ abstract class ToTimestamp
case NonFatal(_) => null
}
}
+ case LongType =>
+ val input = t.asInstanceOf[Long]
+ if ( minRange < input && input < maxRange ) {
+ Math.multiplyExact(input, scaleFactor.asInstanceOf[Long])
Review comment:
`Math.multiplyExact` throws `ArithmeticException`. You don't need to
explicitly check the ranges.
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -846,6 +857,15 @@ abstract class ToTimestamp
case NonFatal(_) => null
}
}
+ case LongType =>
+ val input = t.asInstanceOf[Long]
+ if ( minRange < input && input < maxRange ) {
Review comment:
Just in case, why do you exclude min and max values?
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -789,7 +789,7 @@ abstract class ToTimestamp
protected def downScaleFactor: Long
override def inputTypes: Seq[AbstractDataType] =
- Seq(TypeCollection(StringType, DateType, TimestampType), StringType)
+ Seq(TypeCollection(StringType, DateType, TimestampType, LongType ),
StringType)
Review comment:
Why only LongType. Can be any `IntegralType`
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -922,6 +942,21 @@ abstract class ToTimestamp
if (!${ev.isNull}) {
${ev.value} = $dtu.epochDaysToMicros(${eval1.value}, $zid) /
$downScaleFactor;
}""")
+ case LongType =>
+ val eval1 = left.genCode(ctx)
+ ev.copy(code = code"""
+ ${eval1.code}
+ boolean ${ev.isNull} = ${eval1.isNull};
+ $javaType ${ev.value} = ${CodeGenerator.defaultValue(dataType)};
+ if (!${ev.isNull}) {
+ if ( (${minRange}L < ${eval1.value}) && (${eval1.value} <
${maxRange}L) ) {
+ ${ev.value} = ${eval1.value} * $scaleFactor;
Review comment:
Inconsistent with non-codegen where you use Math.multiplyExact
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -806,6 +806,17 @@ abstract class ToTimestamp
case NonFatal(_) => null
}
+ private lazy val scaleFactor = right.toString match {
+ case "milli" => MICROS_PER_MILLIS
Review comment:
Is Hive case-sensitive?
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -922,6 +942,21 @@ abstract class ToTimestamp
if (!${ev.isNull}) {
${ev.value} = $dtu.epochDaysToMicros(${eval1.value}, $zid) /
$downScaleFactor;
}""")
+ case LongType =>
+ val eval1 = left.genCode(ctx)
+ ev.copy(code = code"""
+ ${eval1.code}
+ boolean ${ev.isNull} = ${eval1.isNull};
+ $javaType ${ev.value} = ${CodeGenerator.defaultValue(dataType)};
+ if (!${ev.isNull}) {
+ if ( (${minRange}L < ${eval1.value}) && (${eval1.value} <
${maxRange}L) ) {
+ ${ev.value} = ${eval1.value} * $scaleFactor;
+ } else {
+ throw new java.lang.IllegalArgumentException(
+ "input [" + ${eval1.value} + "] not from " + ${minRange}L + " to
"
+ + ${maxRange}L + " for format " + "${right.toString}");
Review comment:
hmm, does this string interpolation work in Java?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
