----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review87090 -----------------------------------------------------------
Besides the two minor issues I found in the patch, I was wondering if the approach we are taking is the best. Variable substitution is a server (HS2) behavior, and on this ground I think this should happen in HS2 instead of beeline. Please note that JDBC client may also submit queries with $var in it, and such a case should be also supported. I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective. Shell command (starting with !sh) is executed in the client (Beeline). I think we are fine if variable substituion doesn't work for shell command. We can address that as a followup taks if desirable. beeline/src/java/org/apache/hive/beeline/BeelineVariableSubstitution.java <https://reviews.apache.org/r/35107/#comment139368> Could we refactor and reuse existing VariableSubstition class rather than cloning one? beeline/src/java/org/apache/hive/beeline/Commands.java <https://reviews.apache.org/r/35107/#comment139371> line.trim() is already called in previous line. - Xuefu Zhang On June 5, 2015, 2:09 a.m., cheng xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35107/ > ----------------------------------------------------------- > > (Updated June 5, 2015, 2:09 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 45a7e87 > beeline/src/java/org/apache/hive/beeline/BeelineVariableSubstitution.java > PRE-CREATION > beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 > beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 > > Diff: https://reviews.apache.org/r/35107/diff/ > > > Testing > ------- > > Unit test passed > > > Thanks, > > cheng xu > >
