spark-test commented on code in PR #59733:
URL: https://github.com/apache/spark/pull/59733#discussion_r4355497711
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala:
##########
@@ -629,6 +629,67 @@ class HiveDDLSuite
}
}
+ test("SPARK-60492: Keep Hive incompatible partition columns in alterTable
and renameTable") {
+ Seq(
+ ("TIMESTAMP_NTZ", "TIMESTAMP_NTZ '2018-11-17 13:33:33'",
+ LocalDateTime.of(2018, 11, 17, 13, 33, 33)),
+ ("TIME", "TIME '12:34:56'", LocalTime.of(12, 34, 56))
+ ).foreach { case (dataType, literal, value) =>
+ def checkTable(partitionColumnNames: Seq[String]): Unit = {
+ val table =
spark.sessionState.catalog.getTableMetadata(TableIdentifier("t2"))
+ assert(table.properties.get("k").contains("v"))
+ assert(table.partitionColumnNames == partitionColumnNames)
+ checkAnswer(spark.table("t2"), Row(1, "a", value))
+ }
+ withTable("t", "t2") {
+ sql(s"CREATE TABLE t (i INT, pt $dataType) USING PARQUET PARTITIONED
BY (pt)")
+ // This stores the data columns in Hive metastore as they are Hive
compatible.
+ sql("ALTER TABLE t ADD COLUMNS (c STRING)")
+ sql("ALTER TABLE t SET TBLPROPERTIES ('k' = 'v')")
+ sql(s"INSERT INTO t SELECT 1, 'a', $literal")
+ sql("MSCK REPAIR TABLE t")
+ sql("ALTER TABLE t RENAME TO t2")
+ checkTable(Seq("pt"))
+ }
+ withTable("t", "t2") {
+ sql(s"CREATE TABLE t (i INT, P1 STRING, P2 $dataType) USING PARQUET " +
+ "PARTITIONED BY (P1, P2)")
Review Comment:
**Nit (P3):** This is the only new case that mixes a Hive-compatible and a
Hive-incompatible partition column, and it puts the incompatible column (`P2`)
last. With that layout, the HMS key order `[p1, p2]` is the same as
"Hive-visible raw partitions first, then the restored ones". So the tests
cannot tell whether `getRawTableWithAllPartitionColumns` really follows the
Hive metastore key order, which the PR description promises.
For example, a later simplification to `rawTable.partitionSchema ++
<restored fields missing from raw>` would keep every new test green. It would
break again for a data source table created with `PARTITIONED BY (P2, P1)`,
where HMS holds `[p2 (placeholder), p1]`. The built-in Hive 2.3.10 metastore
compares partition key names by position
(`HiveAlterHandler.checkPartialPartKeysEqual`), so `ALTER TABLE ... SET
TBLPROPERTIES`, `MSCK REPAIR TABLE` and `ALTER TABLE ... RENAME TO` would fail
again with `partition keys can not be changed`.
The current implementation is correct; this is only about the regression
signal. Running the same alter / MSCK / rename sequence on a table whose
incompatible partition column comes first would pin it, for example `CREATE
TABLE t (i INT, P1 STRING, P2 TIMESTAMP_NTZ) USING PARQUET PARTITIONED BY (P2,
P1)`. The INSERT values and the expected row are then reordered to `(i, P2,
P1)`, and `partitionColumnNames` is `Seq("P2", "P1")`.
**Verification:**
- **Regression:** Altering and renaming a data source table partitioned by a
Hive-incompatible column followed by a Hive-compatible column succeeds, keeps
the declared partition column order, and returns the inserted row; the same
scenario fails if restored partition columns are appended after the
Hive-visible ones instead of following the Hive metastore key order.
- **Behavior:** The existing incompatible-last mixed layout, the single
incompatible partition column after ADD COLUMNS, and the Spark 2.0-style table
whose partition columns live only in table properties all keep passing alter
and rename with their current metadata and data assertions.
--
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]
