----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review89421 -----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/Commands.java (line 721) <https://reviews.apache.org/r/35107/#comment142021> Nit: Keep @Override in a separate line. beeline/src/java/org/apache/hive/beeline/Commands.java (line 722) <https://reviews.apache.org/r/35107/#comment142022> I think getHiveVariables() is in the same class, which can be directly accessed. beeline/src/java/org/apache/hive/beeline/Commands.java (line 760) <https://reviews.apache.org/r/35107/#comment142023> I think getHiveVariables() can be private. beeline/src/java/org/apache/hive/beeline/Commands.java (line 772) <https://reviews.apache.org/r/35107/#comment142024> getHiveConf() can be proviate. Remove the caller from BeeLine.java. beeline/src/java/org/apache/hive/beeline/Commands.java (line 850) <https://reviews.apache.org/r/35107/#comment142030> This used to call BeeLine.executeFile() now you have a new implementation. I don't quite follow why. Regardless, I see two problems. 1. we should remove the 2nd argument as all callers provide "false". 2. Some part of the code in executeFile() is to fix some problem with the last line in the script file. Will your new implementation catches that? 3. It's my understanding that in Hive CLI compatible mode, commands in source file should follow Hive CLI syntax, while in normal mode, it should follow Beeline syntax. Is this what's done here? - Xuefu Zhang On June 25, 2015, 5:54 a.m., cheng xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35107/ > ----------------------------------------------------------- > > (Updated June 25, 2015, 5:54 a.m.) > > > Review request for hive, chinna and Xuefu Zhang. > > > Bugs: HIVE-6791 > https://issues.apache.org/jira/browse/HIVE-6791 > > > Repository: hive-git > > > Description > ------- > > Summary: > 1) move the beeline-cli convertor to the place where cli is executed(class > **Commands**) > 2) support substitution for source command > 3) add some unit test for substitution > 4) add one way to get the configuration from HS2 > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e > beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 > beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 > cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c > common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java > PRE-CREATION > common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java > PRE-CREATION > common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755 > > ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java > a5f0a7f > ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java > e8b1d96 > ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java > 0558c53 > ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java > 25ce168 > > ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java > 9052c82 > ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d > ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java > 33ee16b > > Diff: https://reviews.apache.org/r/35107/diff/ > > > Testing > ------- > > Unit test passed > > > Thanks, > > cheng xu > >
