Re: 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/#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
--- 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
--- 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
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
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
--- 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
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
--- 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
--- 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
--- 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
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
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
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
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
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
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
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
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
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
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
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
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
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
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
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/
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
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
--- 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
--- 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
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
