Henry Robinson has posted comments on this change. Change subject: IMPALA-3419: DefaultFS value should be extracted from hive.metastore.warehouse.dir if fully qualified ......................................................................
Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/2881/2/fe/src/main/java/com/cloudera/impala/service/JniFrontend.java File fe/src/main/java/com/cloudera/impala/service/JniFrontend.java: Line 549: String finalValue = ""; : if (request.getName().equals("fs.defaultFS")) { : HiveConf hiveConfig = new HiveConf(JniFrontend.class); : String warehouse_dir = hiveConfig.getVar(HiveConf.ConfVars.METASTOREWAREHOUSE); : if (!warehouse_dir.isEmpty()) { : int authority_index; : // Override the value we return as default FS if the hive metastore warehouse : // directory is fully qualified. : if ((authority_index = warehouse_dir.indexOf(":/")) != -1) { : // Retrieve only the namenode. : finalValue = warehouse_dir.substring( : 0, warehouse_dir.indexOf('/', authority_index + 3)); : } : } : } : if (finalValue.isEmpty()) finalValue = CONF.get(request.getName()); : result.setValue(finalValue); I think it's going to be cleaner to do the following: 1. Add a getHiveConfig() method to JniFrontend 2. Put the logic that decides which config should be the default in the BE method that calls getHiveConfig() The thing is that this logic breaks the contract for getHadoopConfig() - I asked for default FS but got something from Hive?? - in a way that might subtly introduce bugs in the future. -- To view, visit http://gerrit.cloudera.org:8080/2881 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id47be2b0e9c25a8bd838555aa75c949c23050c04 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-HasComments: Yes
