MaxGekk commented on a change in pull request #33280:
URL: https://github.com/apache/spark/pull/33280#discussion_r667382470
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1008,17 +1008,17 @@ case class UnixTimestamp(
copy(timeExp = newLeft, format = newRight)
}
-case class GetTimestampNTZ(
+/**
+ * Gets timestamps from strings using given pattern.
Review comment:
Not only from strings but from dates, for instance. And the given
pattern is optional since it is not used when the input is a date.
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1931,16 +1932,16 @@ case class ParseToTimestamp(left: Expression, format:
Option[Expression], child:
extends RuntimeReplaceable {
def this(left: Expression, format: Expression) = {
- this(left, Option(format), GetTimestamp(left, format))
+ this(left, Option(format), GetTimestamp(left, format,
SQLConf.get.timestampType))
}
- def this(left: Expression) = this(left, None, Cast(left, TimestampType))
+ def this(left: Expression) = this(left, None, Cast(left,
SQLConf.get.timestampType))
override def flatArguments: Iterator[Any] = Iterator(left, format)
override def exprsReplaced: Seq[Expression] = left +: format.toSeq
override def prettyName: String = "to_timestamp"
- override def dataType: DataType = TimestampType
+ override val dataType: DataType = SQLConf.get.timestampType
Review comment:
This can lead to some issue similar to one which was fixed by
https://github.com/apache/spark/pull/27658
##########
File path:
sql/core/src/test/resources/sql-tests/results/timestampNTZ/datetime.sql.out
##########
@@ -1222,26 +1222,26 @@ select to_timestamp('2019-10-06 A', 'yyyy-MM-dd GGGGG')
-- !query schema
struct<>
-- !query output
-org.apache.spark.SparkUpgradeException
-You may get a different result due to the upgrading of Spark 3.0: Fail to
recognize 'yyyy-MM-dd GGGGG' pattern in the DateTimeFormatter. 1) You can set
spark.sql.legacy.timeParserPolicy to LEGACY to restore the behavior before
Spark 3.0. 2) You can form a valid datetime pattern with the guide from
https://spark.apache.org/docs/latest/sql-ref-datetime-pattern.html
+java.lang.RuntimeException
Review comment:
Is it intentional replacement?
##########
File path:
sql/core/src/test/resources/sql-tests/results/timestampNTZ/datetime.sql.out
##########
@@ -146,7 +146,7 @@ NULL 2016-12-31 2016-12-31
-- !query
select to_timestamp(null), to_timestamp('2016-12-31 00:12:00'),
to_timestamp('2016-12-31', 'yyyy-MM-dd')
-- !query schema
-struct<to_timestamp(NULL):timestamp,to_timestamp(2016-12-31
00:12:00):timestamp,to_timestamp(2016-12-31, yyyy-MM-dd):timestamp>
+struct<to_timestamp(NULL):timestamp_ntz,to_timestamp(2016-12-31
00:12:00):timestamp_ntz,to_timestamp(2016-12-31, yyyy-MM-dd):timestamp_ntz>
Review comment:
From PR's description:
> Does this PR introduce any user-facing change?
> No, the new timestmap type is not release yet and the default value of
"spark.sql.timestampType" is TIMESTAMP_LTZ
But the changes in the golden file seems do change the behavior, doesn't 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: [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]
