You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ja...@apache.org on 2023/04/24 18:25:21 UTC
[iceberg] branch master updated: AWS: Fix default warehouse path in Dynamodb catalog (#7358)
This is an automated email from the ASF dual-hosted git repository.
jackye pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/master by this push:
new 69c3249338 AWS: Fix default warehouse path in Dynamodb catalog (#7358)
69c3249338 is described below
commit 69c3249338697b8d09915c1a26bddd829c8f7277
Author: S N Munendra <96...@users.noreply.github.com>
AuthorDate: Mon Apr 24 23:55:13 2023 +0530
AWS: Fix default warehouse path in Dynamodb catalog (#7358)
---
.../iceberg/aws/dynamodb/TestDynamoDbCatalog.java | 15 +++
.../iceberg/aws/dynamodb/DynamoDbCatalog.java | 2 +-
.../iceberg/aws/dynamodb/TestDynamoDbCatalog.java | 105 +++++++++++++++++++++
3 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java b/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java
index f6fbfa55cf..9fae307fbc 100644
--- a/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java
+++ b/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java
@@ -379,6 +379,21 @@ public class TestDynamoDbCatalog {
Assertions.assertThat(catalog.dropNamespace(namespace)).isTrue();
}
+ @Test
+ public void testDefaultWarehousePathWithLocation() {
+ String namespaceName = genRandomName();
+ String defaultLocation = "s3://" + testBucket + "/namespace/" + namespaceName;
+
+ Namespace namespace = Namespace.of(namespaceName);
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put(DynamoDbCatalog.defaultLocationProperty(), defaultLocation);
+ catalog.createNamespace(namespace, properties);
+ String tableName = genRandomName();
+ Assertions.assertThat(
+ catalog.defaultWarehouseLocation(TableIdentifier.of(namespaceName, tableName)))
+ .isEqualTo(defaultLocation + "/" + tableName);
+ }
+
@Test
public void testRegisterExistingTable() {
Namespace namespace = Namespace.of(genRandomName());
diff --git a/aws/src/main/java/org/apache/iceberg/aws/dynamodb/DynamoDbCatalog.java b/aws/src/main/java/org/apache/iceberg/aws/dynamodb/DynamoDbCatalog.java
index 7e1beb2fac..a957b3096a 100644
--- a/aws/src/main/java/org/apache/iceberg/aws/dynamodb/DynamoDbCatalog.java
+++ b/aws/src/main/java/org/apache/iceberg/aws/dynamodb/DynamoDbCatalog.java
@@ -178,7 +178,7 @@ public class DynamoDbCatalog extends BaseMetastoreCatalog
String defaultLocationCol = toPropertyCol(PROPERTY_DEFAULT_LOCATION);
if (response.item().containsKey(defaultLocationCol)) {
return String.format(
- "%s/%s", response.item().get(defaultLocationCol), tableIdentifier.name());
+ "%s/%s", response.item().get(defaultLocationCol).s(), tableIdentifier.name());
} else {
return String.format(
"%s/%s.db/%s", warehousePath, tableIdentifier.namespace(), tableIdentifier.name());
diff --git a/aws/src/test/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java b/aws/src/test/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java
new file mode 100644
index 0000000000..9e6e948408
--- /dev/null
+++ b/aws/src/test/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java
@@ -0,0 +1,105 @@
+/*
+ * 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.dynamodb;
+
+import static org.apache.iceberg.aws.dynamodb.DynamoDbCatalog.toPropertyCol;
+import static org.mockito.ArgumentMatchers.any;
+
+import org.apache.iceberg.aws.AwsProperties;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.assertj.core.api.Assertions;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
+import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
+import software.amazon.awssdk.services.dynamodb.model.GetItemRequest;
+import software.amazon.awssdk.services.dynamodb.model.GetItemResponse;
+
+public class TestDynamoDbCatalog {
+
+ private static final String WAREHOUSE_PATH = "s3://bucket";
+ private static final String CATALOG_NAME = "dynamodb";
+ private static final TableIdentifier TABLE_IDENTIFIER = TableIdentifier.of("db", "table");
+
+ private DynamoDbClient dynamo;
+ private DynamoDbCatalog dynamoCatalog;
+
+ @Before
+ public void before() {
+ dynamo = Mockito.mock(DynamoDbClient.class);
+ dynamoCatalog = new DynamoDbCatalog();
+ dynamoCatalog.initialize(CATALOG_NAME, WAREHOUSE_PATH, new AwsProperties(), dynamo, null);
+ }
+
+ @Test
+ public void testConstructorWarehousePathWithEndSlash() {
+ DynamoDbCatalog catalogWithSlash = new DynamoDbCatalog();
+ catalogWithSlash.initialize(
+ CATALOG_NAME, WAREHOUSE_PATH + "/", new AwsProperties(), dynamo, null);
+ Mockito.doReturn(GetItemResponse.builder().item(Maps.newHashMap()).build())
+ .when(dynamo)
+ .getItem(any(GetItemRequest.class));
+ String location = catalogWithSlash.defaultWarehouseLocation(TABLE_IDENTIFIER);
+ Assertions.assertThat(location).isEqualTo(WAREHOUSE_PATH + "/db.db/table");
+ }
+
+ @Test
+ public void testDefaultWarehouseLocationNoDbUri() {
+ Mockito.doReturn(GetItemResponse.builder().item(Maps.newHashMap()).build())
+ .when(dynamo)
+ .getItem(any(GetItemRequest.class));
+
+ String warehousePath = WAREHOUSE_PATH + "/db.db/table";
+ String defaultWarehouseLocation = dynamoCatalog.defaultWarehouseLocation(TABLE_IDENTIFIER);
+ Assertions.assertThat(defaultWarehouseLocation).isEqualTo(warehousePath);
+ }
+
+ @Test
+ public void testDefaultWarehouseLocationDbUri() {
+ String dbUri = "s3://bucket2/db";
+ Mockito.doReturn(
+ GetItemResponse.builder()
+ .item(
+ ImmutableMap.of(
+ toPropertyCol(DynamoDbCatalog.defaultLocationProperty()),
+ AttributeValue.builder().s(dbUri).build()))
+ .build())
+ .when(dynamo)
+ .getItem(any(GetItemRequest.class));
+
+ String defaultWarehouseLocation = dynamoCatalog.defaultWarehouseLocation(TABLE_IDENTIFIER);
+ Assertions.assertThat(defaultWarehouseLocation).isEqualTo("s3://bucket2/db/table");
+ }
+
+ @Test
+ public void testDefaultWarehouseLocationNoNamespace() {
+ Mockito.doReturn(GetItemResponse.builder().build())
+ .when(dynamo)
+ .getItem(any(GetItemRequest.class));
+
+ Assertions.assertThatThrownBy(() -> dynamoCatalog.defaultWarehouseLocation(TABLE_IDENTIFIER))
+ .as("default warehouse can't be called on non existent namespace")
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContaining("Cannot find default warehouse location:");
+ }
+}