dongjoon-hyun commented on code in PR #58734:
URL: https://github.com/apache/spark/pull/58734#discussion_r3993651495


##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowCreateTableSuiteBase.scala:
##########
@@ -184,6 +185,27 @@ trait ShowCreateTableSuiteBase extends QueryTest with 
DDLCommandTestUtils {
     }
   }
 
+  test("SPARK-59433: SHOW CREATE TABLE keeps TIMESTAMP_LTZ columns when the 
default timestamp " +

Review Comment:
   `SHOW CREATE TABLE ... AS SERDE` also goes through `StructField.toDDL` 
(`showHiveTableHeader` in `tables.scala`). It is documented as generating Hive 
DDL, but under NTZ it will now emit `TIMESTAMP_LTZ` for a timestamp column of a 
Hive SerDe table, which Hive cannot parse. Is this intended? Either way, it 
would be good to cover the `AS SERDE` path in a test.



##########
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 =

Review Comment:
   This changes auto-generated column names too, not only DDL. `Cast.sql` and 
`Literal.sql` use `dataType.sql`, so under 
`spark.sql.timestampType=TIMESTAMP_NTZ`, `SELECT CAST(c AS TIMESTAMP_LTZ)` is 
now named `CAST(c AS TIMESTAMP_LTZ)` instead of `CAST(c AS TIMESTAMP)`.
   
   This can break existing persistent views after upgrade. A view created in an 
NTZ session, e.g. `CREATE VIEW v AS SELECT CAST(ntz_col AS TIMESTAMP_LTZ) FROM 
t`, stores the old query output column name and captures 
`spark.sql.timestampType=TIMESTAMP_NTZ`. On read, the view query is re-analyzed 
under the captured conf, produces the new name, and 
`GetViewColumnByNameAndOrdinal` fails with `INCOMPATIBLE_VIEW_SCHEMA_CHANGE`.
   
   CTAS / DataFrame column names and `toSQLType` error messages change in the 
same way. Could you check this case and mention it in the user-facing change 
section?



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