yuqi1129 commented on code in PR #13550:
URL: https://github.com/apache/gravitino/pull/13550#discussion_r4217314853
##########
catalogs/hive-metastore3-libs/src/main/java/org/apache/gravitino/hive/client/hive3/HiveShimV3.java:
##########
@@ -158,6 +159,13 @@ public HiveTable getTable(String catalogName, String
databaseName, String tableN
if (TableType.VIRTUAL_VIEW.name().equalsIgnoreCase(tb.getTableType())) {
return HiveTableConverter.fromHiveTable(tb);
}
+ if (hasDerivedColumnTypes(tb)) {
+ List<FieldSchema> resolvedColumns =
+ invoke(
+ ExceptionTarget.table(tableName),
+ () -> client.getFields(catalogName, databaseName, tableName));
Review Comment:
On a Hive 3.x HMS with the default configuration, this call fails for
OpenCSVSerde tables, so the load still fails (now as a 500 instead of the
original 400).
In `hive-standalone-metastore-3.1.3`,
`HMSHandler.get_fields_with_environment_context` uses the deserializer only for
SerDes in `metastore.serdes.using.metastore.for.schema`, and OpenCSVSerde is
not in that list. For other SerDes it calls
`getStorageSchemaReader().readSchema(...)`. The default
`metastore.storage.schema.reader.impl` is
`org.apache.hadoop.hive.metastore.DefaultStorageSchemaReader`, whose
`readSchema` is just:
```java
throw new UnsupportedOperationException("Storage schema reading not
supported");
```
HMS wraps that in a `MetaException`, which `HiveExceptionConverter` turns
into a generic `GravitinoRuntimeException`. `HiveConf` 3.1.3 does not override
this default, and neither does our `dev/docker/hive` config.
`SerDeStorageSchemaReader` exists in `hive-metastore-3.1.3.jar`, but only takes
effect when an operator configures it. (Hive 2.3 calls
`MetaStoreUtils.getDeserializer` / `getFieldsFromDeserializer` directly, which
is why Hive 2 works.)
Options, from minimal to most robust:
1. Convert this failure into a clear error that tells users to set
`metastore.storage.schema.reader.impl=org.apache.hadoop.hive.metastore.SerDeStorageSchemaReader`
on HMS, and document it in the Hive catalog docs and the PR description.
2. Add a client-side fallback for OpenCSVSerde: it always exposes every
column as `string`, so it does not need HMS to resolve anything.
3. More generally, degrade unresolved columns to
`Types.UnparsedType`/`ExternalType` so that `loadTable` (and therefore GRANT)
still succeeds. This requires alter to write the marker back unchanged.
The same applies to the Hive 2 shim (`HiveShim.getTable`) when it talks to a
Hive 3 HMS.
##########
catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShim.java:
##########
@@ -334,4 +342,37 @@ protected EnvironmentContext doNotUpdateStatsContext() {
return new EnvironmentContext(
Collections.singletonMap(StatsSetupConst.DO_NOT_UPDATE_STATS,
StatsSetupConst.TRUE));
}
+
+ /**
+ * Returns whether a table contains column types that Hive expects its SerDe
to resolve.
+ *
+ * @param table The Hive metastore table.
+ * @return {@code true} if at least one storage column has a derived type
marker.
+ */
+ protected static boolean hasDerivedColumnTypes(Table table) {
+ return table.getSd() != null
+ && table.getSd().getCols() != null
+ && table.getSd().getCols().stream()
+ .anyMatch(field -> TYPE_FROM_DESERIALIZER.equals(field.getType()));
+ }
+
+ /**
+ * Replaces storage columns containing Hive's derived type marker with
columns resolved by the
+ * table SerDe.
+ *
+ * @param table The Hive metastore table to update in memory.
+ * @param resolvedColumns The storage columns resolved by the Hive metastore.
+ */
+ protected static void replaceDerivedColumns(Table table, List<FieldSchema>
resolvedColumns) {
+ if (resolvedColumns == null
+ || resolvedColumns.isEmpty()
+ || resolvedColumns.stream()
+ .anyMatch(field ->
TYPE_FROM_DESERIALIZER.equals(field.getType()))) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Hive Metastore did not resolve SerDe-derived column types for
table %s.%s",
Review Comment:
Could the message tell users how to fix this? For example: "... Make sure
the table SerDe is on the Hive Metastore classpath and that Hive Metastore can
read schemas from storage (for Hive 3,
`metastore.storage.schema.reader.impl`)." Please also keep the same wording for
the `getFields` failure case discussed on `HiveShimV3`.
--
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]
