You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "lawofcycles (via GitHub)" <gi...@apache.org> on 2025/03/16 08:57:33 UTC
[PR] WIP: add view support for Glue Catalog [iceberg]
lawofcycles opened a new pull request, #12544:
URL: https://github.com/apache/iceberg/pull/12544
Adds support for creating and managing Iceberg views in the AWS Glue Catalog implementation. Although AWS Glue recently introduced a REST Catalog endpoint, it currently does not support creating Iceberg views:
```
java.lang.UnsupportedOperationException: Server does not support endpoint: POST /v1/{prefix}/namespaces/{namespace}/views
```
Even after the REST endpoint adds view support, many users still rely on the older Glue Catalog implementation, which view functionality is currently not supported. Then, this feature addition will enable users to create and manage Iceberg views through Glue Catalog as Iceberg Catalog.
Fixes https://github.com/apache/iceberg/issues/12488
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "lawofcycles (via GitHub)" <gi...@apache.org>.
lawofcycles commented on PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#issuecomment-2833495607
Hi @jackye1995 @geruh when you have a moment, could you please review this PR?
Thanks!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#discussion_r2052002017
##########
aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java:
##########
@@ -679,4 +680,140 @@ public void testTableLevelS3TagProperties() {
S3FileIOProperties.S3_TAG_ICEBERG_NAMESPACE),
"db");
}
+
+ @Test
+ public void testDropView() {
+ TableIdentifier viewIdent = TableIdentifier.of("db", "drop_view");
+ Table glueView =
+ Table.builder()
+ .databaseName("db")
+ .name("drop_view")
+ .tableType("VIRTUAL_VIEW")
+ .parameters(
+ ImmutableMap.of(
+ "table_type", "iceberg-view", "metadata_location", "s3://xx/metadata.json"))
+ .build();
+
+ Mockito.doReturn(GetTableResponse.builder().table(glueView).build())
+ .when(glue)
+ .getTable(Mockito.any(GetTableRequest.class));
+ Mockito.doReturn(DeleteTableResponse.builder().build())
+ .when(glue)
+ .deleteTable(Mockito.any(DeleteTableRequest.class));
+
+ boolean dropped = glueCatalog.dropView(viewIdent);
+ assertThat(dropped).isTrue();
+
+ Mockito.verify(glue, Mockito.times(1))
+ .deleteTable(
+ Mockito.argThat(
+ (DeleteTableRequest r) ->
+ r.databaseName().equals("db") && r.name().equals("drop_view")));
+ }
+
+ @Test
+ public void testDropViewNotFound() {
+ TableIdentifier viewIdent = TableIdentifier.of("db", "no_view");
+ Mockito.doThrow(EntityNotFoundException.builder().build())
+ .when(glue)
+ .getTable(Mockito.any(GetTableRequest.class));
+
+ boolean result = glueCatalog.dropView(viewIdent);
+ assertThat(result).isFalse();
+ }
+
+ @Test
+ public void testDropViewButItsNotView() {
+ TableIdentifier viewIdent = TableIdentifier.of("db", "not_view");
+ Table glueTable =
+ Table.builder()
+ .databaseName("db")
+ .name("not_view")
+ .tableType("EXTERNAL_TABLE")
+ .parameters(ImmutableMap.of("table_type", "iceberg-table")) // not "iceberg-view"
+ .build();
+
+ Mockito.doReturn(GetTableResponse.builder().table(glueTable).build())
+ .when(glue)
+ .getTable(Mockito.any(GetTableRequest.class));
+
+ boolean dropped = glueCatalog.dropView(viewIdent);
+ assertThat(dropped).isFalse();
+ }
+
+ @Test
+ public void testListViews() {
+ Mockito.doReturn(
+ GetTablesResponse.builder()
+ .tableList(
+ Table.builder()
+ .databaseName("db")
+ .name("my_view")
+ .tableType("VIRTUAL_VIEW")
+ .parameters(
+ ImmutableMap.of(
+ "table_type",
+ "iceberg-view",
+ "metadata_location",
+ "s3://v1/metadata.json"))
+ .build(),
+ Table.builder()
+ .databaseName("db")
+ .name("my_table")
+ .tableType("EXTERNAL_TABLE")
+ .parameters(ImmutableMap.of("table_type", "iceberg-table"))
+ .build())
+ .build())
+ .when(glue)
+ .getTables(Mockito.any(GetTablesRequest.class));
+
+ List<TableIdentifier> views = glueCatalog.listViews(Namespace.of("db"));
+ assertThat(views).hasSize(1);
+ assertThat(views.get(0)).isEqualTo(TableIdentifier.of("db", "my_view"));
Review Comment:
```suggestion
assertThat(views)
.singleElement()
.isEqualTo(TableIdentifier.of("db", "my_view"));
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] WIP: add view support for Glue Catalog [iceberg]
Posted by "lawofcycles (via GitHub)" <gi...@apache.org>.
lawofcycles commented on PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#issuecomment-2727276770
I've confirmed that the primary functionality works as expected using both integration tests and an actual AWS environment. I plan to complete the following tasks, and once they're finished, I'll request a review:
Investigate potential integration with Glue's multi-dialect view
Review and refine the overall code flow
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#issuecomment-2978535769
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "lawofcycles (via GitHub)" <gi...@apache.org>.
lawofcycles commented on code in PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#discussion_r2067587100
##########
aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogView.java:
##########
@@ -0,0 +1,883 @@
+/*
+ * 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.iceberg.aws.glue;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.iceberg.BaseMetastoreTableOperations;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.aws.AwsClientFactories;
+import org.apache.iceberg.aws.AwsClientFactory;
+import org.apache.iceberg.aws.AwsIntegTestUtil;
+import org.apache.iceberg.aws.AwsProperties;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.view.BaseView;
+import org.apache.iceberg.view.ImmutableSQLViewRepresentation;
+import org.apache.iceberg.view.ImmutableViewVersion;
+import org.apache.iceberg.view.SQLViewRepresentation;
+import org.apache.iceberg.view.View;
+import org.apache.iceberg.view.ViewCatalogTests;
+import org.apache.iceberg.view.ViewHistoryEntry;
+import org.apache.iceberg.view.ViewMetadata;
+import org.apache.iceberg.view.ViewProperties;
+import org.apache.iceberg.view.ViewUtil;
+import org.assertj.core.api.Assumptions;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
+import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariables;
+import software.amazon.awssdk.services.glue.GlueClient;
+import software.amazon.awssdk.services.glue.model.CreateTableRequest;
+import software.amazon.awssdk.services.glue.model.DeleteTableRequest;
+import software.amazon.awssdk.services.glue.model.EntityNotFoundException;
+import software.amazon.awssdk.services.glue.model.GetTableRequest;
+import software.amazon.awssdk.services.glue.model.GetTableResponse;
+import software.amazon.awssdk.services.glue.model.Table;
+import software.amazon.awssdk.services.glue.model.TableInput;
+import software.amazon.awssdk.services.s3.S3Client;
+
+@EnabledIfEnvironmentVariables({
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_ACCESS_KEY_ID, matches = ".*"),
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_SECRET_ACCESS_KEY, matches = ".*"),
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_SESSION_TOKEN, matches = ".*"),
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_REGION, matches = ".*"),
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_TEST_BUCKET, matches = ".*")
+})
+public class TestGlueCatalogView extends ViewCatalogTests<GlueCatalog> {
+
+ protected static final String TEST_BUCKET_NAME = AwsIntegTestUtil.testBucketName();
+ protected static final String CATALOG_NAME = "glue";
+ protected static final String TEST_PATH_PREFIX = getRandomName();
+ protected static final List<String> NAMESPACES = Lists.newArrayList();
+
+ protected static final AwsClientFactory CLIENT_FACTORY = AwsClientFactories.defaultFactory();
+ protected static final GlueClient GLUE = CLIENT_FACTORY.glue();
+ protected static final S3Client S3 = CLIENT_FACTORY.s3();
+
+ private static GlueCatalog glueCatalog;
+ protected static final String TEST_BUCKET_PATH =
+ "s3://" + TEST_BUCKET_NAME + "/" + TEST_PATH_PREFIX;
+
+ @BeforeEach
+ public void beforeEach() {
+ AwsProperties properties = new AwsProperties();
+ properties.setGlueCatalogSkipNameValidation(true);
+ glueCatalog = new GlueCatalog();
+ Map<String, String> catalogProps = Maps.newHashMap();
+ catalogProps.put(CatalogProperties.VIEW_DEFAULT_PREFIX + "key1", "catalog-default-key1");
+ catalogProps.put(CatalogProperties.VIEW_DEFAULT_PREFIX + "key2", "catalog-default-key2");
+ catalogProps.put(CatalogProperties.VIEW_DEFAULT_PREFIX + "key3", "catalog-default-key3");
+ catalogProps.put(CatalogProperties.VIEW_OVERRIDE_PREFIX + "key3", "catalog-override-key3");
+ catalogProps.put(CatalogProperties.VIEW_OVERRIDE_PREFIX + "key4", "catalog-override-key4");
+ glueCatalog.initialize(
+ CATALOG_NAME,
+ TEST_BUCKET_PATH,
+ properties,
+ new org.apache.iceberg.aws.s3.S3FileIOProperties(),
+ GLUE,
+ null,
+ catalogProps);
+
+ try {
+ Namespace ns = Namespace.of("ns");
+ if (!glueCatalog.namespaceExists(ns)) {
+ glueCatalog.createNamespace(ns);
+ NAMESPACES.add("ns");
+ }
+
+ Namespace ns1 = Namespace.of("ns1");
+ if (!glueCatalog.namespaceExists(ns1)) {
+ glueCatalog.createNamespace(ns1);
+ NAMESPACES.add("ns1");
+ }
+
+ Namespace ns2 = Namespace.of("ns2");
+ if (!glueCatalog.namespaceExists(ns2)) {
+ glueCatalog.createNamespace(ns2);
+ NAMESPACES.add("ns2");
+ }
+ } catch (Exception e) {
+ // Failure to create namespace will be caught by test assertions
+ }
+ cleanupTestData();
+ }
+
+ private void cleanupTestData() {
+ deleteCommonGlueTables();
+ cleanupViewsInNamespaces("ns", "ns1", "ns2");
+ }
+
+ private void deleteCommonGlueTables() {
+ deleteTableIgnoringNotFound("view");
+ deleteTableIgnoringNotFound("renamedView");
+ }
+
+ private void cleanupViewsInNamespaces(String... namespaces) {
+ for (String nsStr : namespaces) {
+ Namespace ns = Namespace.of(nsStr);
+ if (catalog().namespaceExists(ns)) {
+ cleanupNamespace(ns);
+ }
+ }
+ }
+
+ private void cleanupNamespace(Namespace ns) {
+ for (TableIdentifier view : catalog().listViews(ns)) {
+ catalog().dropView(view);
+ }
+ for (TableIdentifier table : catalog().listTables(ns)) {
+ catalog().dropTable(table);
+ }
+ }
+
+ private void deleteTableIgnoringNotFound(String tableName) {
+ try {
+ GLUE.deleteTable(
+ DeleteTableRequest.builder()
+ .catalogId(awsProperties().glueCatalogId())
+ .databaseName("ns")
+ .name(tableName)
+ .build());
+ } catch (EntityNotFoundException e) {
+ // Entity doesn't exist - this is one of the expected states
+ }
+ }
+
+ @AfterAll
+ public static void afterClass() {
+ AwsIntegTestUtil.cleanGlueCatalog(GLUE, NAMESPACES);
+ AwsIntegTestUtil.cleanS3GeneralPurposeBucket(S3, TEST_BUCKET_NAME, TEST_PATH_PREFIX);
+ }
+
+ @Override
+ protected GlueCatalog catalog() {
+ return glueCatalog;
+ }
+
+ @Override
+ protected Catalog tableCatalog() {
+ return glueCatalog;
+ }
+
+ protected static String getRandomName() {
+ return UUID.randomUUID().toString().replace("-", "");
+ }
+
+ protected String createNamespace() {
+ String namespace = getRandomName();
+ NAMESPACES.add(namespace);
+ glueCatalog.createNamespace(Namespace.of(namespace));
+ return namespace;
+ }
+
+ protected AwsProperties awsProperties() {
+ return new AwsProperties();
+ }
+
+ @Override
+ @Test
+ public void completeCreateView() {
+ TableIdentifier identifier = TableIdentifier.of("ns", "view");
+
+ if (requiresNamespaceCreate()) {
+ catalog().createNamespace(identifier.namespace());
+ }
+
+ assertThat(catalog().viewExists(identifier)).as("View should not exist").isFalse();
+
+ String location = String.format("s3://%s/%s/ns/view", TEST_BUCKET_NAME, TEST_PATH_PREFIX);
+
+ View view =
+ catalog()
+ .buildView(identifier)
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(identifier.namespace())
+ .withDefaultCatalog(catalog().name())
+ .withQuery("spark", "select * from ns.tbl")
+ .withQuery("trino", "select * from ns.tbl using X")
+ .withProperty("prop1", "val1")
+ .withProperty("prop2", "val2")
+ .withLocation(location)
+ .create();
+
+ assertThat(view).isNotNull();
+ assertThat(catalog().viewExists(identifier)).as("View should exist").isTrue();
+ assertThat(((BaseView) view).operations().current().metadataFileLocation()).isNotNull();
+
+ if (!overridesRequestedLocation()) {
+ assertThat(view.location()).isEqualTo(location);
+ } else {
+ assertThat(view.location()).isNotNull();
+ }
+
+ // validate view settings
+ assertThat(view.uuid())
+ .isEqualTo(UUID.fromString(((BaseView) view).operations().current().uuid()));
+ assertThat(view.name()).isEqualTo(ViewUtil.fullViewName(catalog().name(), identifier));
+ assertThat(view.properties()).containsEntry("prop1", "val1").containsEntry("prop2", "val2");
+ assertThat(view.history())
+ .hasSize(1)
+ .first()
Review Comment:
addressed
##########
aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java:
##########
@@ -679,4 +680,140 @@ public void testTableLevelS3TagProperties() {
S3FileIOProperties.S3_TAG_ICEBERG_NAMESPACE),
"db");
}
+
+ @Test
+ public void testDropView() {
+ TableIdentifier viewIdent = TableIdentifier.of("db", "drop_view");
+ Table glueView =
+ Table.builder()
+ .databaseName("db")
+ .name("drop_view")
+ .tableType("VIRTUAL_VIEW")
+ .parameters(
+ ImmutableMap.of(
+ "table_type", "iceberg-view", "metadata_location", "s3://xx/metadata.json"))
+ .build();
+
+ Mockito.doReturn(GetTableResponse.builder().table(glueView).build())
+ .when(glue)
+ .getTable(Mockito.any(GetTableRequest.class));
+ Mockito.doReturn(DeleteTableResponse.builder().build())
+ .when(glue)
+ .deleteTable(Mockito.any(DeleteTableRequest.class));
+
+ boolean dropped = glueCatalog.dropView(viewIdent);
+ assertThat(dropped).isTrue();
+
+ Mockito.verify(glue, Mockito.times(1))
+ .deleteTable(
+ Mockito.argThat(
+ (DeleteTableRequest r) ->
+ r.databaseName().equals("db") && r.name().equals("drop_view")));
+ }
+
+ @Test
+ public void testDropViewNotFound() {
+ TableIdentifier viewIdent = TableIdentifier.of("db", "no_view");
+ Mockito.doThrow(EntityNotFoundException.builder().build())
+ .when(glue)
+ .getTable(Mockito.any(GetTableRequest.class));
+
+ boolean result = glueCatalog.dropView(viewIdent);
+ assertThat(result).isFalse();
+ }
+
+ @Test
+ public void testDropViewButItsNotView() {
+ TableIdentifier viewIdent = TableIdentifier.of("db", "not_view");
+ Table glueTable =
+ Table.builder()
+ .databaseName("db")
+ .name("not_view")
+ .tableType("EXTERNAL_TABLE")
+ .parameters(ImmutableMap.of("table_type", "iceberg-table")) // not "iceberg-view"
+ .build();
+
+ Mockito.doReturn(GetTableResponse.builder().table(glueTable).build())
+ .when(glue)
+ .getTable(Mockito.any(GetTableRequest.class));
+
+ boolean dropped = glueCatalog.dropView(viewIdent);
+ assertThat(dropped).isFalse();
+ }
+
+ @Test
+ public void testListViews() {
+ Mockito.doReturn(
+ GetTablesResponse.builder()
+ .tableList(
+ Table.builder()
+ .databaseName("db")
+ .name("my_view")
+ .tableType("VIRTUAL_VIEW")
+ .parameters(
+ ImmutableMap.of(
+ "table_type",
+ "iceberg-view",
+ "metadata_location",
+ "s3://v1/metadata.json"))
+ .build(),
+ Table.builder()
+ .databaseName("db")
+ .name("my_table")
+ .tableType("EXTERNAL_TABLE")
+ .parameters(ImmutableMap.of("table_type", "iceberg-table"))
+ .build())
+ .build())
+ .when(glue)
+ .getTables(Mockito.any(GetTablesRequest.class));
+
+ List<TableIdentifier> views = glueCatalog.listViews(Namespace.of("db"));
+ assertThat(views).hasSize(1);
+ assertThat(views.get(0)).isEqualTo(TableIdentifier.of("db", "my_view"));
Review Comment:
addressed
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #12544: AWS: add view support for Glue Catalog
URL: https://github.com/apache/iceberg/pull/12544
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "lawofcycles (via GitHub)" <gi...@apache.org>.
lawofcycles commented on PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#issuecomment-2816993546
@nastra Hi, when you have a moment, could you please review this PR? If you think anyone else should be looped in, just let me know. No rush—whenever it fits your schedule. Thanks!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: WIP: add view support for Glue Catalog [iceberg]
Posted by "lawofcycles (via GitHub)" <gi...@apache.org>.
lawofcycles commented on PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#issuecomment-2798790816
I explored adding Lake Formation Multi Dialect Views.
However, it introduce several requirements, including:
- Only IAM roles can create Data Catalog views (no other IAM entities).
- The defining role must have Lake Formation SELECT with Grantable on all reference tables.
- A trust policy must allow Lake Formation and AWS Glue to assume the defining role.
- S3 data locations must be registered with Lake Formation.
These constraints fall outside the scope of this PR, which focuses on enabling basic Iceberg view support in the Glue Data Catalog. We may revisit MDVs in the future as a potential enhancement if these requirements become more manageable.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#discussion_r2052002017
##########
aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java:
##########
@@ -679,4 +680,140 @@ public void testTableLevelS3TagProperties() {
S3FileIOProperties.S3_TAG_ICEBERG_NAMESPACE),
"db");
}
+
+ @Test
+ public void testDropView() {
+ TableIdentifier viewIdent = TableIdentifier.of("db", "drop_view");
+ Table glueView =
+ Table.builder()
+ .databaseName("db")
+ .name("drop_view")
+ .tableType("VIRTUAL_VIEW")
+ .parameters(
+ ImmutableMap.of(
+ "table_type", "iceberg-view", "metadata_location", "s3://xx/metadata.json"))
+ .build();
+
+ Mockito.doReturn(GetTableResponse.builder().table(glueView).build())
+ .when(glue)
+ .getTable(Mockito.any(GetTableRequest.class));
+ Mockito.doReturn(DeleteTableResponse.builder().build())
+ .when(glue)
+ .deleteTable(Mockito.any(DeleteTableRequest.class));
+
+ boolean dropped = glueCatalog.dropView(viewIdent);
+ assertThat(dropped).isTrue();
+
+ Mockito.verify(glue, Mockito.times(1))
+ .deleteTable(
+ Mockito.argThat(
+ (DeleteTableRequest r) ->
+ r.databaseName().equals("db") && r.name().equals("drop_view")));
+ }
+
+ @Test
+ public void testDropViewNotFound() {
+ TableIdentifier viewIdent = TableIdentifier.of("db", "no_view");
+ Mockito.doThrow(EntityNotFoundException.builder().build())
+ .when(glue)
+ .getTable(Mockito.any(GetTableRequest.class));
+
+ boolean result = glueCatalog.dropView(viewIdent);
+ assertThat(result).isFalse();
+ }
+
+ @Test
+ public void testDropViewButItsNotView() {
+ TableIdentifier viewIdent = TableIdentifier.of("db", "not_view");
+ Table glueTable =
+ Table.builder()
+ .databaseName("db")
+ .name("not_view")
+ .tableType("EXTERNAL_TABLE")
+ .parameters(ImmutableMap.of("table_type", "iceberg-table")) // not "iceberg-view"
+ .build();
+
+ Mockito.doReturn(GetTableResponse.builder().table(glueTable).build())
+ .when(glue)
+ .getTable(Mockito.any(GetTableRequest.class));
+
+ boolean dropped = glueCatalog.dropView(viewIdent);
+ assertThat(dropped).isFalse();
+ }
+
+ @Test
+ public void testListViews() {
+ Mockito.doReturn(
+ GetTablesResponse.builder()
+ .tableList(
+ Table.builder()
+ .databaseName("db")
+ .name("my_view")
+ .tableType("VIRTUAL_VIEW")
+ .parameters(
+ ImmutableMap.of(
+ "table_type",
+ "iceberg-view",
+ "metadata_location",
+ "s3://v1/metadata.json"))
+ .build(),
+ Table.builder()
+ .databaseName("db")
+ .name("my_table")
+ .tableType("EXTERNAL_TABLE")
+ .parameters(ImmutableMap.of("table_type", "iceberg-table"))
+ .build())
+ .build())
+ .when(glue)
+ .getTables(Mockito.any(GetTablesRequest.class));
+
+ List<TableIdentifier> views = glueCatalog.listViews(Namespace.of("db"));
+ assertThat(views).hasSize(1);
+ assertThat(views.get(0)).isEqualTo(TableIdentifier.of("db", "my_view"));
Review Comment:
```suggestion
assertThat(views)
.hasSize(1)
.containsExactly(TableIdentifier.of("db", "my_view"));
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#discussion_r2051999502
##########
aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogView.java:
##########
@@ -0,0 +1,883 @@
+/*
+ * 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.iceberg.aws.glue;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.iceberg.BaseMetastoreTableOperations;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.aws.AwsClientFactories;
+import org.apache.iceberg.aws.AwsClientFactory;
+import org.apache.iceberg.aws.AwsIntegTestUtil;
+import org.apache.iceberg.aws.AwsProperties;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.view.BaseView;
+import org.apache.iceberg.view.ImmutableSQLViewRepresentation;
+import org.apache.iceberg.view.ImmutableViewVersion;
+import org.apache.iceberg.view.SQLViewRepresentation;
+import org.apache.iceberg.view.View;
+import org.apache.iceberg.view.ViewCatalogTests;
+import org.apache.iceberg.view.ViewHistoryEntry;
+import org.apache.iceberg.view.ViewMetadata;
+import org.apache.iceberg.view.ViewProperties;
+import org.apache.iceberg.view.ViewUtil;
+import org.assertj.core.api.Assumptions;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
+import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariables;
+import software.amazon.awssdk.services.glue.GlueClient;
+import software.amazon.awssdk.services.glue.model.CreateTableRequest;
+import software.amazon.awssdk.services.glue.model.DeleteTableRequest;
+import software.amazon.awssdk.services.glue.model.EntityNotFoundException;
+import software.amazon.awssdk.services.glue.model.GetTableRequest;
+import software.amazon.awssdk.services.glue.model.GetTableResponse;
+import software.amazon.awssdk.services.glue.model.Table;
+import software.amazon.awssdk.services.glue.model.TableInput;
+import software.amazon.awssdk.services.s3.S3Client;
+
+@EnabledIfEnvironmentVariables({
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_ACCESS_KEY_ID, matches = ".*"),
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_SECRET_ACCESS_KEY, matches = ".*"),
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_SESSION_TOKEN, matches = ".*"),
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_REGION, matches = ".*"),
+ @EnabledIfEnvironmentVariable(named = AwsIntegTestUtil.AWS_TEST_BUCKET, matches = ".*")
+})
+public class TestGlueCatalogView extends ViewCatalogTests<GlueCatalog> {
+
+ protected static final String TEST_BUCKET_NAME = AwsIntegTestUtil.testBucketName();
+ protected static final String CATALOG_NAME = "glue";
+ protected static final String TEST_PATH_PREFIX = getRandomName();
+ protected static final List<String> NAMESPACES = Lists.newArrayList();
+
+ protected static final AwsClientFactory CLIENT_FACTORY = AwsClientFactories.defaultFactory();
+ protected static final GlueClient GLUE = CLIENT_FACTORY.glue();
+ protected static final S3Client S3 = CLIENT_FACTORY.s3();
+
+ private static GlueCatalog glueCatalog;
+ protected static final String TEST_BUCKET_PATH =
+ "s3://" + TEST_BUCKET_NAME + "/" + TEST_PATH_PREFIX;
+
+ @BeforeEach
+ public void beforeEach() {
+ AwsProperties properties = new AwsProperties();
+ properties.setGlueCatalogSkipNameValidation(true);
+ glueCatalog = new GlueCatalog();
+ Map<String, String> catalogProps = Maps.newHashMap();
+ catalogProps.put(CatalogProperties.VIEW_DEFAULT_PREFIX + "key1", "catalog-default-key1");
+ catalogProps.put(CatalogProperties.VIEW_DEFAULT_PREFIX + "key2", "catalog-default-key2");
+ catalogProps.put(CatalogProperties.VIEW_DEFAULT_PREFIX + "key3", "catalog-default-key3");
+ catalogProps.put(CatalogProperties.VIEW_OVERRIDE_PREFIX + "key3", "catalog-override-key3");
+ catalogProps.put(CatalogProperties.VIEW_OVERRIDE_PREFIX + "key4", "catalog-override-key4");
+ glueCatalog.initialize(
+ CATALOG_NAME,
+ TEST_BUCKET_PATH,
+ properties,
+ new org.apache.iceberg.aws.s3.S3FileIOProperties(),
+ GLUE,
+ null,
+ catalogProps);
+
+ try {
+ Namespace ns = Namespace.of("ns");
+ if (!glueCatalog.namespaceExists(ns)) {
+ glueCatalog.createNamespace(ns);
+ NAMESPACES.add("ns");
+ }
+
+ Namespace ns1 = Namespace.of("ns1");
+ if (!glueCatalog.namespaceExists(ns1)) {
+ glueCatalog.createNamespace(ns1);
+ NAMESPACES.add("ns1");
+ }
+
+ Namespace ns2 = Namespace.of("ns2");
+ if (!glueCatalog.namespaceExists(ns2)) {
+ glueCatalog.createNamespace(ns2);
+ NAMESPACES.add("ns2");
+ }
+ } catch (Exception e) {
+ // Failure to create namespace will be caught by test assertions
+ }
+ cleanupTestData();
+ }
+
+ private void cleanupTestData() {
+ deleteCommonGlueTables();
+ cleanupViewsInNamespaces("ns", "ns1", "ns2");
+ }
+
+ private void deleteCommonGlueTables() {
+ deleteTableIgnoringNotFound("view");
+ deleteTableIgnoringNotFound("renamedView");
+ }
+
+ private void cleanupViewsInNamespaces(String... namespaces) {
+ for (String nsStr : namespaces) {
+ Namespace ns = Namespace.of(nsStr);
+ if (catalog().namespaceExists(ns)) {
+ cleanupNamespace(ns);
+ }
+ }
+ }
+
+ private void cleanupNamespace(Namespace ns) {
+ for (TableIdentifier view : catalog().listViews(ns)) {
+ catalog().dropView(view);
+ }
+ for (TableIdentifier table : catalog().listTables(ns)) {
+ catalog().dropTable(table);
+ }
+ }
+
+ private void deleteTableIgnoringNotFound(String tableName) {
+ try {
+ GLUE.deleteTable(
+ DeleteTableRequest.builder()
+ .catalogId(awsProperties().glueCatalogId())
+ .databaseName("ns")
+ .name(tableName)
+ .build());
+ } catch (EntityNotFoundException e) {
+ // Entity doesn't exist - this is one of the expected states
+ }
+ }
+
+ @AfterAll
+ public static void afterClass() {
+ AwsIntegTestUtil.cleanGlueCatalog(GLUE, NAMESPACES);
+ AwsIntegTestUtil.cleanS3GeneralPurposeBucket(S3, TEST_BUCKET_NAME, TEST_PATH_PREFIX);
+ }
+
+ @Override
+ protected GlueCatalog catalog() {
+ return glueCatalog;
+ }
+
+ @Override
+ protected Catalog tableCatalog() {
+ return glueCatalog;
+ }
+
+ protected static String getRandomName() {
+ return UUID.randomUUID().toString().replace("-", "");
+ }
+
+ protected String createNamespace() {
+ String namespace = getRandomName();
+ NAMESPACES.add(namespace);
+ glueCatalog.createNamespace(Namespace.of(namespace));
+ return namespace;
+ }
+
+ protected AwsProperties awsProperties() {
+ return new AwsProperties();
+ }
+
+ @Override
+ @Test
+ public void completeCreateView() {
+ TableIdentifier identifier = TableIdentifier.of("ns", "view");
+
+ if (requiresNamespaceCreate()) {
+ catalog().createNamespace(identifier.namespace());
+ }
+
+ assertThat(catalog().viewExists(identifier)).as("View should not exist").isFalse();
+
+ String location = String.format("s3://%s/%s/ns/view", TEST_BUCKET_NAME, TEST_PATH_PREFIX);
+
+ View view =
+ catalog()
+ .buildView(identifier)
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(identifier.namespace())
+ .withDefaultCatalog(catalog().name())
+ .withQuery("spark", "select * from ns.tbl")
+ .withQuery("trino", "select * from ns.tbl using X")
+ .withProperty("prop1", "val1")
+ .withProperty("prop2", "val2")
+ .withLocation(location)
+ .create();
+
+ assertThat(view).isNotNull();
+ assertThat(catalog().viewExists(identifier)).as("View should exist").isTrue();
+ assertThat(((BaseView) view).operations().current().metadataFileLocation()).isNotNull();
+
+ if (!overridesRequestedLocation()) {
+ assertThat(view.location()).isEqualTo(location);
+ } else {
+ assertThat(view.location()).isNotNull();
+ }
+
+ // validate view settings
+ assertThat(view.uuid())
+ .isEqualTo(UUID.fromString(((BaseView) view).operations().current().uuid()));
+ assertThat(view.name()).isEqualTo(ViewUtil.fullViewName(catalog().name(), identifier));
+ assertThat(view.properties()).containsEntry("prop1", "val1").containsEntry("prop2", "val2");
+ assertThat(view.history())
+ .hasSize(1)
+ .first()
Review Comment:
Nit: use `singleElement()` rather than `hasSize(1).first()`
```suggestion
assertThat(view.history())
singleElement()
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "lawofcycles (via GitHub)" <gi...@apache.org>.
lawofcycles commented on PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#issuecomment-2799590136
I ran the integration tests in my environment.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: add view support for Glue Catalog [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12544:
URL: https://github.com/apache/iceberg/pull/12544#issuecomment-2998354383
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org