diqiu50 commented on code in PR #14588:
URL: https://github.com/apache/gravitino/pull/14588#discussion_r4489901088
##########
trino-connector/src/common/main/java/org/apache/gravitino/trino/connector/catalog/glue/GlueMetadataAdapter.java:
##########
@@ -158,8 +206,27 @@ public GravitinoTable createTable(ConnectorTableMetadata
tableMetadata) {
tableMetadata.getProperties().containsKey(HivePropertyMeta.HIVE_PARTITION_KEY)
? (List<String>)
tableMetadata.getProperties().get(HivePropertyMeta.HIVE_PARTITION_KEY)
: Collections.emptyList();
+ @SuppressWarnings("unchecked")
+ List<String> sortFields =
+
tableMetadata.getProperties().containsKey(HivePropertyMeta.HIVE_SORT_ORDER_KEY)
+ ? (List<String>)
tableMetadata.getProperties().get(HivePropertyMeta.HIVE_SORT_ORDER_KEY)
+ : Collections.emptyList();
- GravitinoTable table = super.createTable(tableMetadata);
+ Object type = tableMetadata.getProperties().get(LAKEHOUSE_TABLE_TYPE);
+ String tableType = type == null ? defaultTableType : type.toString();
+ boolean icebergTable =
GlueConstants.TABLE_FORMAT_ICEBERG.equalsIgnoreCase(tableType);
+
+ GravitinoTable table =
super.createTable(withHiveSortColumns(tableMetadata, icebergTable));
+
+ if (icebergTable) {
+ if (!sortFields.isEmpty()) {
+
table.setSortOrders(ExpressionUtil.sortOrderFiledToExpression(sortFields));
Review Comment:
Fixed in 2fff3d540. `ExpressionUtil` now parses sort fields with the grammar
of the Trino Iceberg connector, so the direction and null ordering are optional
and column names can be quoted. Names that need quoting are shown quoted, with
embedded quotes escaped. Added CREATE and metadata-read/recreate tests for both
cases. Since the parser is shared, Iceberg catalogs get the same behavior.
Partition fields are unchanged here, so quoted column names are still not
supported there.
##########
spark-connector/spark-common/src/main/java/org/apache/gravitino/spark/connector/glue/GravitinoGlueCatalog.java:
##########
@@ -233,6 +241,36 @@ protected SparkTypeConverter getSparkTypeConverter() {
return new SparkHiveTypeConverter();
}
+ /**
+ * {@inheritDoc} A table created without {@code table-format} gets the
catalog's default table
+ * format.
+ */
+ @Override
+ protected SparkTypeConverter getSparkTypeConverter(Map<String, String>
gravitinoTableProperties) {
+ String tableFormat =
+ gravitinoTableProperties.getOrDefault(
+ GlueConstants.TABLE_FORMAT,
+ catalogProperties == null
+ ? null
+ : catalogProperties.get(GlueConstants.DEFAULT_TABLE_FORMAT));
+ return GlueConstants.TABLE_FORMAT_ICEBERG.equalsIgnoreCase(tableFormat)
+ ? ICEBERG_TYPE_CONVERTER
+ : super.getSparkTypeConverter(gravitinoTableProperties);
+ }
+
+ /** {@inheritDoc} Changes of an Iceberg table follow Iceberg type semantics.
*/
+ @Override
+ protected SparkTableChangeConverter getSparkTableChangeConverter(Identifier
ident) {
+ try {
+ if (isIcebergTable(loadGravitinoTable(ident))) {
Review Comment:
Fixed in 2fff3d540. Spark now loads the table only when an `AddColumn` or
`UpdateColumnType` sets a timestamp type, including nested ones, since that is
the only place the Hive and Iceberg mappings differ. Trino loads it only when
the Hive rules refuse the type. Added tests that no table is loaded for other
changes and types, and one that goes through `alterTable`.
--
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]
