You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ao...@apache.org on 2021/03/24 18:41:31 UTC
[iceberg] 16/18: AWS: Do not list non-iceberg table in GlueCatalog
(#2267)
This is an automated email from the ASF dual-hosted git repository.
aokolnychyi pushed a commit to branch 0.11.x
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit f1a972b01293a78577f272183b4867486c5aa6a0
Author: Jack Ye <yz...@amazon.com>
AuthorDate: Wed Mar 3 16:23:29 2021 -0800
AWS: Do not list non-iceberg table in GlueCatalog (#2267)
---
.../iceberg/aws/glue/GlueCatalogNamespaceTest.java | 24 +++++++-
.../org/apache/iceberg/aws/glue/GlueCatalog.java | 27 ++++++--
.../apache/iceberg/aws/glue/GlueCatalogTest.java | 72 +++++++++++++++++++---
3 files changed, 106 insertions(+), 17 deletions(-)
diff --git a/aws/src/integration/java/org/apache/iceberg/aws/glue/GlueCatalogNamespaceTest.java b/aws/src/integration/java/org/apache/iceberg/aws/glue/GlueCatalogNamespaceTest.java
index 9dc3ce5..d7f1058 100644
--- a/aws/src/integration/java/org/apache/iceberg/aws/glue/GlueCatalogNamespaceTest.java
+++ b/aws/src/integration/java/org/apache/iceberg/aws/glue/GlueCatalogNamespaceTest.java
@@ -21,6 +21,7 @@ package org.apache.iceberg.aws.glue;
import java.util.List;
import java.util.Map;
+import java.util.UUID;
import org.apache.iceberg.AssertHelpers;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.exceptions.AlreadyExistsException;
@@ -31,9 +32,11 @@ import org.apache.iceberg.relocated.com.google.common.collect.Maps;
import org.apache.iceberg.relocated.com.google.common.collect.Sets;
import org.junit.Assert;
import org.junit.Test;
+import software.amazon.awssdk.services.glue.model.CreateTableRequest;
import software.amazon.awssdk.services.glue.model.Database;
import software.amazon.awssdk.services.glue.model.EntityNotFoundException;
import software.amazon.awssdk.services.glue.model.GetDatabaseRequest;
+import software.amazon.awssdk.services.glue.model.TableInput;
public class GlueCatalogNamespaceTest extends GlueTestBase {
@@ -131,12 +134,27 @@ public class GlueCatalogNamespaceTest extends GlueTestBase {
}
@Test
- public void testDropNamespaceNonEmpty() {
+ public void testDropNamespaceNonEmpty_containsIcebergTable() {
String namespace = createNamespace();
createTable(namespace);
- AssertHelpers.assertThrows("namespace should not be dropped when still has table",
+ AssertHelpers.assertThrows("namespace should not be dropped when still has Iceberg table",
NamespaceNotEmptyException.class,
- "it is not empty",
+ "still contains Iceberg tables",
+ () -> glueCatalog.dropNamespace(Namespace.of(namespace)));
+ }
+
+ @Test
+ public void testDropNamespaceNonEmpty_containsNonIcebergTable() {
+ String namespace = createNamespace();
+ glue.createTable(CreateTableRequest.builder()
+ .databaseName(namespace)
+ .tableInput(TableInput.builder()
+ .name(UUID.randomUUID().toString())
+ .build())
+ .build());
+ AssertHelpers.assertThrows("namespace should not be dropped when still has non-Iceberg table",
+ NamespaceNotEmptyException.class,
+ "still contains non-Iceberg tables",
() -> glueCatalog.dropNamespace(Namespace.of(namespace)));
}
diff --git a/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java b/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java
index a4d407d..197a730 100644
--- a/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java
+++ b/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java
@@ -28,6 +28,7 @@ import java.util.stream.Collectors;
import org.apache.hadoop.conf.Configurable;
import org.apache.hadoop.conf.Configuration;
import org.apache.iceberg.BaseMetastoreCatalog;
+import org.apache.iceberg.BaseMetastoreTableOperations;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.CatalogUtil;
import org.apache.iceberg.TableMetadata;
@@ -176,6 +177,7 @@ public class GlueCatalog extends BaseMetastoreCatalog implements Closeable, Supp
nextToken = response.nextToken();
if (response.hasTableList()) {
results.addAll(response.tableList().stream()
+ .filter(this::isGlueIcebergTable)
.map(GlueToIcebergConverter::toTableId)
.collect(Collectors.toList()));
}
@@ -185,6 +187,12 @@ public class GlueCatalog extends BaseMetastoreCatalog implements Closeable, Supp
return results;
}
+ private boolean isGlueIcebergTable(Table table) {
+ return table.parameters() != null &&
+ BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase(
+ table.parameters().get(BaseMetastoreTableOperations.TABLE_TYPE_PROP));
+ }
+
@Override
public boolean dropTable(TableIdentifier identifier, boolean purge) {
try {
@@ -336,10 +344,21 @@ public class GlueCatalog extends BaseMetastoreCatalog implements Closeable, Supp
@Override
public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyException {
namespaceExists(namespace);
- List<TableIdentifier> tableIdentifiers = listTables(namespace);
- if (tableIdentifiers != null && !tableIdentifiers.isEmpty()) {
- throw new NamespaceNotEmptyException("Cannot drop namespace %s because it is not empty. " +
- "The following tables still exist under the namespace: %s", namespace, tableIdentifiers);
+
+ GetTablesResponse response = glue.getTables(GetTablesRequest.builder()
+ .catalogId(awsProperties.glueCatalogId())
+ .databaseName(IcebergToGlueConverter.toDatabaseName(namespace))
+ .build());
+
+ if (response.hasTableList() && !response.tableList().isEmpty()) {
+ Table table = response.tableList().get(0);
+ if (isGlueIcebergTable(table)) {
+ throw new NamespaceNotEmptyException(
+ "Cannot drop namespace %s because it still contains Iceberg tables", namespace);
+ } else {
+ throw new NamespaceNotEmptyException(
+ "Cannot drop namespace %s because it still contains non-Iceberg tables", namespace);
+ }
}
glue.deleteDatabase(DeleteDatabaseRequest.builder()
diff --git a/aws/src/test/java/org/apache/iceberg/aws/glue/GlueCatalogTest.java b/aws/src/test/java/org/apache/iceberg/aws/glue/GlueCatalogTest.java
index 95dfad6..669a73e 100644
--- a/aws/src/test/java/org/apache/iceberg/aws/glue/GlueCatalogTest.java
+++ b/aws/src/test/java/org/apache/iceberg/aws/glue/GlueCatalogTest.java
@@ -31,6 +31,7 @@ import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.NamespaceNotEmptyException;
import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.relocated.com.google.common.collect.Sets;
import org.junit.Assert;
@@ -123,8 +124,29 @@ public class GlueCatalogTest {
.when(glue).getDatabase(Mockito.any(GetDatabaseRequest.class));
Mockito.doReturn(GetTablesResponse.builder()
.tableList(
- Table.builder().databaseName("db1").name("t1").build(),
- Table.builder().databaseName("db1").name("t2").build()
+ Table.builder().databaseName("db1").name("t1").parameters(
+ ImmutableMap.of(
+ BaseMetastoreTableOperations.TABLE_TYPE_PROP, BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE
+ )
+ ).build(),
+ Table.builder().databaseName("db1").name("t2").parameters(
+ ImmutableMap.of(
+ "key", "val",
+ BaseMetastoreTableOperations.TABLE_TYPE_PROP, BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE
+ )
+ ).build(),
+ Table.builder().databaseName("db1").name("t3").parameters(
+ ImmutableMap.of(
+ "key", "val",
+ BaseMetastoreTableOperations.TABLE_TYPE_PROP, "wrongVal"
+ )
+ ).build(),
+ Table.builder().databaseName("db1").name("t4").parameters(
+ ImmutableMap.of(
+ "key", "val"
+ )
+ ).build(),
+ Table.builder().databaseName("db1").name("t5").parameters(null).build()
).build())
.when(glue).getTables(Mockito.any(GetTablesRequest.class));
Assert.assertEquals(
@@ -148,14 +170,23 @@ public class GlueCatalogTest {
if (counter.decrementAndGet() > 0) {
return GetTablesResponse.builder()
.tableList(
- Table.builder().databaseName("db1").name(
- UUID.randomUUID().toString().replace("-", "")).build()
+ Table.builder()
+ .databaseName("db1")
+ .name(UUID.randomUUID().toString().replace("-", ""))
+ .parameters(ImmutableMap.of(
+ BaseMetastoreTableOperations.TABLE_TYPE_PROP,
+ BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE
+ ))
+ .build()
)
.nextToken("token")
.build();
} else {
return GetTablesResponse.builder()
- .tableList(Table.builder().databaseName("db1").name("tb1").build())
+ .tableList(Table.builder().databaseName("db1").name("tb1").parameters(ImmutableMap.of(
+ BaseMetastoreTableOperations.TABLE_TYPE_PROP,
+ BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE
+ )).build())
.build();
}
}
@@ -313,11 +344,14 @@ public class GlueCatalogTest {
}
@Test
- public void dropNamespace_notEmpty() {
+ public void dropNamespace_notEmpty_containsIcebergTable() {
Mockito.doReturn(GetTablesResponse.builder()
.tableList(
- Table.builder().databaseName("db1").name("t1").build(),
- Table.builder().databaseName("db1").name("t2").build()
+ Table.builder().databaseName("db1").name("t1").parameters(
+ ImmutableMap.of(
+ BaseMetastoreTableOperations.TABLE_TYPE_PROP, BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE
+ )
+ ).build()
).build())
.when(glue).getTables(Mockito.any(GetTablesRequest.class));
Mockito.doReturn(GetDatabaseResponse.builder()
@@ -325,9 +359,27 @@ public class GlueCatalogTest {
.when(glue).getDatabase(Mockito.any(GetDatabaseRequest.class));
Mockito.doReturn(DeleteDatabaseResponse.builder().build())
.when(glue).deleteDatabase(Mockito.any(DeleteDatabaseRequest.class));
- AssertHelpers.assertThrows("namespace should not be dropped when still has table",
+ AssertHelpers.assertThrows("namespace should not be dropped when still has Iceberg table",
NamespaceNotEmptyException.class,
- "Cannot drop namespace",
+ "still contains Iceberg tables",
+ () -> glueCatalog.dropNamespace(Namespace.of("db1")));
+ }
+
+ @Test
+ public void dropNamespace_notEmpty_containsNonIcebergTable() {
+ Mockito.doReturn(GetTablesResponse.builder()
+ .tableList(
+ Table.builder().databaseName("db1").name("t1").build()
+ ).build())
+ .when(glue).getTables(Mockito.any(GetTablesRequest.class));
+ Mockito.doReturn(GetDatabaseResponse.builder()
+ .database(Database.builder().name("db1").build()).build())
+ .when(glue).getDatabase(Mockito.any(GetDatabaseRequest.class));
+ Mockito.doReturn(DeleteDatabaseResponse.builder().build())
+ .when(glue).deleteDatabase(Mockito.any(DeleteDatabaseRequest.class));
+ AssertHelpers.assertThrows("namespace should not be dropped when still has non-Iceberg table",
+ NamespaceNotEmptyException.class,
+ "still contains non-Iceberg tables",
() -> glueCatalog.dropNamespace(Namespace.of("db1")));
}