MaxGekk commented on a change in pull request #32312:
URL: https://github.com/apache/spark/pull/32312#discussion_r619128561
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala
##########
@@ -304,18 +304,23 @@ object CatalystTypeConverters {
row.getUTF8String(column).toString
}
- private object DateConverter extends CatalystTypeConverter[Date, Date, Any] {
- override def toCatalystImpl(scalaValue: Date): Int =
DateTimeUtils.fromJavaDate(scalaValue)
+ private object DateConverter extends CatalystTypeConverter[Any, Date, Any] {
+ override def toCatalystImpl(scalaValue: Any): Int = scalaValue match {
+ case d: Date => DateTimeUtils.fromJavaDate(d)
+ case l: LocalDate => DateTimeUtils.localDateToDays(l)
+ case other => throw new IllegalArgumentException(
+ s"The value (${other.toString}) of the type
(${other.getClass.getCanonicalName}) "
+ + s"cannot be converted to the date type")
Review comment:
`s` is not needed.
##########
File path:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/CatalystTypeConvertersSuite.scala
##########
@@ -294,4 +294,44 @@ class CatalystTypeConvertersSuite extends SparkFunSuite
with SQLHelper {
}
}
}
+
+ test("SPARK-35204: createToCatalystConverter for date") {
+ Seq(true, false).foreach { enable =>
+ withSQLConf(SQLConf.DATETIME_JAVA8API_ENABLED.key -> enable.toString) {
+ Seq(-1234, 0, 1234).foreach { days =>
+ val converter =
CatalystTypeConverters.createToCatalystConverter(DateType)
+
+ val ld = LocalDate.ofEpochDay(days)
+ val result1 = converter(ld)
+
+ val d = java.sql.Date.valueOf(ld)
+ val result2 = converter(d)
+
+ val expected = DateTimeUtils.localDateToDays(ld)
+ assert(result1 === expected)
+ assert(result2 === expected)
+ }
+ }
+ }
+ }
+
+ test("SPARK-35204: createToCatalystConverter for timestamp") {
+ Seq(true, false).foreach { enable =>
+ withSQLConf(SQLConf.DATETIME_JAVA8API_ENABLED.key -> enable.toString) {
+ Seq(-1234, 0, 1234).foreach { seconds =>
+ val converter =
CatalystTypeConverters.createToCatalystConverter(TimestampType)
+
+ val i = Instant.ofEpochSecond(seconds)
+ val result1 = converter(i)
+
+ val t = new java.sql.Timestamp(seconds * 1000)
Review comment:
Use MILLIS_PER_SECOND
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala
##########
@@ -324,19 +329,24 @@ object CatalystTypeConverters {
DateTimeUtils.daysToLocalDate(row.getInt(column))
}
- private object TimestampConverter extends CatalystTypeConverter[Timestamp,
Timestamp, Any] {
- override def toCatalystImpl(scalaValue: Timestamp): Long =
- DateTimeUtils.fromJavaTimestamp(scalaValue)
+ private object TimestampConverter extends CatalystTypeConverter[Any,
Timestamp, Any] {
+ override def toCatalystImpl(scalaValue: Any): Long = scalaValue match {
+ case t: Timestamp => DateTimeUtils.fromJavaTimestamp(t)
+ case i: Instant => DateTimeUtils.instantToMicros(i)
+ case other => throw new IllegalArgumentException(
+ s"The value (${other.toString}) of the type
(${other.getClass.getCanonicalName}) "
+ + s"cannot be converted to the timestamp type")
Review comment:
The same - `s` is not needed
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala
##########
@@ -304,18 +304,23 @@ object CatalystTypeConverters {
row.getUTF8String(column).toString
}
- private object DateConverter extends CatalystTypeConverter[Date, Date, Any] {
- override def toCatalystImpl(scalaValue: Date): Int =
DateTimeUtils.fromJavaDate(scalaValue)
+ private object DateConverter extends CatalystTypeConverter[Any, Date, Any] {
+ override def toCatalystImpl(scalaValue: Any): Int = scalaValue match {
+ case d: Date => DateTimeUtils.fromJavaDate(d)
+ case l: LocalDate => DateTimeUtils.localDateToDays(l)
+ case other => throw new IllegalArgumentException(
+ s"The value (${other.toString}) of the type
(${other.getClass.getCanonicalName}) "
+ + s"cannot be converted to the date type")
Review comment:
> to the date type
How about to call a method of `DateType` to render type name.
--
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]
