> 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
>
>
