LuciferYang commented on code in PR #53026:
URL: https://github.com/apache/spark/pull/53026#discussion_r2521861936
##########
sql/connect/client/jdbc/src/test/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectJdbcDataTypeSuite.scala:
##########
@@ -482,4 +434,133 @@ class SparkConnectJdbcDataTypeSuite extends
ConnectFunSuite with RemoteSparkSess
assert(!rs.next())
}
}
+
+ test("get time type") {
+ withExecuteQuery("SELECT time '12:34:56.123456'") { rs =>
+ assert(rs.next())
+ val time = rs.getTime(1)
+ // Verify milliseconds are preserved (123 from 123456 microseconds)
+ val expectedMillis = 12 * 3600000L + 34 * 60000L + 56 * 1000L + 123L
+ assert(time.getTime === expectedMillis)
+ assert(!rs.wasNull)
+ assert(!rs.next())
+
+ val metaData = rs.getMetaData
+ assert(metaData.getColumnCount === 1)
+ assert(metaData.getColumnName(1) === "TIME '12:34:56.123456'")
+ assert(metaData.getColumnLabel(1) === "TIME '12:34:56.123456'")
+ assert(metaData.getColumnType(1) === Types.TIME)
+ assert(metaData.getColumnTypeName(1) === "TIME(6)")
+ assert(metaData.getColumnClassName(1) === "java.sql.Time")
+ assert(metaData.isSigned(1) === false)
+ assert(metaData.getPrecision(1) === 6)
+ assert(metaData.getScale(1) === 0)
+ assert(metaData.getColumnDisplaySize(1) === 15)
+ }
+ }
+
+ test("get time type with null") {
+ withExecuteQuery("SELECT cast(null as time)") { rs =>
+ assert(rs.next())
+ assert(rs.getTime(1) === null)
+ assert(rs.wasNull)
+ assert(!rs.next())
+
+ val metaData = rs.getMetaData
+ assert(metaData.getColumnCount === 1)
+ assert(metaData.getColumnName(1) === "CAST(NULL AS TIME(6))")
+ assert(metaData.getColumnLabel(1) === "CAST(NULL AS TIME(6))")
+ assert(metaData.getColumnType(1) === Types.TIME)
+ assert(metaData.getColumnTypeName(1) === "TIME(6)")
+ assert(metaData.getColumnClassName(1) === "java.sql.Time")
+ assert(metaData.isSigned(1) === false)
+ assert(metaData.getPrecision(1) === 6)
+ assert(metaData.getScale(1) === 0)
+ assert(metaData.getColumnDisplaySize(1) === 15)
+ }
+ }
+
+ test("get time type by column label") {
+ withExecuteQuery("SELECT time '09:15:30.456789' as test_time") { rs =>
+ assert(rs.next())
+ val time = rs.getTime("test_time")
+ // Verify milliseconds are preserved (456 from 456789 microseconds)
+ val expectedMillis = 9 * 3600000L + 15 * 60000L + 30 * 1000L + 456L
Review Comment:
can extract a helper function like:
```
private def timeToMillis(hour: Int, minute: Int, second: Int, millis: Int):
Long = {
hour * 3600000L + minute * 60000L + second * 1000L + millis
}
```
##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectResultSet.scala:
##########
@@ -147,8 +148,22 @@ class SparkConnectResultSet(
getColumnValue(columnIndex, null: Date) { idx => currentRow.getDate(idx) }
}
- override def getTime(columnIndex: Int): Time =
- throw new SQLFeatureNotSupportedException
+ override def getTime(columnIndex: Int): Time = {
+ getColumnValue(columnIndex, null: Time) { idx =>
+ val localTime = currentRow.get(idx).asInstanceOf[LocalTime]
+ // Convert LocalTime to milliseconds since midnight to preserve
fractional seconds.
+ // Note: java.sql.Time can only store up to millisecond precision (3
digits).
+ // For TIME types with higher precision (TIME(4-9)),
microseconds/nanoseconds are truncated.
+ // If user needs full precision,
+ // should use: getObject(columnIndex, classOf[java.time.LocalTime])
+ val millisSinceMidnight =
Review Comment:
Can we use `val millisSinceMidnight =
java.time.temporal.ChronoUnit.MILLIS.between(LocalTime.MIDNIGHT, localTime)`?
##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectResultSet.scala:
##########
@@ -501,28 +516,22 @@ class SparkConnectResultSet(
throw new SQLFeatureNotSupportedException
override def getDate(columnIndex: Int, cal: Calendar): Date = {
- val date = getDate(columnIndex)
- if (date == null || cal == null) {
- return date
- }
-
- val targetCalendar = cal.clone().asInstanceOf[Calendar]
- targetCalendar.setTime(date)
- targetCalendar.set(Calendar.HOUR_OF_DAY, 0)
- targetCalendar.set(Calendar.MINUTE, 0)
- targetCalendar.set(Calendar.SECOND, 0)
- targetCalendar.set(Calendar.MILLISECOND, 0)
- new Date(targetCalendar.getTimeInMillis)
+ // The Calendar parameter in JDBC methods is meant for timezone-aware
types,
Review Comment:
I suggest turning it into class or method comments so that it can be
reflected in the Javadoc.
--
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]
