You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by cheng xu <ch...@intel.com> on 2015/06/05 04:03:25 UTC
Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/
-----------------------------------------------------------
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
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by Xuefu Zhang <xz...@cloudera.com>.
> On June 24, 2015, 10:21 p.m., Xuefu Zhang wrote:
> > beeline/src/java/org/apache/hive/beeline/Commands.java, line 820
> > <https://reviews.apache.org/r/35107/diff/3/?file=991102#file991102line820>
> >
> > Does this mean that env and sys variables are not being substituted for shell command?
>
> cheng xu wrote:
> No, this method is only used for retrieving hive configurations. For env and sys variables, they are subsituted by VariableSubstitution.
Not sure if I understand it. Could you outline the process in which the variables get substituted? Thanks.
- Xuefu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/#review89251
-----------------------------------------------------------
On June 25, 2015, 5:54 a.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35107/
> -----------------------------------------------------------
>
> (Updated June 25, 2015, 5:54 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 b7d2f2e
> beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
> common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
> common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
> ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
> ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
> ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
> ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
> ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
> ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c
> service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b
>
> Diff: https://reviews.apache.org/r/35107/diff/
>
>
> Testing
> -------
>
> Unit test passed
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
> On June 25, 2015, 6:21 a.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java, line 146
> > <https://reviews.apache.org/r/35107/diff/3/?file=991115#file991115line146>
> >
> > Could we keep @Override at a separate line? Same for other places.
I am using the code-style file(eclipse-styles.xml) under the dev-support folder. Seems annotation before class/method is not wrapped. Anyway, I just update all the places in this patch. Thank you for figuring this out.
> On June 25, 2015, 6:21 a.m., Xuefu Zhang wrote:
> > beeline/src/java/org/apache/hive/beeline/Commands.java, line 820
> > <https://reviews.apache.org/r/35107/diff/3/?file=991102#file991102line820>
> >
> > Does this mean that env and sys variables are not being substituted for shell command?
No, this method is only used for retrieving hive configurations. For env and sys variables, they are subsituted by VariableSubstitution.
- cheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/#review89251
-----------------------------------------------------------
On June 25, 2015, 1:54 p.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35107/
> -----------------------------------------------------------
>
> (Updated June 25, 2015, 1:54 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 b7d2f2e
> beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
> common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
> common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
> ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
> ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
> ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
> ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
> ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
> ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c
> service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b
>
> Diff: https://reviews.apache.org/r/35107/diff/
>
>
> Testing
> -------
>
> Unit test passed
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
> On June 25, 2015, 6:21 a.m., Xuefu Zhang wrote:
> > beeline/src/java/org/apache/hive/beeline/Commands.java, line 820
> > <https://reviews.apache.org/r/35107/diff/3/?file=991102#file991102line820>
> >
> > Does this mean that env and sys variables are not being substituted for shell command?
>
> cheng xu wrote:
> No, this method is only used for retrieving hive configurations. For env and sys variables, they are subsituted by VariableSubstitution.
>
> Xuefu Zhang wrote:
> Not sure if I understand it. Could you outline the process in which the variables get substituted? Thanks.
The class *VariableSubstitution* handles the case of hive variables. It's overriding the super class *SystemVariables* who deals with the system variables, env variables and hive conf variables. The process is as follows:
1. Construct a *VariableSubsitution* instance with the source where you could retrieve the hive variables
2. Call the method *substitute* from *VariableSubsitution*:
SystemVariables#substitute(super.substitute) -> SystemVariables#getSubstitute(get the eval from sys, env and hive conf) -> VariableSubstitution#getSubstitute(get the eval from hive variables source)
- cheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/#review89251
-----------------------------------------------------------
On June 25, 2015, 1:54 p.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35107/
> -----------------------------------------------------------
>
> (Updated June 25, 2015, 1:54 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 b7d2f2e
> beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
> common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
> common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
> ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
> ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
> ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
> ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
> ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
> ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c
> service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b
>
> Diff: https://reviews.apache.org/r/35107/diff/
>
>
> Testing
> -------
>
> Unit test passed
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/#review89251
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/Commands.java (line 717)
<https://reviews.apache.org/r/35107/#comment141839>
Nit: method naming. substitution() -> substituteVariables() or substitue()
beeline/src/java/org/apache/hive/beeline/Commands.java (line 791)
<https://reviews.apache.org/r/35107/#comment141854>
Do we need to close statement when done?
beeline/src/java/org/apache/hive/beeline/Commands.java (line 814)
<https://reviews.apache.org/r/35107/#comment141842>
Does this mean that env and sys variables are not being substituted for shell command?
beeline/src/java/org/apache/hive/beeline/Commands.java (line 861)
<https://reviews.apache.org/r/35107/#comment141843>
Don't we need to close reader object?
ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java (line 145)
<https://reviews.apache.org/r/35107/#comment141846>
Could we keep @Override at a separate line? Same for other places.
- Xuefu Zhang
On June 24, 2015, 7:39 a.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35107/
> -----------------------------------------------------------
>
> (Updated June 24, 2015, 7:39 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 b7d2f2e
> beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
> common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
> common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
> ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
> ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
> ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
> ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
> ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
> ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c
> service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b
>
> Diff: https://reviews.apache.org/r/35107/diff/
>
>
> Testing
> -------
>
> Unit test passed
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/#review89686
-----------------------------------------------------------
Ship it!
Ship It!
- Xuefu Zhang
On June 26, 2015, 8:02 a.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35107/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 8:02 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 66fe322
> beeline/src/java/org/apache/hive/beeline/Commands.java aaf6aec
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
> common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
> common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 669e6be
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
> ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
> ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
> ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
> ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
> ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
> ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java d271d6d
> service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java cc9df76
>
> Diff: https://reviews.apache.org/r/35107/diff/
>
>
> Testing
> -------
>
> Unit test passed
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/
-----------------------------------------------------------
(Updated June 26, 2015, 4:02 p.m.)
Review request for hive, chinna and Xuefu Zhang.
Changes
-------
Summary:
1) rebase code
2) add some java doc
3) do some code clean work
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 (updated)
-----
beeline/src/java/org/apache/hive/beeline/BeeLine.java 66fe322
beeline/src/java/org/apache/hive/beeline/Commands.java aaf6aec
beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/Driver.java 669e6be
ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java d271d6d
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java cc9df76
Diff: https://reviews.apache.org/r/35107/diff/
Testing
-------
Unit test passed
Thanks,
cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/
-----------------------------------------------------------
(Updated June 26, 2015, 2 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 (updated)
-----
beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e
beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3
beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755
ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b
Diff: https://reviews.apache.org/r/35107/diff/
Testing
-------
Unit test passed
Thanks,
cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
> On June 26, 2015, 5:54 a.m., Xuefu Zhang wrote:
> > beeline/src/java/org/apache/hive/beeline/Commands.java, line 855
> > <https://reviews.apache.org/r/35107/diff/4/?file=991896#file991896line855>
> >
> > This used to call BeeLine.executeFile() now you have a new implementation. I don't quite follow why. Regardless, I see two problems.
> > 1. we should remove the 2nd argument as all callers provide "false".
> > 2. Some part of the code in executeFile() is to fix some problem with the last line in the script file. Will your new implementation catches that?
> > 3. It's my understanding that in Hive CLI compatible mode, commands in source file should follow Hive CLI syntax, while in normal mode, it should follow Beeline syntax. Is this what's done here?
This is a little tricky code. The follows are the reasons I prefer not to use the *executeFile* in *BeeLine* to implement the source command:
1. In the executeFile method, it will reset the consoleReader. So the command after *source* will be skipped. Given "source xx;desc tbl;" as example, this "desc tbl" will be ignored since console reader is reset. It's OK for other places(execute initial file and execute file) to invoke this since it is called after the consoleReader initialized. This issue is not revealled in the last patch and I add one new test *testSourceCmd2* to cover this.
2. Furthermore, source command is executed in the cli side like sql and call command which is unlike the initial file and -f option. So I prefer to handle it in the command level.
3. WRT the last line issue, do you mean **output(""); // dummy new line**? If so, I think it's not necessary since we treat it as a single command. The content should not be displayed in the cli as the old CLI.
```
>cat /root/workspace/test.sql
create table test2(a string, b string);
```
In old cli
```
hive> source /root/workspace/test.sql
> ;
OK
Time taken: 0.192 seconds
hive> show tables;
OK
test2
Time taken: 0.117 seconds, Fetched: 1 row(s)
```
- cheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/#review89421
-----------------------------------------------------------
On June 25, 2015, 1:54 p.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35107/
> -----------------------------------------------------------
>
> (Updated June 25, 2015, 1:54 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 b7d2f2e
> beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
> common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
> common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
> ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
> ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
> ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
> ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
> ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
> ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c
> service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b
>
> Diff: https://reviews.apache.org/r/35107/diff/
>
>
> Testing
> -------
>
> Unit test passed
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/#review89421
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/Commands.java (line 721)
<https://reviews.apache.org/r/35107/#comment142021>
Nit: Keep @Override in a separate line.
beeline/src/java/org/apache/hive/beeline/Commands.java (line 722)
<https://reviews.apache.org/r/35107/#comment142022>
I think getHiveVariables() is in the same class, which can be directly accessed.
beeline/src/java/org/apache/hive/beeline/Commands.java (line 760)
<https://reviews.apache.org/r/35107/#comment142023>
I think getHiveVariables() can be private.
beeline/src/java/org/apache/hive/beeline/Commands.java (line 772)
<https://reviews.apache.org/r/35107/#comment142024>
getHiveConf() can be proviate. Remove the caller from BeeLine.java.
beeline/src/java/org/apache/hive/beeline/Commands.java (line 850)
<https://reviews.apache.org/r/35107/#comment142030>
This used to call BeeLine.executeFile() now you have a new implementation. I don't quite follow why. Regardless, I see two problems.
1. we should remove the 2nd argument as all callers provide "false".
2. Some part of the code in executeFile() is to fix some problem with the last line in the script file. Will your new implementation catches that?
3. It's my understanding that in Hive CLI compatible mode, commands in source file should follow Hive CLI syntax, while in normal mode, it should follow Beeline syntax. Is this what's done here?
- Xuefu Zhang
On June 25, 2015, 5:54 a.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35107/
> -----------------------------------------------------------
>
> (Updated June 25, 2015, 5:54 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 b7d2f2e
> beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
> common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
> common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
> ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
> ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
> ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
> ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
> ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
> ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c
> service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b
>
> Diff: https://reviews.apache.org/r/35107/diff/
>
>
> Testing
> -------
>
> Unit test passed
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/
-----------------------------------------------------------
(Updated June 25, 2015, 1:54 p.m.)
Review request for hive, chinna and Xuefu Zhang.
Changes
-------
code clean
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 (updated)
-----
beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e
beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3
beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755
ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b
Diff: https://reviews.apache.org/r/35107/diff/
Testing
-------
Unit test passed
Thanks,
cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35107/
-----------------------------------------------------------
(Updated June 24, 2015, 3:39 p.m.)
Review request for hive, chinna and Xuefu Zhang.
Changes
-------
Update patch addressing Xuefu's comments
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 (updated)
-----
beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e
beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3
beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION
common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755
ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f
ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96
ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53
ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168
ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82
ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d
ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b
Diff: https://reviews.apache.org/r/35107/diff/
Testing
-------
Unit test passed
Thanks,
cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
> 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.
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
-----------------------------------------------------------
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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by Lefty Leverenz <le...@gmail.com>.
> 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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
> 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.
>
> 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.
>
> Lefty Leverenz wrote:
> 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 .
Thanks Lefty and Xuefu for providing this information.
- 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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
> 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.
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.
- 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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by Xuefu Zhang <xz...@cloudera.com>.
> 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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
> 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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by Xuefu Zhang <xz...@cloudera.com>.
> 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.
>
> 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)
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.
- 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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
> 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?
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)
- 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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by Xuefu Zhang <xz...@cloudera.com>.
> 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.
>
> 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!
I don't think they are specifically documented. I believe they are in SQLLIne doc, to which Beeline doc has a reference.
- 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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 35107: HIVE-6791 Support variable substition for
Beeline shell command
Posted by cheng xu <ch...@intel.com>.
-----------------------------------------------------------
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 (updated)
-----
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