> On June 9, 2015, 12: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.
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! - cheng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review87090 ----------------------------------------------------------- On June 5, 2015, 10: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, 10: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 > >
