Skip to content

Commit 9846386

Browse files
committed
HIVE-29461: Iceberg: HIVE_METASTORE_WAREHOUSE_EXTERNAL is ignored when initializing HiveCatalog
HiveCatalog.initialize() propagated CatalogProperties.WAREHOUSE_LOCATION to the Hadoop configuration but ignored the equivalent external-warehouse property, so getExternalWarehouseLocation() and convertToDatabase() failed with an NPE ("Warehouse location is not set: hive.metastore.warehouse.external.dir=null") whenever a caller reached HiveCatalog through the standard Iceberg Catalog.initialize(name, properties) API without separately mutating the Configuration. Callers worked around this either by re-setting the value on the Configuration before initialize() (HMSCatalogFactory) or by relying on a side-channel setConf() carrying the value through (IcebergSummaryHandler), and in both cases used a different, undocumented property key ("external-warehouse" vs "externalwarehouse"). Add HiveCatalog.EXTERNAL_WAREHOUSE_LOCATION ("external-warehouse") as the canonical catalog property and propagate it from initialize() to hive.metastore.warehouse.external.dir, mirroring the existing handling for WAREHOUSE_LOCATION (including LocationUtil.stripTrailingSlash). Update the two in-tree callers to use the constant and drop HMSCatalogFactory's redundant configuration.set() workaround. The unhyphenated key in IcebergSummaryHandler was effectively dead code (HiveCatalog never read it) and is replaced rather than retained. Adds testInitializeCatalogWithExternalWarehouseProperty mirroring testInitializeCatalogWithProperties to lock in the propagation behavior.
1 parent ee6848d commit 9846386

4 files changed

Lines changed: 30 additions & 4 deletions

File tree

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,13 @@ public class HiveCatalog extends BaseMetastoreViewCatalog
7878
public static final String LIST_ALL_TABLES = "list-all-tables";
7979
public static final String LIST_ALL_TABLES_DEFAULT = "false";
8080

81+
/**
82+
* Catalog property for the external (unmanaged) warehouse location, propagated by
83+
* {@link #initialize(String, Map)} to {@code hive.metastore.warehouse.external.dir}.
84+
* Mirrors {@link CatalogProperties#WAREHOUSE_LOCATION} which targets the managed warehouse.
85+
*/
86+
public static final String EXTERNAL_WAREHOUSE_LOCATION = "external-warehouse";
87+
8188
public static final String HMS_TABLE_OWNER = "hive.metastore.table.owner";
8289
public static final String HMS_DB_OWNER = "hive.metastore.database.owner";
8390
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) {
115122
LocationUtil.stripTrailingSlash(properties.get(CatalogProperties.WAREHOUSE_LOCATION)));
116123
}
117124

125+
if (properties.containsKey(EXTERNAL_WAREHOUSE_LOCATION)) {
126+
this.conf.set(HiveConf.ConfVars.HIVE_METASTORE_WAREHOUSE_EXTERNAL.varname,
127+
LocationUtil.stripTrailingSlash(properties.get(EXTERNAL_WAREHOUSE_LOCATION)));
128+
}
129+
118130
this.listAllTables = Boolean.parseBoolean(properties.getOrDefault(LIST_ALL_TABLES, LIST_ALL_TABLES_DEFAULT));
119131

120132
String fileIOImpl = properties.get(CatalogProperties.FILE_IO_IMPL);

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,22 @@ public void testInitializeCatalogWithProperties() {
283283
.isEqualTo("/user/hive/testwarehouse");
284284
}
285285

286+
@Test
287+
public void testInitializeCatalogWithExternalWarehouseProperty() {
288+
Map<String, String> properties = Maps.newHashMap();
289+
properties.put("warehouse", "/user/hive/testwarehouse/");
290+
properties.put(HiveCatalog.EXTERNAL_WAREHOUSE_LOCATION, "/user/hive/external/");
291+
HiveCatalog hiveCatalog = new HiveCatalog();
292+
hiveCatalog.initialize("hive", properties);
293+
294+
// Both warehouse properties should be propagated to the Hadoop configuration
295+
// and trailing slashes should be stripped (matching the managed warehouse behavior).
296+
assertThat(hiveCatalog.getConf().get("hive.metastore.warehouse.dir"))
297+
.isEqualTo("/user/hive/testwarehouse");
298+
assertThat(hiveCatalog.getConf().get("hive.metastore.warehouse.external.dir"))
299+
.isEqualTo("/user/hive/external");
300+
}
301+
286302
@Test
287303
public void testCreateTableTxnBuilder() throws Exception {
288304
Schema schema = getTestSchema();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void initialize(String catalogName, boolean jsonFormat, MetaSummarySchema
6464
LOG.info("Initializing iceberg summary handler with warehouse:{},external warehouse:{},uris:{}",
6565
mgdWarehouse, extWarehouse, uris);
6666
propertiesMap.put("warehouse", mgdWarehouse);
67-
propertiesMap.put("externalwarehouse", extWarehouse);
67+
propertiesMap.put(HiveCatalog.EXTERNAL_WAREHOUSE_LOCATION, extWarehouse);
6868
propertiesMap.put("uri", uris);
6969
PrivilegedAction<HiveCatalog> action = () -> {
7070
HiveCatalog hiveCatalog = new HiveCatalog();

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,7 @@ private Catalog createCatalog() {
8585
}
8686
final String configExtWarehouse = MetastoreConf.getVar(configuration, MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL);
8787
if (configExtWarehouse != null) {
88-
properties.put("external-warehouse", configExtWarehouse);
89-
// HiveCatalog reads this property directly from Configuration, not from properties map
90-
configuration.set(MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL.getHiveName(), configExtWarehouse);
88+
properties.put(HiveCatalog.EXTERNAL_WAREHOUSE_LOCATION, configExtWarehouse);
9189
}
9290
if (configuration.get(SERVLET_ID_KEY) != null) {
9391
// For the testing purpose. HiveCatalog caches a metastore client in a static field. As our tests can spin up

0 commit comments

Comments
 (0)