hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Vary <pv...@cloudera.com>
Subject Re: Review Request 49498: HIVE-14123: Add beeline configuration option to show database in the prompt
Date Tue, 05 Jul 2016 09:16:30 GMT


> On July 1, 2016, 9:58 p.m., Vihang Karajgaonkar wrote:
> > Thanks for the changes. Can you please test your code by setting USE_DEPRECATED_CLI=false.

Ok. Maybe something is not yet clear for me :)
Could you please explain?

My understanding of the command prompt clients:
- CLI - Deprecated - uses org.apache.hadoop.hive.cli.CliDriver class, when the USE_DEPRECATED_CLI
is set, then it uses this class to start - no developement is planned
- Beeline Cli - The new one, and this should be used, and developed further, and could start
in two modes:
-- Beeline mode - org.apache.hive.beeline.BeeLine class will start in this mode (beeLine.isBeeLine()==true)
-- Compatibility mode - org.apache.hive.beeline.cli.HiveCli class will start in this mode
(beeLine.isBeeLine()==false)

Depending on the connection URL, the Beeline Cli could start an embedded HS2 server - This
is independent of the compatibility mode settings, and dependent only on the connection url.
I have learned this since your, and Szehon Ho's review, so I have to change my comments at
least :)

Assuming the above is correct, I think we should not change the behavior of the CLI, and the
Beeline in Compatiblity mode.
I think we should only use the new parameter in Beeline/Beeline mode, like it is happened
with the showHeader command line option.

If something is wrong with the above assumptions, or you think we should use the new parameter
in another modes as well, just say so, and I will look at it

Thanks, and sorry, but still learning the code, and the hive specific practices


> On July 1, 2016, 9:58 p.m., Vihang Karajgaonkar wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 1446
> > <https://reviews.apache.org/r/49498/diff/1/?file=1434737#file1434737line1446>
> >
> >     Is there a reason to remove previous check for HiveConf.ConfVars.CLIPRINTCURRENTDB?
Looks like this method is getting called from getPromptForCli() and if the beeline option
for db is false, it will not set db name in the cli prompt too irrespective of the value of
HiveConf.ConfVars.CLIPRINTCURRENTDB. btw, you may want to look at HIVE-14151 if you finding
trouble while using beeline as cli

I moved the HiveConf.ConfVars.CLIPRINTCURRENTDB check inside the getShowDbInPrompt() method,
to match the outline of the getShowHeader() parameter.

Thanks for the pointer, it helped a lot in testing and understanding :)


- Peter


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49498/#review140391
-----------------------------------------------------------


On July 1, 2016, 3:08 p.m., Peter Vary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49498/
> -----------------------------------------------------------
> 
> (Updated July 1, 2016, 3:08 p.m.)
> 
> 
> Review request for hive, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14123
>     https://issues.apache.org/jira/browse/HIVE-14123
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> There are several jira issues complaining that, the Beeline does not respect hive.cli.print.current.db.
> This is partially true, since in embedded mode, it uses the hive.cli.print.current.db
to change the prompt, since HIVE-10511.
> In remote mode, I think this function should use a beeline command line option instead,
like for the showHeader option emphasizing, that this is a client side option.
> 
> The patch contains:
> - New configuration option
> - Changing the help text
> - Updating command hooks, to run in remote mode as well
> - Adding new hooks, for connect and go sqllite commands
> - Generalize database connection refresh command
> - Changing prompt
> 
> 
> Diffs
> -----
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java 66185f6 
>   beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 5aaa385 
>   beeline/src/java/org/apache/hive/beeline/ClientCommandHookFactory.java c4d97bc 
>   beeline/src/java/org/apache/hive/beeline/Commands.java 3a204c0 
>   beeline/src/java/org/apache/hive/beeline/ReflectiveCommandHandler.java 3b863ae 
>   beeline/src/main/resources/BeeLine.properties 7500df9 
>   beeline/src/test/org/apache/hive/beeline/TestBeelineArgParsing.java ce1f538 
>   beeline/src/test/org/apache/hive/beeline/TestClientCommandHookFactory.java c86de0a

> 
> Diff: https://reviews.apache.org/r/49498/diff/
> 
> 
> Testing
> -------
> 
> Unit tests for the hooks, and the configuration option
> 
> Manual test in remote, and embedded mode
> 
> 
> Thanks,
> 
> Peter Vary
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message