----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37660/#review96087 -----------------------------------------------------------
jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java (line 97) <https://reviews.apache.org/r/37660/#comment151301> (this is mostly a nit, as this perf impact would be hard to notice. but the tweaks are also minor) - The pattern can be a static final member. (It is not mutable and thread safe). I think a slightly more efficient pattern would be -(add '=' to the non key char, that way the pattern evaluation would not have to back track after going beyond '='). "([^=;]*)=([^;]*)[;]?" (seems faster by around 20% in a quick test i did) public class TClass { private static String key; private static String value; public static void main(String[] args) throws InterruptedException { long ts = System.currentTimeMillis(); String confStr = "hiveserver.configparam.key1=value1;hiveserver.configparam.key2=value2;hiveserver.configparam.key3=value3;"; Pattern pattern = Pattern.compile("([^;]*)=([^;]*)[;]?"); // for (int i = 0; i < 1000000; i++) { for (int i = 0; i < 1; i++) { Matcher matcher = pattern.matcher(confStr); while (matcher.find()) { key = matcher.group(1); value = matcher.group(2); // System.out.println(key + ":" + value); } } System.out.println(System.currentTimeMillis() - ts); } } jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java (line 102) <https://reviews.apache.org/r/37660/#comment151309> The "matcher.group(1) != null" can be moved outside of each if statement to a common one on top. if(matcher.group(1) == null) { continue; } If the key is not null but value is null, i think we should throw an error. It would indicate a bug or some corruption. if(matcher.group(2) == null) { throw .. } jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java (line 103) <https://reviews.apache.org/r/37660/#comment151312> hive configuration is case sensitive, lets keep it consistent here. service/src/java/org/apache/hive/service/server/HiveServer2.java (line 109) <https://reviews.apache.org/r/37660/#comment151313> typo - "initialize" service/src/java/org/apache/hive/service/server/HiveServer2.java (line 133) <https://reviews.apache.org/r/37660/#comment151315> we don't seem to have any existing support for HIVE_SERVER2_AUTHENTICATION env variable. service/src/java/org/apache/hive/service/server/HiveServer2.java (line 220) <https://reviews.apache.org/r/37660/#comment151316> Guava Joiner can be used here (it likely uses StringBuilder underneath) - Joiner.on(';').withKeyValueSeparator("=").join(confsToPublish); service/src/java/org/apache/hive/service/server/HiveServer2.java (line 273) <https://reviews.apache.org/r/37660/#comment151318> do we need to publish the password based authentication modes ? - Thejas Nair On Aug. 20, 2015, 10:04 p.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37660/ > ----------------------------------------------------------- > > (Updated Aug. 20, 2015, 10:04 p.m.) > > > Review request for hive and Thejas Nair. > > > Bugs: HIVE-11581 > https://issues.apache.org/jira/browse/HIVE-11581 > > > Repository: hive-git > > > Description > ------- > > https://issues.apache.org/jira/browse/HIVE-11581 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 9a6781b > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java bb2b695 > jdbc/src/java/org/apache/hive/jdbc/Utils.java 0e4693b > jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java e24b3dc > service/src/java/org/apache/hive/service/server/HiveServer2.java 4a4be97 > > Diff: https://reviews.apache.org/r/37660/diff/ > > > Testing > ------- > > Manually, will publish test matrix shortly. > > > Thanks, > > Vaibhav Gumashta > >
