yuqi1129 commented on code in PR #11073:
URL: https://github.com/apache/gravitino/pull/11073#discussion_r3285631853
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java:
##########
@@ -132,17 +136,23 @@ static boolean
isIcebergTable(software.amazon.awssdk.services.glue.model.Table g
/**
* Creates an Iceberg {@link Catalog} backed by AWS Glue.
*
- * @param config Gravitino catalog configuration (region, credentials,
endpoint, etc.)
+ * @param config Gravitino catalog configuration (region, credentials,
endpoint, etc.). Must
+ * contain {@code aws-region} and {@code warehouse}.
* @return an initialized Iceberg Glue catalog
+ * @throws IllegalArgumentException if {@code aws-region} or {@code
warehouse} is not configured
*/
static Catalog createGlueCatalog(Map<String, String> config) {
String region = config.get(GlueConstants.AWS_REGION);
Preconditions.checkArgument(region != null, "AWS region is required for
Iceberg Glue catalog");
+ String warehouse = config.get(GlueConstants.WAREHOUSE);
+ Preconditions.checkArgument(
+ warehouse != null,
+ "Warehouse is required for Iceberg Glue catalog; configure '%s' on the
catalog.",
Review Comment:
Based on `resolveTableLocation.`, it seems to be required, so the document
is not correct here.
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java:
##########
@@ -99,6 +100,16 @@ public class GlueCatalogPropertiesMetadata extends
BaseCatalogPropertiesMetadata
false /* immutable */,
DEFAULT_TABLE_FORMAT_FILTER,
false /* hidden */))
+ .put(
+ WAREHOUSE,
+ stringOptionalPropertyEntry(
+ WAREHOUSE,
+ "Base S3 path used as warehouse for Hive tables when no
explicit location is"
Review Comment:
Does it only support S3? Other cloud storage like GCS, Azure cloud stoage?
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java:
##########
@@ -207,6 +234,71 @@ static void loadTable(Catalog icebergCatalog, String
dbName, String tableName, G
}
}
+ /**
+ * Converts an Iceberg type to the equivalent Gravitino type.
+ *
+ * <p>TIME and TIMESTAMP types are always returned with microsecond (6)
precision, matching
+ * Iceberg's internal representation.
+ */
+ // The parameter uses FQN because org.apache.iceberg.types.Type and
+ // org.apache.gravitino.rel.types.Type share the same simple name.
+ static Type fromIcebergType(org.apache.iceberg.types.Type icebergType) {
Review Comment:
I suppose that code also exists in the Iceberg catalog type converter, is
possbile to reuse it?
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java:
##########
@@ -64,6 +64,13 @@ public final class GlueConstants {
/** Default value for {@link #TABLE_FORMAT_FILTER}: expose all table
formats. */
public static final String DEFAULT_TABLE_FORMAT_FILTER = "all";
+ /**
+ * Base S3 path used as a warehouse when no explicit {@code location} is
given at table creation
Review Comment:
ditto
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/glue/GlueMetadataAdapter.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.trino.connector.catalog.glue;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static io.trino.spi.session.PropertyMetadata.integerProperty;
+import static io.trino.spi.session.PropertyMetadata.stringProperty;
+import static io.trino.spi.type.VarcharType.VARCHAR;
+
+import com.google.common.collect.ImmutableList;
+import io.trino.spi.connector.ConnectorTableMetadata;
+import io.trino.spi.session.PropertyMetadata;
+import io.trino.spi.type.ArrayType;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.gravitino.catalog.property.PropertyConverter;
+import org.apache.gravitino.rel.expressions.transforms.Transform;
+import org.apache.gravitino.trino.connector.catalog.hive.HiveMetadataAdapter;
+import org.apache.gravitino.trino.connector.catalog.hive.HivePropertyMeta;
+import org.apache.gravitino.trino.connector.catalog.hive.SortingColumn;
+import org.apache.gravitino.trino.connector.catalog.iceberg.ExpressionUtil;
+import org.apache.gravitino.trino.connector.metadata.GravitinoTable;
+
+/**
+ * Transforming Apache Gravitino Glue metadata to Trino. This adapter handles
properties that are
+ * specific to the Glue catalog and the lakehouse connector, excluding
properties that conflict with
+ * the lakehouse connector's native properties.
+ */
+public class GlueMetadataAdapter extends HiveMetadataAdapter {
+
+ /** The table type property for lakehouse connector (ICEBERG, HIVE, DELTA).
*/
+ static final String LAKEHOUSE_TABLE_TYPE = "type";
+
+ private static final List<PropertyMetadata<?>> GLUE_TABLE_PROPERTY_META =
+ ImmutableList.of(
+ stringProperty(LAKEHOUSE_TABLE_TYPE, "The type of table (ICEBERG,
HIVE)", null, false),
+ new PropertyMetadata<>(
+ HivePropertyMeta.HIVE_PARTITION_KEY,
+ "Partition columns",
+ new ArrayType(VARCHAR),
+ List.class,
+ ImmutableList.of(),
+ false,
+ value ->
+ ((List<?>) value)
+ .stream()
+ .map(name -> ((String)
name).toLowerCase(Locale.ROOT))
+ .collect(ImmutableList.toImmutableList()),
+ value -> value),
+ new PropertyMetadata<>(
+ HivePropertyMeta.HIVE_BUCKET_KEY,
+ "Bucketing columns",
+ new ArrayType(VARCHAR),
+ List.class,
+ ImmutableList.of(),
+ false,
+ value ->
+ ((List<?>) value)
+ .stream()
+ .map(name -> ((String)
name).toLowerCase(Locale.ROOT))
+ .collect(ImmutableList.toImmutableList()),
+ value -> value),
+ integerProperty(
+ HivePropertyMeta.HIVE_BUCKET_COUNT_KEY,
+ "The number of buckets for the table",
+ null,
+ false),
+ new PropertyMetadata<>(
+ HivePropertyMeta.HIVE_SORT_ORDER_KEY,
+ "Bucket sorting columns",
+ new ArrayType(VARCHAR),
+ List.class,
+ ImmutableList.of(),
+ false,
+ value ->
+ ((List<?>) value)
+ .stream()
+ .map(String.class::cast)
+ .map(name -> name.toLowerCase(Locale.ROOT))
+ .map(SortingColumn::sortingColumnFromString)
+ .collect(toImmutableList()),
+ value ->
+ ((List<?>) value)
+ .stream()
+ .map(SortingColumn.class::cast)
+ .map(SortingColumn::sortingColumnToString)
+ .collect(toImmutableList())),
+ stringProperty(
+ "format", "The format of the data files (PARQUET, ORC, etc.)",
null, false),
+ stringProperty("location", "The S3 storage location for the table",
null, false));
+
+ private final PropertyConverter tableConverter = new
GlueTablePropertyConverter();
+
+ /**
+ * Constructs a new GlueMetadataAdapter.
+ *
+ * @param schemaProperties the schema properties metadata
+ */
+ public GlueMetadataAdapter(List<PropertyMetadata<?>> schemaProperties) {
+ super(
+ schemaProperties,
+ GLUE_TABLE_PROPERTY_META,
+ ImmutableList.of(),
+ new GlueDataTypeTransformer());
+ }
+
+ @Override
+ public Map<String, Object> toTrinoTableProperties(Map<String, String>
properties) {
+ // Convert Gravitino keys (e.g. table-format) to Trino keys (e.g. type),
+ // then filter to only properties declared in GLUE_TABLE_PROPERTY_META.
+ // We must NOT call super here because HiveMetadataAdapter would apply
+ // HiveTablePropertyConverter a second time, corrupting the
already-converted keys.
+ Map<String, String> converted =
tableConverter.gravitinoToEngineProperties(properties);
+ Map<String, Object> result = new HashMap<>();
+ for (PropertyMetadata<?> meta : GLUE_TABLE_PROPERTY_META) {
+ String key = meta.getName();
+ if (converted.containsKey(key)) {
+ result.put(key, converted.get(key));
+ }
+ }
+ return result;
+ }
+
+ @Override
+ public Map<String, String> toGravitinoTableProperties(Map<String, Object>
properties) {
+ // Convert Trino keys (e.g. type) to Gravitino keys (e.g. table-format),
+ // then drop null values. Must NOT call super for the same reason as above.
+ Map<String, Object> converted =
tableConverter.engineToGravitinoProperties(properties);
+ return converted.entrySet().stream()
+ .filter(e -> e.getValue() != null)
+ .collect(Collectors.toMap(Map.Entry::getKey, e ->
e.getValue().toString()));
+ }
+
+ @Override
+ public GravitinoTable createTable(ConnectorTableMetadata tableMetadata) {
+ @SuppressWarnings("unchecked")
+ List<String> partitionExpressions =
+
tableMetadata.getProperties().containsKey(HivePropertyMeta.HIVE_PARTITION_KEY)
+ ? (List<String>)
tableMetadata.getProperties().get(HivePropertyMeta.HIVE_PARTITION_KEY)
+ : Collections.emptyList();
+
+ GravitinoTable table = super.createTable(tableMetadata);
+
+ if (!partitionExpressions.isEmpty()) {
+ Transform[] transforms =
ExpressionUtil.partitionFiledToExpression(partitionExpressions);
+ table.setPartitioning(transforms);
+ }
+
+ return table;
+ }
+
+ @Override
+ public ConnectorTableMetadata getTableMetadata(GravitinoTable
gravitinoTable) {
+ Transform[] originalPartitioning = gravitinoTable.getPartitioning();
+
+ // Clear partitioning before calling super to avoid ClassCastException
when transforms
+ // contain BucketTransform or TruncateTransform (not SingleFieldTransform
subclasses).
+ gravitinoTable.setPartitioning(new Transform[0]);
+
+ ConnectorTableMetadata metadata = super.getTableMetadata(gravitinoTable);
Review Comment:
This method is rather hacky, if there a better way?
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/glue/GlueConnectorAdapter.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.trino.connector.catalog.glue;
+
+import io.trino.spi.session.PropertyMetadata;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.trino.connector.catalog.CatalogConnectorAdapter;
+import
org.apache.gravitino.trino.connector.catalog.CatalogConnectorMetadataAdapter;
+import org.apache.gravitino.trino.connector.catalog.HasPropertyMeta;
+import org.apache.gravitino.trino.connector.catalog.hive.HivePropertyMeta;
+import org.apache.gravitino.trino.connector.metadata.GravitinoCatalog;
+
+/**
+ * Transforming Apache Gravitino Glue catalog configuration and components
into Apache Gravitino
+ * connector.
+ */
+public class GlueConnectorAdapter implements CatalogConnectorAdapter {
+
+ private static final String CONNECTOR_LAKEHOUSE = "lakehouse";
+
+ // Gravitino catalog property keys for AWS Glue
+ private static final String PROP_AWS_REGION = "aws-region";
+ private static final String PROP_AWS_GLUE_CATALOG_ID = "aws-glue-catalog-id";
+ private static final String PROP_AWS_ACCESS_KEY_ID = "aws-access-key-id";
+ private static final String PROP_AWS_SECRET_ACCESS_KEY =
"aws-secret-access-key";
+ private static final String PROP_AWS_GLUE_ENDPOINT = "aws-glue-endpoint";
+
+ // Trino Hive connector configuration keys for Glue
+ private static final String HIVE_METASTORE = "hive.metastore";
+ private static final String HIVE_METASTORE_GLUE_REGION =
"hive.metastore.glue.region";
+ private static final String HIVE_METASTORE_GLUE_CATALOG_ID =
"hive.metastore.glue.catalogid";
+ private static final String HIVE_METASTORE_GLUE_ACCESS_KEY =
"hive.metastore.glue.aws-access-key";
+ private static final String HIVE_METASTORE_GLUE_SECRET_KEY =
"hive.metastore.glue.aws-secret-key";
+ private static final String HIVE_METASTORE_GLUE_ENDPOINT =
"hive.metastore.glue.endpoint-url";
+ private static final String HIVE_S3_ACCESS_KEY = "hive.s3.aws-access-key";
+ private static final String HIVE_S3_SECRET_KEY = "hive.s3.aws-secret-key";
+
+ private final HasPropertyMeta propertyMetadata;
+
+ /** Constructs a new GlueConnectorAdapter. */
+ public GlueConnectorAdapter() {
+ this.propertyMetadata = new HivePropertyMeta();
+ }
+
+ @Override
+ public Map<String, String> buildInternalConnectorConfig(GravitinoCatalog
catalog)
+ throws Exception {
+ Map<String, String> config = new HashMap<>();
+ config.put(HIVE_METASTORE, "glue");
+ config.put(HIVE_METASTORE_GLUE_REGION,
catalog.getRequiredProperty(PROP_AWS_REGION));
+ config.put("hive.security", "allow-all");
Review Comment:
What's the meaning of this configuration?
--
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]
