peter-toth commented on code in PR #59783:
URL: https://github.com/apache/spark/pull/59783#discussion_r4375276751
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala:
##########
@@ -1155,8 +1155,14 @@ object DDLUtils extends Logging {
_.toLowerCase(Locale.ROOT) match {
case HIVE_PROVIDER =>
val serde = table.storage.serde
+ // Hive has no TIMESTAMP_NTZ type. Check the nested types too.
+ val timestampNTZColumn =
+
schema.find(_.dataType.existsRecursively(_.isInstanceOf[TimestampNTZType]))
if (schema.exists(_.dataType.isInstanceOf[AnsiIntervalType])) {
throw hiveTableWithAnsiIntervalsError(table.identifier)
+ } else if (timestampNTZColumn.isDefined) {
+ throw QueryCompilationErrors.dataTypeUnsupportedByDataSourceError(
Review Comment:
**Finding 1.** This check runs during analysis, from
`PreprocessTableCreation`
([`rules.scala:276`](https://github.com/apache/spark/blob/c13103ce13795be4f2a2cb5d7d4d18874f3ff27e/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala#L276)
and
[`:296`](https://github.com/apache/spark/blob/c13103ce13795be4f2a2cb5d7d4d18874f3ff27e/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala#L296)).
That is before `IF NOT EXISTS` (`SaveMode.Ignore`) can make the statement a
no-op, so an existing table now makes it fail:
```sql
CREATE TABLE t (ts TIMESTAMP) STORED AS PARQUET; -- default timestampType
SET spark.sql.timestampType=TIMESTAMP_NTZ;
CREATE TABLE IF NOT EXISTS t (ts TIMESTAMP) STORED AS PARQUET;
-- base: no-op
-- head: UNSUPPORTED_DATA_TYPE_FOR_DATASOURCE for `ts`
```
I ran three variants on base and head: this one, the CTAS form (`CREATE
TABLE IF NOT EXISTS t STORED AS PARQUET AS SELECT TIMESTAMP'...' AS ts`), and
an explicit `TIMESTAMP_NTZ` column. All three pass on base and fail on head.
`saveAsTable` with `mode("ignore")` is fine on both, because
`SaveAsV1TableCommand` returns before it builds the CTAS. The bot review raised
the same point
([pullrequestreview-5645998752](https://github.com/apache/spark/pull/59783#pullrequestreview-5645998752)).
The ANSI interval check above already behaves this way. This one is easier
to hit, though, because a plain `TIMESTAMP` under
`spark.sql.timestampType=TIMESTAMP_NTZ` triggers it from an idempotent DDL
script. Could the check skip an `IF NOT EXISTS` create when the table already
exists? Or it could run where the table is actually created.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala:
##########
@@ -1155,8 +1155,14 @@ object DDLUtils extends Logging {
_.toLowerCase(Locale.ROOT) match {
case HIVE_PROVIDER =>
val serde = table.storage.serde
+ // Hive has no TIMESTAMP_NTZ type. Check the nested types too.
+ val timestampNTZColumn =
+
schema.find(_.dataType.existsRecursively(_.isInstanceOf[TimestampNTZType]))
Review Comment:
**Finding 2.** The check reads `table.dataSchema`, so it doesn't see a
trailing partition column. The bot review points this out too. For Parquet and
ORC, though, such a column is caught later, after `createTable`. That turns a
CTAS that used to succeed into a failure:
```scala
spark.range(3).write.format("hive").saveAsTable("t")
spark.range(1).selectExpr("1 AS i", "TIMESTAMP_NTZ'2026-01-01 00:00:00' AS
p")
.write.format("hive").option("fileFormat", "parquet").partitionBy("p")
.mode("overwrite").saveAsTable("t")
// base: OK, t has only column i (p is lost)
// head: UNSUPPORTED_DATA_TYPE_FOR_DATASOURCE for `p`, and t is gone
```
`toHiveTable` stores `p` with the placeholder type, so the created table
reads back unpartitioned. `RelationConversions` then converts the CTAS write
and calls this check with `p` as a data column
([`HiveStrategies.scala:285`](https://github.com/apache/spark/blob/c13103ce13795be4f2a2cb5d7d4d18874f3ff27e/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveStrategies.scala#L285)).
By then the overwrite has dropped the existing table, and the CTAS drops the
new one. `CREATE TABLE t STORED AS PARQUET PARTITIONED BY (p) AS SELECT ...`
fails the same way. `textfile` is not converted, so it still succeeds without
`p` on both, just like a plain `CREATE TABLE ... PARTITIONED BY (p
TIMESTAMP_NTZ)`.
SPARK-60493 ([#59734](https://github.com/apache/spark/pull/59734)) rejects
such a partition column before the drop. So landing it first closes this.
Otherwise, could the user-facing section mention the Parquet/ORC change?
--
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]
