HIVE-29461: Iceberg: HIVE_METASTORE_WAREHOUSE_EXTERNAL is ignored when initializing HiveCatalog

iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveCatalog.java

@@ -78,6 +78,13 @@ public class HiveCatalog extends BaseMetastoreViewCatalog
   public static final String LIST_ALL_TABLES = "list-all-tables";
   public static final String LIST_ALL_TABLES_DEFAULT = "false";
 
+  /**
+   * Catalog property for the external (unmanaged) warehouse location, propagated by
+   * {@link #initialize(String, Map)} to {@code hive.metastore.warehouse.external.dir}.
+   * Mirrors {@link CatalogProperties#WAREHOUSE_LOCATION} which targets the managed warehouse.
+   */
+  public static final String EXTERNAL_WAREHOUSE_LOCATION = "external-warehouse";
+
   public static final String HMS_TABLE_OWNER = "hive.metastore.table.owner";
   public static final String HMS_DB_OWNER = "hive.metastore.database.owner";
   public static final String HMS_DB_OWNER_TYPE = "hive.metastore.database.owner-type";
@@ -115,6 +122,11 @@ public void initialize(String inputName, Map<String, String> properties) {
           LocationUtil.stripTrailingSlash(properties.get(CatalogProperties.WAREHOUSE_LOCATION)));
     }
 
+    if (properties.containsKey(EXTERNAL_WAREHOUSE_LOCATION)) {
+      this.conf.set(HiveConf.ConfVars.HIVE_METASTORE_WAREHOUSE_EXTERNAL.varname,
+          LocationUtil.stripTrailingSlash(properties.get(EXTERNAL_WAREHOUSE_LOCATION)));
+    }
+
     this.listAllTables = Boolean.parseBoolean(properties.getOrDefault(LIST_ALL_TABLES, LIST_ALL_TABLES_DEFAULT));
 
     String fileIOImpl = properties.get(CatalogProperties.FILE_IO_IMPL);

iceberg/iceberg-catalog/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java

@@ -283,6 +283,22 @@ public void testInitializeCatalogWithProperties() {
         .isEqualTo("/user/hive/testwarehouse");
   }
 
+  @Test
+  void testInitializeCatalogWithExternalWarehouseProperty() {
+    Map<String, String> properties = Maps.newHashMap();
+    properties.put("warehouse", "/user/hive/testwarehouse/");
+    properties.put(HiveCatalog.EXTERNAL_WAREHOUSE_LOCATION, "/user/hive/external/");
+    HiveCatalog hiveCatalog = new HiveCatalog();
+    hiveCatalog.initialize("hive", properties);
+
+    // Both warehouse properties should be propagated to the Hadoop configuration
+    // and trailing slashes should be stripped (matching the managed warehouse behavior).
+    assertThat(hiveCatalog.getConf().get("hive.metastore.warehouse.dir"))
+        .isEqualTo("/user/hive/testwarehouse");
+    assertThat(hiveCatalog.getConf().get("hive.metastore.warehouse.external.dir"))
+        .isEqualTo("/user/hive/external");
+  }
+
   @Test
   public void testCreateTableTxnBuilder() throws Exception {
     Schema schema = getTestSchema();

iceberg/iceberg-handler/src/main/java/org/apache/iceberg/metasummary/IcebergSummaryHandler.java

@@ -34,6 +34,7 @@
 import org.apache.hadoop.hive.metastore.utils.StringUtils;
 import org.apache.hadoop.security.SecurityUtil;
 import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.iceberg.CatalogProperties;
 import org.apache.iceberg.Table;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.hive.HiveCatalog;
@@ -63,9 +64,15 @@ public void initialize(String catalogName, boolean jsonFormat, MetaSummarySchema
     Map<String, String> propertiesMap = Maps.newHashMap();
     LOG.info("Initializing iceberg summary handler with warehouse:{}，external warehouse:{}，uris:{}",
         mgdWarehouse, extWarehouse, uris);
-    propertiesMap.put("warehouse", mgdWarehouse);
-    propertiesMap.put("externalwarehouse", extWarehouse);
-    propertiesMap.put("uri", uris);
+    if (!StringUtils.isEmpty(mgdWarehouse)) {
+      propertiesMap.put(CatalogProperties.WAREHOUSE_LOCATION, mgdWarehouse);
+    }
+    if (!StringUtils.isEmpty(extWarehouse)) {
+      propertiesMap.put(HiveCatalog.EXTERNAL_WAREHOUSE_LOCATION, extWarehouse);
+    }
+    if (!StringUtils.isEmpty(uris)) {
+      propertiesMap.put(CatalogProperties.URI, uris);
+    }
     PrivilegedAction<HiveCatalog> action = () -> {
       HiveCatalog hiveCatalog = new HiveCatalog();
       hiveCatalog.setConf(configuration);

standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogFactory.java

@@ -23,6 +23,7 @@
 import java.util.Map;
 import java.util.TreeMap;
 import javax.servlet.http.HttpServlet;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hive.metastore.ServletSecurity;
 import org.apache.hadoop.hive.metastore.ServletSecurity.AuthType;
@@ -71,23 +72,19 @@ private Catalog createCatalog() {
     final Map<String, String> properties = new TreeMap<>();
     final String configUri = MetastoreConf.getVar(configuration, MetastoreConf.ConfVars.THRIFT_URIS);
     // Clear THRIFT_URIS so HiveCatalog doesn't accidentally use Thrift connection
-    // when REST Catalog is embedded in HMS (same JVM). HiveCatalog reads from Configuration
-    // as fallback, so clearing it ensures it uses embedded connection when "uri" is not set.
+    // when REST Catalog is embedded in HMS (same JVM).
     MetastoreConf.setVar(configuration, MetastoreConf.ConfVars.THRIFT_URIS, "");
-    // Only set "uri" property if THRIFT_URIS was configured (standalone mode)
     // This tells HiveCatalog to use Thrift connection to external HMS
-    if (configUri != null && !configUri.isEmpty()) {
-      properties.put("uri", configUri);
+    if (!StringUtils.isEmpty(configUri)) {
+      properties.put(CatalogProperties.URI, configUri);
     }
     final String configWarehouse = MetastoreConf.getVar(configuration, MetastoreConf.ConfVars.WAREHOUSE);
-    if (configWarehouse != null) {
-      properties.put("warehouse", configWarehouse);
+    if (!StringUtils.isEmpty(configWarehouse)) {
+      properties.put(CatalogProperties.WAREHOUSE_LOCATION, configWarehouse);
     }
     final String configExtWarehouse = MetastoreConf.getVar(configuration, MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL);
-    if (configExtWarehouse != null) {
-      properties.put("external-warehouse", configExtWarehouse);
-      // HiveCatalog reads this property directly from Configuration, not from properties map
-      configuration.set(MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL.getHiveName(), configExtWarehouse);
+    if (!StringUtils.isEmpty(configExtWarehouse)) {
+      properties.put(HiveCatalog.EXTERNAL_WAREHOUSE_LOCATION, configExtWarehouse);
     }
     if (configuration.get(SERVLET_ID_KEY) != null) {
       // For the testing purpose. HiveCatalog caches a metastore client in a static field. As our tests can spin up