spark-test commented on code in PR #59757:
URL: https://github.com/apache/spark/pull/59757#discussion_r4366077587
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:
##########
@@ -306,6 +306,16 @@ private[spark] class HiveExternalCatalog(conf: SparkConf,
hadoopConf: Configurat
}
}
+ /**
+ * Runs the checks that Hive runs on a new Hive serde table before creating
it in Hive
+ * metastore, e.g., the SerDe initialization with the table columns, without
creating it.
+ * A command that replaces an existing table calls this before dropping the
existing table.
+ */
+ private[hive] def validateHiveSerdeTable(tableDefinition: CatalogTable):
Unit = withClient {
+ // Like `createTable`, remove the collations, which Hive doesn't support.
+ client.validateTable(tableDefinition.copy(schema =
removeCollation(tableDefinition.schema)))
Review Comment:
**Nit (P3):** This `removeCollation` call is what lets a valid overwrite
with a collated string column pass the new pre-drop check, but no test
exercises it. Without it, `toHiveColumn` would emit a type string like `string
collate UNICODE`, which the Hive SerDe that `checkValidity` initializes can't
parse. Such an overwrite would then fail before the drop with a Hive type-parse
`AnalysisException`, even though `createTable` would accept the same table.
None of the existing tests reaches this combination:
- The new `MetastoreDataSourcesSuite` test has only rejection cases.
- The valid Hive-format overwrites of an existing table in `HiveDDLSuite`
use plain types.
- The partitioned overwrite in `InsertSuite` writes a table that doesn't
exist yet, so it skips the replace path.
- The `HiveExternalCatalogSuite` collation test covers `createTable` and
`alterTableSchema` only.
- The new `HiveClientSuite` test calls the client directly, bypassing this
method.
The current code is correct. The gap is that a later change to this second
copy of the collation handling would break valid collated overwrites without
failing any test. A positive collated case in the new
`MetastoreDataSourcesSuite` test would close it.
**Verification:**
- **Regression:** Overwriting an existing Hive serde table through
saveAsTable with a DataFrame that contains a collated string column succeeds,
the table then contains the new rows, and its schema reports the collated type.
The case must fail when the collation stripping in the pre-drop validation is
removed.
--
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]
