gengliangwang commented on code in PR #58734:
URL: https://github.com/apache/spark/pull/58734#discussion_r3992350921


##########
sql/api/src/main/scala/org/apache/spark/sql/types/TimestampType.scala:
##########
@@ -35,6 +36,11 @@ class TimestampType private () extends DatetimeType {
    */
   override def defaultSize: Int = 8
 
+  // A bare `TIMESTAMP` resolves to the session default timestamp type, so 
render this type
+  // explicitly when that default is not this type, keeping the generated DDL 
round-trippable.
+  override def sql: String =
+    if (SqlApiConf.get.timestampType == TimestampNTZType) "TIMESTAMP_LTZ" else 
"TIMESTAMP"

Review Comment:
   **[P1] Do not use client-local config to render LTZ.** `SqlApiConf` is 
explicitly hardcoded in sql-api-only/Spark Connect, so this branch is never 
taken by a Scala Connect client even when the remote session has 
`spark.sql.timestampType=TIMESTAMP_NTZ`. For example, `DataFrameReader` sends a 
concrete `StructType` as `schema.toDDL`; the server reparses that DDL under the 
remote NTZ session, so a requested `TimestampType` is sent as bare `TIMESTAMP` 
and silently becomes `TimestampNTZType`. The concrete-schema overloads of 
`from_json`, `from_csv`, and `from_xml` have the same issue. Please use an 
unambiguous schema transport/rendering for concrete types and add Scala Connect 
coverage under an NTZ remote session.



-- 
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]

Reply via email to