> On June 9, 2015, 4:41 a.m., Xuefu Zhang wrote: > > 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. > > cheng xu wrote: > Thanks for your comments. > > `I also noticed that in Driver class, there is code handling variable > substitution. I'm wondering why it's not effective.` > > The substitution works well in HS2 currently. > There are two reasons for me to add API getting the conf from HS2. One is > to support substitution in sh and source command. In the old cli, source > command and sh command worked well with substitution. So this part of this > patch is addressing this purpose. Another consideration is for > https://issues.apache.org/jira/browse/HIVE-10847 which required some > configuration from hive-site.xml.
Yeah. It's a little trickier than thought. Shell command is executed at client side (Beeline) and it doesn't seem making sense to use server specific variables such (env, sys, hiveconf, hivevar) in the shell commands. More importantly, Beeline can connect to multiple serves at the same time, so which configurations should be used to substitue the variables? User should be able to execute shell commands w/o any server connection. For CLI, server and client are together, so these don't matter. But for beeline + HS2 deployment, story will be different. I don't know what's the best, and all I'm saying is that we need to be very careful on what we doing. Before we decide what to do, we need to clearly define the problem we are trying to solve first. - Xuefu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review87090 ----------------------------------------------------------- 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 > >
