> On June 8, 2015, 9:41 p.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. > > Xuefu Zhang wrote: > 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. > > cheng xu wrote: > Thank you for your prompt reply. > `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.` > I'm not sure whether substitution for sh and source is useful. We can > enable the support of substitution after connection established for beeline > unless connected. For the new CLI who is using an embedded connection, it > should be supported WRT the backwards compatibility. > > I am a little confused about `connect to multiple serves at the same > time`. Does it mean you can use beeline to connect any server in one > connection and you can have multiple beeline instances running? (It's the > case that user executes the command > */opt/apache-hive-1.2.0-SNAPSHOT-bin/bin/hive --service beeline* with specify > any hostname) > If so, I think it may cause some errors if no connection available since > the current implementation is based on connection by using **SetProcessor**. > AFAIK, it's safe to get the configurations from HS2 via **SetProcessor** > which is what beeline actually did after connection is established. > Connection(session) should only be assiocated with one server. If user didn't > connect to any HS2, the substitution for *sh* and *source* should be > disabled. To be honest, it will have some negative impacts for the > performance since it requires to execute set command. WRT the performance, we > can make this support configurable. > > In summary, substitution is enabled unless connection is established for > source or sh command considering the backwards compatibility. And we can > disable the support for beeline if not reasonable or brings lower performce. > For HIVE-10847, I think we still need one way to access the configuration > from server side but it is only needed when start a connection. > > Any thoughts? > > cheng xu wrote: > Sorry for below typo. > I am a little confused about connect to multiple serves at the same time. > Does it mean you can use beeline to connect any server in one connection and > you can have multiple beeline instances running? (It's the case that user > executes the command /opt/apache-hive-1.2.0-SNAPSHOT-bin/bin/hive --service > beeline **without** specify any hostname) > > Xuefu Zhang wrote: > I think it's possible to start beeline without any connection. To do > that, just run beeline w/o -u parameter. Once beeline starts, you can run > "!connect jdbc url" to make a connection. I also believe it's also possible > to make another connection using "!connect jdbc url" w/o disconnecting from > the previous connection. You can run "!list" to get a list of connections, > and "!go index" to select a particular connection as active. In addition, you > can even connect to a non-Hive database. So, this gets really complicated. > > I think supporting variable substituion for shell command is fine when > beeline is running in cli-compatible mode. In such mode server and client are > on the same host, so there is no ambiguity. However, I'd think twice when > considering the same support for Beeline in general. > > cheng xu wrote: > Understood. I missed the non-hive database case. It's really not easy to > deal with. I'll update the patch only providing support for cli-compatible > mode. And the API to retrive configurations from HS2 should only be useful in > CLI mode as well. For beeline, the support for substitution needs further > discussion if exists some requirements in the future. BTW, are **go** and > **list** command DOCed in hive wiki? They're really cool commands! > > Xuefu Zhang wrote: > I don't think they are specifically documented. I believe they are in > SQLLIne doc, to which Beeline doc has a reference.
Yes, **go** and **list** are documented here: http://sqlline.sourceforge.net/#sect_command_go and http://sqlline.sourceforge.net/#sect_command_list . If you want to draw attention to **go** and **list** in the Beeline doc, you could add some usage examples here: https://cwiki.apache.org/confluence/display/Hive/HiveServer2+Clients#HiveServer2Clients-BeelineCommands . - Lefty ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review87090 ----------------------------------------------------------- On June 4, 2015, 7:09 p.m., cheng xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35107/ > ----------------------------------------------------------- > > (Updated June 4, 2015, 7:09 p.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 > >