You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2026/09/11 08:43:52 UTC
[PR] [SPARK-59433][SQL] Render TIMESTAMP_LTZ explicitly when spark.sql.timestampType=TIMESTAMP_NTZ [spark]
pan3793 opened a new pull request, #58734:
URL: https://github.com/apache/spark/pull/58734
### What changes were proposed in this pull request?
Make LTZ type rendering follow spark.sql.timestampType: when the session default is
TIMESTAMP_NTZ, SHOW CREATE TABLE / schema DDL and LTZ literals spell TIMESTAMP_LTZ
explicitly, so the emitted text re-parses to the same type. Nested fields are covered.
No new configuration.
### Why are the changes needed?
Under spark.sql.timestampType=TIMESTAMP_NTZ the emitted DDL is not round-trippable:
SET spark.sql.timestampType=TIMESTAMP_NTZ;
CREATE TABLE t (ltz TIMESTAMP_LTZ) USING parquet;
SHOW CREATE TABLE t;
-- before: ltz TIMESTAMP -- reparse yields TIMESTAMP_NTZ
-- after: ltz TIMESTAMP_LTZ
LTZ literals have the same problem in plan and column text.
### Does this PR introduce _any_ user-facing change?
Yes, only when spark.sql.timestampType=TIMESTAMP_NTZ: LTZ columns show as TIMESTAMP_LTZ
in SHOW CREATE TABLE / schema DDL, and LTZ literals render as TIMESTAMP_LTZ '...'. The
default (TIMESTAMP_LTZ) output is unchanged.
### How was this patch tested?
Added unit tests for both defaults and NTZ-default golden variants (goldens regenerated
with SPARK_GENERATE_GOLDEN_FILES=1 and reviewed). Verified with the catalyst literal and
type suites and the sql timestamp and SHOW CREATE TABLE suites.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Codex
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59433][SQL] Render TIMESTAMP_LTZ explicitly when spark.sql.timestampType=TIMESTAMP_NTZ [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59433][SQL] Render TIMESTAMP_LTZ explicitly when spark.sql.timestampType=TIMESTAMP_NTZ [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #58734:
URL: https://github.com/apache/spark/pull/58734#issuecomment-5631866278
cc @gengliangwang
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59433][SQL] Render TIMESTAMP_LTZ explicitly when spark.sql.timestampType=TIMESTAMP_NTZ [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org