From 8e492670c368a2b37e425d92c2e94c901bbfc2f2 Mon Sep 17 00:00:00 2001 From: Ivan Date: Sat, 19 Nov 2022 13:10:10 +0100 Subject: [PATCH 1/4] Use provided glue catalog id when listing databases in defaultWarehouseLocation --- aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java | 1 + 1 file changed, 1 insertion(+) 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 71ccce8c1f50..0b199f761ace 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 @@ -261,6 +261,7 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) { GetDatabaseResponse response = glue.getDatabase( GetDatabaseRequest.builder() + .catalogId(awsProperties.glueCatalogId()) .name( IcebergToGlueConverter.getDatabaseName( tableIdentifier, awsProperties.glueCatalogSkipNameValidation())) From a3dbc60afe6fdb30171e504bf655032b555217b1 Mon Sep 17 00:00:00 2001 From: Ivan Date: Sun, 20 Nov 2022 11:07:54 +0100 Subject: [PATCH 2/4] Added unit test to validated catalogId gets set in defaultWarehouseLocation method. --- .../iceberg/aws/glue/TestGlueCatalog.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java index 34d54a1ebcc3..531ee8989941 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java +++ b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java @@ -144,6 +144,32 @@ public void testDefaultWarehouseLocationDbUri() { Assert.assertEquals("s3://bucket2/db/table", location); } + @Test + public void testDefaultWarehouseLocationCustomCatalogId() { + GlueCatalog catalogWithCustomCatalogId = new GlueCatalog(); + String catalogId = "myCatalogId"; + ImmutableMap.Builder catalogIdPropertiesBuilder = + ImmutableMap.builder().put(AwsProperties.GLUE_CATALOG_ID, catalogId); + catalogWithCustomCatalogId.initialize( + CATALOG_NAME, + WAREHOUSE_PATH + "/", + new AwsProperties(catalogIdPropertiesBuilder.build()), + glue, + LockManagers.defaultLockManager(), + null, + ImmutableMap.of()); + + Mockito.doReturn( + GetDatabaseResponse.builder() + .database(Database.builder().name("db").locationUri("s3://bucket2/db").build()) + .build()) + .when(glue) + .getDatabase(Mockito.any(GetDatabaseRequest.class)); + catalogWithCustomCatalogId.defaultWarehouseLocation(TableIdentifier.of("db", "table")); + Mockito.verify(glue) + .getDatabase(Mockito.argThat((GetDatabaseRequest req) -> req.catalogId() == catalogId)); + } + @Test public void testListTables() { Mockito.doReturn( From 94bdcb1c6f1fc91078cdf5af4e50a5c2f083bee3 Mon Sep 17 00:00:00 2001 From: Ivan Date: Mon, 21 Nov 2022 07:49:44 +0100 Subject: [PATCH 3/4] Changed how AwsProperties are initialized custom catalog id test. Setters are now used instead of a constructor map. --- .../java/org/apache/iceberg/aws/glue/TestGlueCatalog.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java index 531ee8989941..636801f47c01 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java +++ b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java @@ -148,12 +148,12 @@ public void testDefaultWarehouseLocationDbUri() { public void testDefaultWarehouseLocationCustomCatalogId() { GlueCatalog catalogWithCustomCatalogId = new GlueCatalog(); String catalogId = "myCatalogId"; - ImmutableMap.Builder catalogIdPropertiesBuilder = - ImmutableMap.builder().put(AwsProperties.GLUE_CATALOG_ID, catalogId); + AwsProperties awsProperties = new AwsProperties(); + awsProperties.setGlueCatalogId(catalogId); catalogWithCustomCatalogId.initialize( CATALOG_NAME, WAREHOUSE_PATH + "/", - new AwsProperties(catalogIdPropertiesBuilder.build()), + awsProperties, glue, LockManagers.defaultLockManager(), null, From 55b438d5a6eaa2824162380f1c4bb39cf6755ff3 Mon Sep 17 00:00:00 2001 From: Ivan Date: Fri, 25 Nov 2022 08:11:33 +0100 Subject: [PATCH 4/4] Using equals for string comparison in Glue catalogId test --- .../test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java index 636801f47c01..440d85c9d112 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java +++ b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java @@ -167,7 +167,8 @@ public void testDefaultWarehouseLocationCustomCatalogId() { .getDatabase(Mockito.any(GetDatabaseRequest.class)); catalogWithCustomCatalogId.defaultWarehouseLocation(TableIdentifier.of("db", "table")); Mockito.verify(glue) - .getDatabase(Mockito.argThat((GetDatabaseRequest req) -> req.catalogId() == catalogId)); + .getDatabase( + Mockito.argThat((GetDatabaseRequest req) -> req.catalogId().equals(catalogId))); } @Test