Copilot commented on code in PR #11087: URL: https://github.com/apache/gravitino/pull/11087#discussion_r3257055930
########## catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GravitinoGlueCredentialsProvider.java: ########## @@ -0,0 +1,62 @@ +/* + * 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.catalog.glue; + +import java.util.Map; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.AwsCredentials; +import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; + +/** + * AWS credentials provider for Iceberg {@code GlueCatalog} that reads static credentials from a + * properties map. + * + * <p>Iceberg 1.10+ no longer supports {@code client.access-key-id} directly; credentials must be + * supplied via {@code client.credentials-provider}. This class is configured in {@link + * GlueIcebergTableHelper#createGlueCatalog} when explicit credentials are provided. + */ +class GravitinoGlueCredentialsProvider implements AwsCredentialsProvider { + + private static final String ACCESS_KEY_ID = "access-key-id"; + private static final String SECRET_ACCESS_KEY = "secret-access-key"; + + private final String accessKeyId; + private final String secretAccessKey; + Review Comment: This public method is missing Javadoc. The repository guidelines require Javadocs for new public/protected methods (AGENTS.md:25), and checkstyle runs with warnings as errors, so this can fail CI unless the method is documented or made package-private if reflection does not require it to be public. ########## catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java: ########## @@ -0,0 +1,697 @@ +/* + * 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.catalog.glue; + +import com.google.common.base.Preconditions; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.regex.Pattern; +import javax.annotation.Nullable; +import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants; +import org.apache.gravitino.exceptions.NoSuchTableException; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.TableChange; +import org.apache.gravitino.rel.expressions.Expression; +import org.apache.gravitino.rel.expressions.FunctionExpression; +import org.apache.gravitino.rel.expressions.NamedReference; +import org.apache.gravitino.rel.expressions.literals.Literal; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.expressions.sorts.NullOrdering; +import org.apache.gravitino.rel.expressions.sorts.SortDirection; +import org.apache.gravitino.rel.expressions.sorts.SortOrder; +import org.apache.gravitino.rel.expressions.sorts.SortOrders; +import org.apache.gravitino.rel.expressions.transforms.Transform; +import org.apache.gravitino.rel.expressions.transforms.Transforms; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; +import org.apache.iceberg.NullOrder; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.UpdateProperties; +import org.apache.iceberg.UpdateSchema; +import org.apache.iceberg.aws.glue.GlueCatalog; +import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.expressions.Expressions; +import org.apache.iceberg.expressions.UnboundTerm; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Helper that delegates all Iceberg table operations to the Iceberg SDK's {@code GlueCatalog}. + * + * <p>Unlike the native AWS Glue SDK {@code OpenTableFormatInput} approach, the Iceberg SDK writes + * the {@code metadata.json} file to S3 and registers the table in Glue with the correct {@code + * metadata_location} parameter, making the table usable by Trino Lakehouse connector and other + * Iceberg-native query engines. + */ +final class GlueIcebergTableHelper { + + private static final Logger LOG = LoggerFactory.getLogger(GlueIcebergTableHelper.class); + + private static final String DOT = "."; + + // Iceberg GlueCatalog properties (not defined in IcebergConstants). + private static final String CATALOG_IMPL = "catalog-impl"; + private static final String GLUE_CATALOG = "GlueCatalog"; + private static final String GLUE_ID = "glue.id"; + private static final String GLUE_ENDPOINT = "glue.endpoint"; + private static final String CLIENT_CREDENTIALS_PROVIDER = "client.credentials-provider"; + private static final String CLIENT_CREDENTIALS_PROVIDER_ACCESS_KEY_ID = + "client.credentials-provider.access-key-id"; + private static final String CLIENT_CREDENTIALS_PROVIDER_SECRET_ACCESS_KEY = + "client.credentials-provider.secret-access-key"; + + private static final Set<String> EXCLUDED_TABLE_PROPS = + Set.of( + GlueConstants.LOCATION, + GlueConstants.TABLE_TYPE, + GlueConstants.METADATA_LOCATION, + GlueConstants.INPUT_FORMAT, + GlueConstants.OUTPUT_FORMAT, + GlueConstants.SERDE_LIB); + + // Iceberg 1.10+ uses "bucket[n]" / "truncate[w]" format for Transform.toString(). + private static final Pattern TRANSFORM_PARAM_PATTERN = Pattern.compile(".*\\[(\\d+)\\]"); + + private GlueIcebergTableHelper() {} + + /** + * Returns true if the Glue table is an Iceberg-format table. + * + * <p>Checks for {@code table_type=ICEBERG} in {@code Table.parameters()}. + */ + static boolean isIcebergTable(software.amazon.awssdk.services.glue.model.Table glueTable) { + Preconditions.checkArgument(glueTable != null, "glueTable cannot be null"); + if (!glueTable.hasParameters()) return false; + return GlueConstants.ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase( + glueTable.parameters().get(GlueConstants.TABLE_TYPE_PARAM)); + } + + /** + * Creates an Iceberg {@link Catalog} backed by AWS Glue. + * + * @param config Gravitino catalog configuration (region, credentials, endpoint, etc.) + * @return an initialized Iceberg Glue catalog + */ + 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"); + + Map<String, String> icebergProps = new HashMap<>(); + // Warehouse is required by Iceberg catalog initialization but is not used when each table + // provides an explicit location. + icebergProps.put(IcebergConstants.WAREHOUSE, "/tmp/gravitino-glue-iceberg"); + icebergProps.put(CATALOG_IMPL, GLUE_CATALOG); + icebergProps.put(IcebergConstants.AWS_S3_REGION, region); + + String catalogId = config.get(GlueConstants.AWS_GLUE_CATALOG_ID); + if (catalogId != null) { + icebergProps.put(GLUE_ID, catalogId); + } + + String accessKey = config.get(GlueConstants.AWS_ACCESS_KEY_ID); + String secretKey = config.get(GlueConstants.AWS_SECRET_ACCESS_KEY); + if (accessKey != null && secretKey != null) { + icebergProps.put( + CLIENT_CREDENTIALS_PROVIDER, GravitinoGlueCredentialsProvider.class.getName()); + icebergProps.put(CLIENT_CREDENTIALS_PROVIDER_ACCESS_KEY_ID, accessKey); + icebergProps.put(CLIENT_CREDENTIALS_PROVIDER_SECRET_ACCESS_KEY, secretKey); + } + + String endpoint = config.get(GlueConstants.AWS_GLUE_ENDPOINT); + if (endpoint != null) { + icebergProps.put(GLUE_ENDPOINT, endpoint); + icebergProps.put(IcebergConstants.ICEBERG_S3_ENDPOINT, endpoint); Review Comment: This assigns the Glue endpoint URL as the S3 endpoint for Iceberg's S3FileIO. `aws-glue-endpoint` is documented as a Glue endpoint, so configurations that point Glue at a VPC/moto endpoint but use a different S3 endpoint will make Iceberg metadata writes call the wrong service; use a separate S3 endpoint config or omit the S3 endpoint unless it is explicitly provided. ########## catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestAwsGlueCatalogOperations.java: ########## @@ -0,0 +1,226 @@ +/* + * 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.catalog.glue; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.HashMap; +import java.util.Map; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.TableChange; +import org.apache.gravitino.rel.expressions.distributions.Distributions; +import org.apache.gravitino.rel.expressions.sorts.SortOrders; +import org.apache.gravitino.rel.expressions.transforms.Transforms; +import org.apache.gravitino.rel.indexes.Indexes; +import org.apache.gravitino.rel.types.Types; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; + +/** + * Integration tests for {@link GlueCatalogOperations} against a real AWS Glue endpoint. + * + * <p>Only runs when {@code AWS_ACCESS_KEY_ID} is set. Required environment variables: + * + * <ul> + * <li>{@code AWS_ACCESS_KEY_ID} + * <li>{@code AWS_SECRET_ACCESS_KEY} + * <li>{@code AWS_DEFAULT_REGION} (e.g. {@code ap-northeast-1}) + * <li>{@code GLUE_CATALOG_ID} (12-digit AWS account ID; optional) + * </ul> + */ +@EnabledIfEnvironmentVariable(named = "AWS_ACCESS_KEY_ID", matches = ".+") +class TestAwsGlueCatalogOperations { + + private static GlueCatalogOperations ops; + private static final String TEST_SCHEMA = "aws_glue_ops_it_" + System.currentTimeMillis(); + private static final Namespace NS = Namespace.of("metalake", "catalog"); + + @BeforeAll + static void setup() { + Map<String, String> config = new HashMap<>(); + config.put( + GlueConstants.AWS_REGION, System.getenv().getOrDefault("AWS_DEFAULT_REGION", "us-east-1")); + String accessKey = System.getenv("AWS_ACCESS_KEY_ID"); + String secretKey = System.getenv("AWS_SECRET_ACCESS_KEY"); + if (accessKey != null) { + config.put(GlueConstants.AWS_ACCESS_KEY_ID, accessKey); + } + if (secretKey != null) { + config.put(GlueConstants.AWS_SECRET_ACCESS_KEY, secretKey); + } + String catalogId = System.getenv("GLUE_CATALOG_ID"); + if (catalogId != null) { + config.put(GlueConstants.AWS_GLUE_CATALOG_ID, catalogId); + } + + ops = new GlueCatalogOperations(); + ops.initialize(config, null, null); + + ops.createSchema(NameIdentifier.of(NS, TEST_SCHEMA), "IT schema", Map.of()); + } + + @AfterAll + static void teardown() { + if (ops == null) { + return; + } + try { + NameIdentifier[] tables = ops.listTables(Namespace.of(NS.level(0), NS.level(1), TEST_SCHEMA)); + for (NameIdentifier t : tables) { + ops.dropTable(t); + } + } catch (Exception ignored) { + } + try { + ops.dropSchema(NameIdentifier.of(NS, TEST_SCHEMA), true); + } catch (Exception ignored) { + } + ops.glueClient.close(); + } + + @Test + void testRenameTableIsUnsupported() { + String tableName = "rename_test_" + System.currentTimeMillis(); + NameIdentifier ident = NameIdentifier.of(NS.level(0), NS.level(1), TEST_SCHEMA, tableName); + + Column col = Column.of("id", Types.IntegerType.get(), "pk"); + ops.createTable( + ident, + new Column[] {col}, + "test table", + Map.of(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + SortOrders.NONE, + Indexes.EMPTY_INDEXES); + + try { + assertThrows( + UnsupportedOperationException.class, + () -> ops.alterTable(ident, TableChange.rename("new_name"))); + } finally { + ops.dropTable(ident); + } + } + + @Test + void testCreateIcebergTable() { + String tableName = "iceberg_create_" + System.currentTimeMillis(); + NameIdentifier ident = NameIdentifier.of(NS.level(0), NS.level(1), TEST_SCHEMA, tableName); + + Column col = Column.of("id", Types.LongType.get(), "pk", false, false, null); + Map<String, String> props = new HashMap<>(); + props.put(GlueConstants.TABLE_FORMAT, "ICEBERG"); + props.put(GlueConstants.LOCATION, "s3://ice-glue-test-01/iceberg/" + tableName); Review Comment: These real-AWS integration tests write Iceberg metadata to a hard-coded bucket. Anyone with AWS credentials but without access to `ice-glue-test-01` will get failures, and contributors who do have access could write test data to an unintended shared location; follow the `AwsGlueCatalogIT` pattern and require an `AWS_S3_TEST_BUCKET` environment variable instead. ########## catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestAwsGlueCatalogOperations.java: ########## @@ -0,0 +1,226 @@ +/* + * 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.catalog.glue; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.HashMap; +import java.util.Map; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.TableChange; +import org.apache.gravitino.rel.expressions.distributions.Distributions; +import org.apache.gravitino.rel.expressions.sorts.SortOrders; +import org.apache.gravitino.rel.expressions.transforms.Transforms; +import org.apache.gravitino.rel.indexes.Indexes; +import org.apache.gravitino.rel.types.Types; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; + +/** + * Integration tests for {@link GlueCatalogOperations} against a real AWS Glue endpoint. + * + * <p>Only runs when {@code AWS_ACCESS_KEY_ID} is set. Required environment variables: + * + * <ul> + * <li>{@code AWS_ACCESS_KEY_ID} + * <li>{@code AWS_SECRET_ACCESS_KEY} + * <li>{@code AWS_DEFAULT_REGION} (e.g. {@code ap-northeast-1}) + * <li>{@code GLUE_CATALOG_ID} (12-digit AWS account ID; optional) + * </ul> + */ +@EnabledIfEnvironmentVariable(named = "AWS_ACCESS_KEY_ID", matches = ".+") +class TestAwsGlueCatalogOperations { + + private static GlueCatalogOperations ops; + private static final String TEST_SCHEMA = "aws_glue_ops_it_" + System.currentTimeMillis(); + private static final Namespace NS = Namespace.of("metalake", "catalog"); + + @BeforeAll + static void setup() { + Map<String, String> config = new HashMap<>(); + config.put( + GlueConstants.AWS_REGION, System.getenv().getOrDefault("AWS_DEFAULT_REGION", "us-east-1")); + String accessKey = System.getenv("AWS_ACCESS_KEY_ID"); + String secretKey = System.getenv("AWS_SECRET_ACCESS_KEY"); + if (accessKey != null) { + config.put(GlueConstants.AWS_ACCESS_KEY_ID, accessKey); + } + if (secretKey != null) { + config.put(GlueConstants.AWS_SECRET_ACCESS_KEY, secretKey); + } + String catalogId = System.getenv("GLUE_CATALOG_ID"); + if (catalogId != null) { + config.put(GlueConstants.AWS_GLUE_CATALOG_ID, catalogId); + } + + ops = new GlueCatalogOperations(); + ops.initialize(config, null, null); + + ops.createSchema(NameIdentifier.of(NS, TEST_SCHEMA), "IT schema", Map.of()); + } + + @AfterAll + static void teardown() { + if (ops == null) { + return; + } + try { + NameIdentifier[] tables = ops.listTables(Namespace.of(NS.level(0), NS.level(1), TEST_SCHEMA)); + for (NameIdentifier t : tables) { + ops.dropTable(t); + } + } catch (Exception ignored) { + } + try { + ops.dropSchema(NameIdentifier.of(NS, TEST_SCHEMA), true); + } catch (Exception ignored) { + } + ops.glueClient.close(); + } + + @Test + void testRenameTableIsUnsupported() { + String tableName = "rename_test_" + System.currentTimeMillis(); + NameIdentifier ident = NameIdentifier.of(NS.level(0), NS.level(1), TEST_SCHEMA, tableName); + + Column col = Column.of("id", Types.IntegerType.get(), "pk"); + ops.createTable( + ident, + new Column[] {col}, + "test table", + Map.of(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + SortOrders.NONE, + Indexes.EMPTY_INDEXES); + + try { + assertThrows( + UnsupportedOperationException.class, + () -> ops.alterTable(ident, TableChange.rename("new_name"))); + } finally { + ops.dropTable(ident); + } + } + + @Test + void testCreateIcebergTable() { + String tableName = "iceberg_create_" + System.currentTimeMillis(); + NameIdentifier ident = NameIdentifier.of(NS.level(0), NS.level(1), TEST_SCHEMA, tableName); + + Column col = Column.of("id", Types.LongType.get(), "pk", false, false, null); + Map<String, String> props = new HashMap<>(); + props.put(GlueConstants.TABLE_FORMAT, "ICEBERG"); + props.put(GlueConstants.LOCATION, "s3://ice-glue-test-01/iceberg/" + tableName); + + try { + Table created = + ops.createTable( + ident, + new Column[] {col}, + "iceberg table", + props, + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + SortOrders.NONE, + Indexes.EMPTY_INDEXES); + + assertEquals("iceberg table", created.comment()); + assertEquals(1, created.columns().length); + assertEquals("id", created.columns()[0].name()); + assertEquals(Types.LongType.get(), created.columns()[0].dataType()); + + Table loaded = ops.loadTable(ident); + assertEquals("ICEBERG", loaded.properties().get(GlueConstants.TABLE_FORMAT)); + assertNotNull(loaded.properties().get(GlueConstants.METADATA_LOCATION)); + } finally { + ops.dropTable(ident); + } + } + + @Test + void testAlterIcebergTableAddColumn() { + String tableName = "iceberg_alter_col_" + System.currentTimeMillis(); + NameIdentifier ident = NameIdentifier.of(NS.level(0), NS.level(1), TEST_SCHEMA, tableName); + + Column col = Column.of("id", Types.LongType.get(), "pk", false, false, null); + Map<String, String> props = new HashMap<>(); + props.put(GlueConstants.TABLE_FORMAT, "ICEBERG"); + props.put(GlueConstants.LOCATION, "s3://ice-glue-test-01/iceberg/" + tableName); + + try { + ops.createTable( + ident, + new Column[] {col}, + "iceberg table", + props, + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + SortOrders.NONE, + Indexes.EMPTY_INDEXES); + + ops.alterTable( + ident, TableChange.addColumn(new String[] {"score"}, Types.DoubleType.get(), true)); + + Table loaded = ops.loadTable(ident); + assertEquals(2, loaded.columns().length); + assertEquals("score", loaded.columns()[1].name()); + assertEquals(Types.DoubleType.get(), loaded.columns()[1].dataType()); + } finally { + ops.dropTable(ident); + } + } + + @Test + void testAlterIcebergTableSetProperty() { + String tableName = "iceberg_alter_prop_" + System.currentTimeMillis(); + NameIdentifier ident = NameIdentifier.of(NS.level(0), NS.level(1), TEST_SCHEMA, tableName); + + Column col = Column.of("id", Types.LongType.get(), "pk", false, false, null); + Map<String, String> props = new HashMap<>(); + props.put(GlueConstants.TABLE_FORMAT, "ICEBERG"); + props.put(GlueConstants.LOCATION, "s3://ice-glue-test-01/iceberg/" + tableName); Review Comment: This real-AWS test should not depend on the hard-coded `ice-glue-test-01` bucket. Read the S3 bucket from the environment and skip when it is absent, otherwise the test is not portable and may write metadata to an unintended bucket. ########## catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java: ########## @@ -0,0 +1,697 @@ +/* + * 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.catalog.glue; + +import com.google.common.base.Preconditions; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.regex.Pattern; +import javax.annotation.Nullable; +import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants; +import org.apache.gravitino.exceptions.NoSuchTableException; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.TableChange; +import org.apache.gravitino.rel.expressions.Expression; +import org.apache.gravitino.rel.expressions.FunctionExpression; +import org.apache.gravitino.rel.expressions.NamedReference; +import org.apache.gravitino.rel.expressions.literals.Literal; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.expressions.sorts.NullOrdering; +import org.apache.gravitino.rel.expressions.sorts.SortDirection; +import org.apache.gravitino.rel.expressions.sorts.SortOrder; +import org.apache.gravitino.rel.expressions.sorts.SortOrders; +import org.apache.gravitino.rel.expressions.transforms.Transform; +import org.apache.gravitino.rel.expressions.transforms.Transforms; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; +import org.apache.iceberg.NullOrder; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.UpdateProperties; +import org.apache.iceberg.UpdateSchema; +import org.apache.iceberg.aws.glue.GlueCatalog; +import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.expressions.Expressions; +import org.apache.iceberg.expressions.UnboundTerm; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Helper that delegates all Iceberg table operations to the Iceberg SDK's {@code GlueCatalog}. + * + * <p>Unlike the native AWS Glue SDK {@code OpenTableFormatInput} approach, the Iceberg SDK writes + * the {@code metadata.json} file to S3 and registers the table in Glue with the correct {@code + * metadata_location} parameter, making the table usable by Trino Lakehouse connector and other + * Iceberg-native query engines. + */ +final class GlueIcebergTableHelper { + + private static final Logger LOG = LoggerFactory.getLogger(GlueIcebergTableHelper.class); + + private static final String DOT = "."; + + // Iceberg GlueCatalog properties (not defined in IcebergConstants). + private static final String CATALOG_IMPL = "catalog-impl"; + private static final String GLUE_CATALOG = "GlueCatalog"; + private static final String GLUE_ID = "glue.id"; + private static final String GLUE_ENDPOINT = "glue.endpoint"; + private static final String CLIENT_CREDENTIALS_PROVIDER = "client.credentials-provider"; + private static final String CLIENT_CREDENTIALS_PROVIDER_ACCESS_KEY_ID = + "client.credentials-provider.access-key-id"; + private static final String CLIENT_CREDENTIALS_PROVIDER_SECRET_ACCESS_KEY = + "client.credentials-provider.secret-access-key"; + + private static final Set<String> EXCLUDED_TABLE_PROPS = + Set.of( + GlueConstants.LOCATION, + GlueConstants.TABLE_TYPE, + GlueConstants.METADATA_LOCATION, + GlueConstants.INPUT_FORMAT, + GlueConstants.OUTPUT_FORMAT, + GlueConstants.SERDE_LIB); + + // Iceberg 1.10+ uses "bucket[n]" / "truncate[w]" format for Transform.toString(). + private static final Pattern TRANSFORM_PARAM_PATTERN = Pattern.compile(".*\\[(\\d+)\\]"); + + private GlueIcebergTableHelper() {} + + /** + * Returns true if the Glue table is an Iceberg-format table. + * + * <p>Checks for {@code table_type=ICEBERG} in {@code Table.parameters()}. + */ + static boolean isIcebergTable(software.amazon.awssdk.services.glue.model.Table glueTable) { + Preconditions.checkArgument(glueTable != null, "glueTable cannot be null"); + if (!glueTable.hasParameters()) return false; + return GlueConstants.ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase( + glueTable.parameters().get(GlueConstants.TABLE_TYPE_PARAM)); + } + + /** + * Creates an Iceberg {@link Catalog} backed by AWS Glue. + * + * @param config Gravitino catalog configuration (region, credentials, endpoint, etc.) + * @return an initialized Iceberg Glue catalog + */ + 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"); + + Map<String, String> icebergProps = new HashMap<>(); + // Warehouse is required by Iceberg catalog initialization but is not used when each table + // provides an explicit location. + icebergProps.put(IcebergConstants.WAREHOUSE, "/tmp/gravitino-glue-iceberg"); + icebergProps.put(CATALOG_IMPL, GLUE_CATALOG); + icebergProps.put(IcebergConstants.AWS_S3_REGION, region); + + String catalogId = config.get(GlueConstants.AWS_GLUE_CATALOG_ID); + if (catalogId != null) { + icebergProps.put(GLUE_ID, catalogId); + } + + String accessKey = config.get(GlueConstants.AWS_ACCESS_KEY_ID); + String secretKey = config.get(GlueConstants.AWS_SECRET_ACCESS_KEY); + if (accessKey != null && secretKey != null) { + icebergProps.put( + CLIENT_CREDENTIALS_PROVIDER, GravitinoGlueCredentialsProvider.class.getName()); + icebergProps.put(CLIENT_CREDENTIALS_PROVIDER_ACCESS_KEY_ID, accessKey); + icebergProps.put(CLIENT_CREDENTIALS_PROVIDER_SECRET_ACCESS_KEY, secretKey); + } + + String endpoint = config.get(GlueConstants.AWS_GLUE_ENDPOINT); + if (endpoint != null) { + icebergProps.put(GLUE_ENDPOINT, endpoint); + icebergProps.put(IcebergConstants.ICEBERG_S3_ENDPOINT, endpoint); + } + + if (accessKey != null && secretKey != null) { + icebergProps.put(IcebergConstants.ICEBERG_S3_ACCESS_KEY_ID, accessKey); + icebergProps.put(IcebergConstants.ICEBERG_S3_SECRET_ACCESS_KEY, secretKey); + } + + icebergProps.put(IcebergConstants.IO_IMPL, "org.apache.iceberg.aws.s3.S3FileIO"); + + GlueCatalog glueCatalog = new GlueCatalog(); + glueCatalog.initialize("gravitino-glue-iceberg", icebergProps); + LOG.info("Initialized Iceberg GlueCatalog for region {}", region); + return glueCatalog; + } + + /** + * Loads an Iceberg table and recovers partitioning and sort orders. + * + * @param icebergCatalog the Iceberg Glue catalog + * @param dbName the Glue database name + * @param tableName the table name + * @param table the GlueTable to populate with recovered metadata + */ + static void loadTable(Catalog icebergCatalog, String dbName, String tableName, GlueTable table) { + Table icebergTable = icebergCatalog.loadTable(TableIdentifier.of(dbName, tableName)); + if (icebergTable == null) { + throw new NoSuchTableException( + "Iceberg table %s.%s not found in Iceberg catalog", dbName, tableName); + } + + // Merge Iceberg table properties (stored in metadata.json) into the Gravitino properties. + // Iceberg properties take precedence over Glue parameters for overlapping keys. + Map<String, String> mergedProps = new HashMap<>(table.properties()); + mergedProps.putAll(icebergTable.properties()); + table.setProperties(mergedProps); + + if (!icebergTable.spec().fields().isEmpty()) { + table.setPartitioning(convertPartitionSpec(icebergTable.spec(), icebergTable.schema())); + } + if (!icebergTable.sortOrder().fields().isEmpty()) { + table.setSortOrders(convertIcebergSortOrder(icebergTable.sortOrder(), icebergTable.schema())); + } + } + + /** + * Creates an Iceberg table via the Iceberg SDK. + * + * <p>The table is written to S3 (metadata.json) and registered in Glue automatically. + * + * @param icebergCatalog the Iceberg Glue catalog + * @param dbName the Glue database name + * @param tableName the table name + * @param columns the table columns + * @param comment the table comment + * @param properties Gravitino table properties (must contain {@code location}) + * @param partitions partition transforms + * @param sortOrders sort orders + */ + static void createTable( + Catalog icebergCatalog, + String dbName, + String tableName, + Column[] columns, + @Nullable String comment, + Map<String, String> properties, + @Nullable Transform[] partitions, + @Nullable SortOrder[] sortOrders) { + + Schema schema = toIcebergSchema(columns); + PartitionSpec spec = toPartitionSpec(schema, partitions); + org.apache.iceberg.SortOrder icebergSortOrder = toSortOrder(schema, sortOrders); + + Map<String, String> tableProps = new HashMap<>(); + // Forward user-provided properties to Iceberg, excluding Gravitino/Glue internal keys. + properties.forEach( + (k, v) -> { + if (!EXCLUDED_TABLE_PROPS.contains(k)) { + tableProps.put(k, v); + } + }); + if (comment != null) { + tableProps.put("comment", comment); + } + + String location = properties.get(GlueConstants.LOCATION); + Preconditions.checkArgument( + location != null, "Location is required for Iceberg table creation"); + + String format = properties.get(GlueConstants.FORMAT); + if (format != null) { + tableProps.put("write.format.default", format.toLowerCase(Locale.ROOT)); + } + + String tableFormat = properties.get(GlueConstants.TABLE_FORMAT); + if (tableFormat != null) { + tableProps.put(GlueConstants.TABLE_FORMAT, tableFormat); + } + + TableIdentifier tableId = TableIdentifier.of(dbName, tableName); + + LOG.info("Creating Iceberg table {} at location {} via Iceberg SDK", tableId, location); + + try { + icebergCatalog + .buildTable(tableId, schema) + .withLocation(location) + .withPartitionSpec(spec) + .withSortOrder(icebergSortOrder) + .withProperties(tableProps) + .create(); + } catch (org.apache.iceberg.exceptions.AlreadyExistsException e) { + throw new org.apache.gravitino.exceptions.TableAlreadyExistsException( + e, "Table %s.%s already exists", dbName, tableName); + } catch (org.apache.iceberg.exceptions.ValidationException e) { + throw new IllegalArgumentException("Invalid table definition: " + e.getMessage(), e); + } + } + + /** + * Alters an Iceberg table via the Iceberg SDK. + * + * <p>Delegates schema changes to {@link UpdateSchema} and property changes to {@link + * UpdateProperties}. + * + * <p><b>Note:</b> Schema changes and property changes are committed in two separate transactions. + * If the schema commit succeeds but the property commit fails, the table is left in a partially + * altered state. This is a known limitation of the current Iceberg SDK integration. + * + * @param icebergCatalog the Iceberg Glue catalog + * @param dbName the Glue database name + * @param tableName the table name + * @param changes the table changes to apply + */ + static void alterTable( + Catalog icebergCatalog, String dbName, String tableName, TableChange... changes) { + Table table = icebergCatalog.loadTable(TableIdentifier.of(dbName, tableName)); + + boolean hasSchemaChange = false; + boolean hasPropChange = false; + for (TableChange change : changes) { + if (change instanceof TableChange.ColumnChange) { + hasSchemaChange = true; + } else if (change instanceof TableChange.SetProperty + || change instanceof TableChange.RemoveProperty) { + hasPropChange = true; + } else { + throw new IllegalArgumentException( + "Unsupported table change for Iceberg table: " + change.getClass().getSimpleName()); + } + } + + if (hasSchemaChange) { + UpdateSchema update = table.updateSchema(); + for (TableChange change : changes) { + if (change instanceof TableChange.AddColumn) { + TableChange.AddColumn add = (TableChange.AddColumn) change; + Preconditions.checkArgument( + add.fieldName().length == 1, "Nested column additions are not supported"); + update.addColumn(add.fieldName()[0], toIcebergType(add.getDataType()), add.getComment()); + if (!add.isNullable()) { + update.requireColumn(add.fieldName()[0]); + } + } else if (change instanceof TableChange.DeleteColumn) { + TableChange.DeleteColumn del = (TableChange.DeleteColumn) change; + Preconditions.checkArgument( + del.fieldName().length == 1, "Nested column deletions are not supported"); + if (del.getIfExists()) { + try { + update.deleteColumn(del.fieldName()[0]); + } catch (IllegalArgumentException e) { + // Column does not exist; ignore as requested by ifExists=true. + } + } else { + update.deleteColumn(del.fieldName()[0]); + } + } else if (change instanceof TableChange.RenameColumn) { + TableChange.RenameColumn rename = (TableChange.RenameColumn) change; + Preconditions.checkArgument( + rename.fieldName().length == 1, "Nested column renames are not supported"); + update.renameColumn(rename.fieldName()[0], rename.getNewName()); + } else if (change instanceof TableChange.UpdateColumnType) { + TableChange.UpdateColumnType upd = (TableChange.UpdateColumnType) change; + Preconditions.checkArgument( + upd.fieldName().length == 1, "Nested column type updates are not supported"); + update.updateColumn( + upd.fieldName()[0], + (org.apache.iceberg.types.Type.PrimitiveType) toIcebergType(upd.getNewDataType())); + } else if (change instanceof TableChange.UpdateColumnComment) { + TableChange.UpdateColumnComment upd = (TableChange.UpdateColumnComment) change; + Preconditions.checkArgument( + upd.fieldName().length == 1, "Nested column comment updates are not supported"); + update.updateColumnDoc(upd.fieldName()[0], upd.getNewComment()); + } else if (change instanceof TableChange.UpdateColumnNullability) { + TableChange.UpdateColumnNullability upd = (TableChange.UpdateColumnNullability) change; + Preconditions.checkArgument( + upd.fieldName().length == 1, "Nested column nullability updates are not supported"); + if (upd.nullable()) { + update.makeColumnOptional(upd.fieldName()[0]); + } else { + update.requireColumn(upd.fieldName()[0]); + } + } + } + update.commit(); + LOG.info("Altered Iceberg table {}.{} schema via Iceberg SDK", dbName, tableName); + } + + if (hasPropChange) { + UpdateProperties update = table.updateProperties(); + for (TableChange change : changes) { + if (change instanceof TableChange.SetProperty) { + TableChange.SetProperty sp = (TableChange.SetProperty) change; + update.set(sp.getProperty(), sp.getValue()); + } else if (change instanceof TableChange.RemoveProperty) { + TableChange.RemoveProperty rp = (TableChange.RemoveProperty) change; + update.remove(rp.getProperty()); + } + } + update.commit(); + LOG.info("Altered Iceberg table {}.{} properties via Iceberg SDK", dbName, tableName); + } + } + + // --------------------------------------------------------------------------- + // Partition / sort conversion (Iceberg -> Gravitino) + // --------------------------------------------------------------------------- + + /** + * Converts an Iceberg {@link org.apache.iceberg.PartitionSpec} to Gravitino {@link Transform}s. + * + * <p>Supports identity, year, month, day, hour, bucket, and truncate transforms. + */ + static Transform[] convertPartitionSpec( + org.apache.iceberg.PartitionSpec spec, org.apache.iceberg.Schema schema) { + return spec.fields().stream() + .map( + field -> { + String colName = schema.findColumnName(field.sourceId()); + String transformStr = field.transform().toString().toLowerCase(Locale.ROOT); + if (transformStr.startsWith("identity")) { + return Transforms.identity(colName); + } else if (transformStr.startsWith("year")) { + return Transforms.year(colName); + } else if (transformStr.startsWith("month")) { + return Transforms.month(colName); + } else if (transformStr.startsWith("day")) { + return Transforms.day(colName); + } else if (transformStr.startsWith("hour")) { + return Transforms.hour(colName); + } else if (transformStr.startsWith("bucket")) { + int numBuckets = extractTransformParam(field.transform().toString()); + return Transforms.bucket(numBuckets, new String[] {colName}); + } else if (transformStr.startsWith("truncate")) { + int width = extractTransformParam(field.transform().toString()); + return Transforms.truncate(width, new String[] {colName}); + } else { + throw new IllegalArgumentException( + "Unsupported partition transform: " + transformStr); + } + }) + .toArray(Transform[]::new); + } + + /** Converts an Iceberg {@link org.apache.iceberg.SortOrder} to Gravitino {@link SortOrder}s. */ + static SortOrder[] convertIcebergSortOrder( + org.apache.iceberg.SortOrder iceSortOrder, org.apache.iceberg.Schema schema) { + if (iceSortOrder == null || iceSortOrder.fields().isEmpty()) { + return new SortOrder[0]; + } + return iceSortOrder.fields().stream() + .map( + field -> { + String colName = schema.findColumnName(field.sourceId()); + SortDirection direction = + field.direction() == org.apache.iceberg.SortDirection.ASC + ? SortDirection.ASCENDING + : SortDirection.DESCENDING; + NullOrdering nullOrdering = + field.nullOrder() == org.apache.iceberg.NullOrder.NULLS_FIRST + ? NullOrdering.NULLS_FIRST + : NullOrdering.NULLS_LAST; + + org.apache.iceberg.transforms.Transform<?, ?> transform = field.transform(); + String transformStr = transform.toString().toLowerCase(Locale.ROOT); + if (transformStr.startsWith("identity")) { + return SortOrders.of(NamedReference.field(colName), direction, nullOrdering); + } + + Expression expr; + if (transformStr.startsWith("year")) { + expr = FunctionExpression.of("year", NamedReference.field(colName)); + } else if (transformStr.startsWith("month")) { + expr = FunctionExpression.of("month", NamedReference.field(colName)); + } else if (transformStr.startsWith("day")) { + expr = FunctionExpression.of("day", NamedReference.field(colName)); + } else if (transformStr.startsWith("hour")) { + expr = FunctionExpression.of("hour", NamedReference.field(colName)); + } else if (transformStr.startsWith("bucket")) { + int numBuckets = extractTransformParam(transformStr); + expr = + FunctionExpression.of( + "bucket", + Literals.integerLiteral(numBuckets), + NamedReference.field(colName)); + } else if (transformStr.startsWith("truncate")) { + int width = extractTransformParam(transformStr); + expr = + FunctionExpression.of( + "truncate", Literals.integerLiteral(width), NamedReference.field(colName)); + } else { + expr = NamedReference.field(colName); + } + return SortOrders.of(expr, direction, nullOrdering); + }) + .toArray(SortOrder[]::new); + } + + // --------------------------------------------------------------------------- + // Type conversion (Gravitino -> Iceberg) + // --------------------------------------------------------------------------- + + private static Schema toIcebergSchema(Column[] columns) { + List<org.apache.iceberg.types.Types.NestedField> fields = new java.util.ArrayList<>(); Review Comment: Use a normal import for `ArrayList` instead of an inline fully-qualified class name. The project guidelines explicitly prohibit FQNs inside Java methods when there is no name collision (AGENTS.md:26-29), so this should be `import java.util.ArrayList;` plus `new ArrayList<>()`. ########## catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestAwsGlueCatalogOperations.java: ########## @@ -0,0 +1,226 @@ +/* + * 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.catalog.glue; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.HashMap; +import java.util.Map; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.TableChange; +import org.apache.gravitino.rel.expressions.distributions.Distributions; +import org.apache.gravitino.rel.expressions.sorts.SortOrders; +import org.apache.gravitino.rel.expressions.transforms.Transforms; +import org.apache.gravitino.rel.indexes.Indexes; +import org.apache.gravitino.rel.types.Types; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; + +/** + * Integration tests for {@link GlueCatalogOperations} against a real AWS Glue endpoint. + * + * <p>Only runs when {@code AWS_ACCESS_KEY_ID} is set. Required environment variables: + * + * <ul> + * <li>{@code AWS_ACCESS_KEY_ID} + * <li>{@code AWS_SECRET_ACCESS_KEY} + * <li>{@code AWS_DEFAULT_REGION} (e.g. {@code ap-northeast-1}) + * <li>{@code GLUE_CATALOG_ID} (12-digit AWS account ID; optional) + * </ul> + */ +@EnabledIfEnvironmentVariable(named = "AWS_ACCESS_KEY_ID", matches = ".+") +class TestAwsGlueCatalogOperations { + + private static GlueCatalogOperations ops; + private static final String TEST_SCHEMA = "aws_glue_ops_it_" + System.currentTimeMillis(); + private static final Namespace NS = Namespace.of("metalake", "catalog"); + + @BeforeAll + static void setup() { + Map<String, String> config = new HashMap<>(); + config.put( + GlueConstants.AWS_REGION, System.getenv().getOrDefault("AWS_DEFAULT_REGION", "us-east-1")); + String accessKey = System.getenv("AWS_ACCESS_KEY_ID"); + String secretKey = System.getenv("AWS_SECRET_ACCESS_KEY"); + if (accessKey != null) { + config.put(GlueConstants.AWS_ACCESS_KEY_ID, accessKey); + } + if (secretKey != null) { + config.put(GlueConstants.AWS_SECRET_ACCESS_KEY, secretKey); + } + String catalogId = System.getenv("GLUE_CATALOG_ID"); + if (catalogId != null) { + config.put(GlueConstants.AWS_GLUE_CATALOG_ID, catalogId); + } + + ops = new GlueCatalogOperations(); + ops.initialize(config, null, null); + + ops.createSchema(NameIdentifier.of(NS, TEST_SCHEMA), "IT schema", Map.of()); + } + + @AfterAll + static void teardown() { + if (ops == null) { + return; + } + try { + NameIdentifier[] tables = ops.listTables(Namespace.of(NS.level(0), NS.level(1), TEST_SCHEMA)); + for (NameIdentifier t : tables) { + ops.dropTable(t); + } + } catch (Exception ignored) { + } + try { + ops.dropSchema(NameIdentifier.of(NS, TEST_SCHEMA), true); + } catch (Exception ignored) { + } + ops.glueClient.close(); + } + + @Test + void testRenameTableIsUnsupported() { + String tableName = "rename_test_" + System.currentTimeMillis(); + NameIdentifier ident = NameIdentifier.of(NS.level(0), NS.level(1), TEST_SCHEMA, tableName); + + Column col = Column.of("id", Types.IntegerType.get(), "pk"); + ops.createTable( + ident, + new Column[] {col}, + "test table", + Map.of(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + SortOrders.NONE, + Indexes.EMPTY_INDEXES); + + try { + assertThrows( + UnsupportedOperationException.class, + () -> ops.alterTable(ident, TableChange.rename("new_name"))); + } finally { + ops.dropTable(ident); + } + } + + @Test + void testCreateIcebergTable() { + String tableName = "iceberg_create_" + System.currentTimeMillis(); + NameIdentifier ident = NameIdentifier.of(NS.level(0), NS.level(1), TEST_SCHEMA, tableName); + + Column col = Column.of("id", Types.LongType.get(), "pk", false, false, null); + Map<String, String> props = new HashMap<>(); + props.put(GlueConstants.TABLE_FORMAT, "ICEBERG"); + props.put(GlueConstants.LOCATION, "s3://ice-glue-test-01/iceberg/" + tableName); + + try { + Table created = + ops.createTable( + ident, + new Column[] {col}, + "iceberg table", + props, + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + SortOrders.NONE, + Indexes.EMPTY_INDEXES); + + assertEquals("iceberg table", created.comment()); + assertEquals(1, created.columns().length); + assertEquals("id", created.columns()[0].name()); + assertEquals(Types.LongType.get(), created.columns()[0].dataType()); + + Table loaded = ops.loadTable(ident); + assertEquals("ICEBERG", loaded.properties().get(GlueConstants.TABLE_FORMAT)); + assertNotNull(loaded.properties().get(GlueConstants.METADATA_LOCATION)); + } finally { + ops.dropTable(ident); + } + } + + @Test + void testAlterIcebergTableAddColumn() { + String tableName = "iceberg_alter_col_" + System.currentTimeMillis(); + NameIdentifier ident = NameIdentifier.of(NS.level(0), NS.level(1), TEST_SCHEMA, tableName); + + Column col = Column.of("id", Types.LongType.get(), "pk", false, false, null); + Map<String, String> props = new HashMap<>(); + props.put(GlueConstants.TABLE_FORMAT, "ICEBERG"); + props.put(GlueConstants.LOCATION, "s3://ice-glue-test-01/iceberg/" + tableName); Review Comment: This test also uses the hard-coded `ice-glue-test-01` bucket for real AWS writes. Use a caller-provided test bucket (for example `AWS_S3_TEST_BUCKET`) so the test does not fail or write to the wrong account-specific location when AWS credentials are present. -- 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]
