Hive: Hadoop Path fails on s3 endpoint (#5405)

apache · Aug 4, 2022 · 7f5ecad · 7f5ecad
1 parent 28b5d0e
commit 7f5ecad
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 10 deletions.
diff --git a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
@@ -24,7 +24,6 @@
 import java.util.stream.Collectors;
 import org.apache.hadoop.conf.Configurable;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.IMetaStoreClient;
 import org.apache.hadoop.hive.metastore.api.AlreadyExistsException;
@@ -482,18 +481,17 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
       throw new RuntimeException("Interrupted during commit", e);
     }
 
-    // Otherwise stick to the {WAREHOUSE_DIR}/{DB_NAME}.db/{TABLE_NAME} path
-    String warehouseLocation = getWarehouseLocation();
-    return String.format(
-        "%s/%s.db/%s",
-        warehouseLocation, tableIdentifier.namespace().levels()[0], tableIdentifier.name());
+    // Otherwise, stick to the {WAREHOUSE_DIR}/{DB_NAME}.db/{TABLE_NAME} path
+    String databaseLocation = databaseLocation(tableIdentifier.namespace().levels()[0]);
+    return String.format("%s/%s", databaseLocation, tableIdentifier.name());
   }
 
-  private String getWarehouseLocation() {
+  private String databaseLocation(String databaseName) {
     String warehouseLocation = conf.get(HiveConf.ConfVars.METASTOREWAREHOUSE.varname);
     Preconditions.checkNotNull(
         warehouseLocation, "Warehouse location is not set: hive.metastore.warehouse.dir=null");
-    return warehouseLocation;
+    warehouseLocation = LocationUtil.stripTrailingSlash(warehouseLocation);
+    return String.format("%s/%s.db", warehouseLocation, databaseName);
   }
 
   private Map<String, String> convertToMetadata(Database database) {
@@ -518,8 +516,7 @@ Database convertToDatabase(Namespace namespace, Map<String, String> meta) {
     Map<String, String> parameter = Maps.newHashMap();
 
     database.setName(namespace.level(0));
-    database.setLocationUri(
-        new Path(getWarehouseLocation(), namespace.level(0)).toString() + ".db");
+    database.setLocationUri(databaseLocation(namespace.level(0)));
 
     meta.forEach(
         (key, value) -> {

diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java
@@ -740,4 +740,18 @@ public void testTablePropsDefinedAtCatalogLevel() {
       hiveCatalog.dropTable(tableIdent);
     }
   }
+
+  @Test
+  public void testDatabaseLocationWithSlashInWarehouseDir() {
+    Configuration conf = new Configuration();
+    // With a trailing slash
+    conf.set("hive.metastore.warehouse.dir", "s3://bucket/");
+
+    HiveCatalog catalog = new HiveCatalog();
+    catalog.setConf(conf);
+
+    Database database = catalog.convertToDatabase(Namespace.of("database"), ImmutableMap.of());
+
+    Assert.assertEquals("s3://bucket/database.db", database.getLocationUri());
+  }
 }