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/09/02 09:16:15 UTC
Review Request 38044: HIVE-11640 Shell command doesn't work for new
CLI[Beeline-cli branch]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38044/
-----------------------------------------------------------
Review request for hive and Xuefu Zhang.
Bugs: HIVE-11640
https://issues.apache.org/jira/browse/HIVE-11640
Repository: hive-git
Description
-------
Fixed issues includes:
* Support the case that one line contains multi-commands with ";" seperated
* Fix the nullempty string for new cli mode
* Resolve command doesn't support issue when executing files
Diffs
-----
beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b
beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec
beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea
Diff: https://reviews.apache.org/r/38044/diff/
Testing
-------
Local smoke test passed and unit test passed locally
Thanks,
cheng xu
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for
new CLI[Beeline-cli branch]
Posted by cheng xu <ch...@intel.com>.
> On Sept. 7, 2015, 9:33 p.m., Xuefu Zhang wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 1094
> > <https://reviews.apache.org/r/38044/diff/1/?file=1062103#file1062103line1094>
> >
> > I meant if it's possible to have multiple matches (cmdMap.size() > 1). I asked this because in line 1080-1084 we detect if there is a match when putting a match into the map and if so throw an error. Therefore, when we reach line 1091, cmdMap.size() should be less than or equal to 1. Did I miss anything?
I got your point. Line 1080-1084 handles the following case.
```java
new ReflectiveCommandHandler(this, new String[] {"A","B"},
new Completer[] {new TableNameCompletor(this)}),
new ReflectiveCommandHandler(this, new String[] {"A"},
new Completer[] {new TableNameCompletor(this)}),
```
If the line is "!A B;" then the error will be thrown. It's more like a internal check for programmer to avoid them use the same keywords for a command hander.
Remove Line 1080-1084 since it's not reasonable to have single line match the multiple ReflectiveCommandHandler.
- cheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38044/#review97919
-----------------------------------------------------------
On Sept. 8, 2015, 9:45 a.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38044/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2015, 9:45 a.m.)
>
>
> Review request for hive and Xuefu Zhang.
>
>
> Bugs: HIVE-11640
> https://issues.apache.org/jira/browse/HIVE-11640
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Fixed issues includes:
> * Support the case that one line contains multi-commands with ";" seperated
> * Fix the nullempty string for new cli mode
> * Resolve command doesn't support issue when executing files
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b
> beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea
>
> Diff: https://reviews.apache.org/r/38044/diff/
>
>
> Testing
> -------
>
> Local smoke test passed and unit test passed locally
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for
new CLI[Beeline-cli branch]
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38044/#review97919
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 1091)
<https://reviews.apache.org/r/38044/#comment154029>
I meant if it's possible to have multiple matches (cmdMap.size() > 1). I asked this because in line 1080-1084 we detect if there is a match when putting a match into the map and if so throw an error. Therefore, when we reach line 1091, cmdMap.size() should be less than or equal to 1. Did I miss anything?
- Xuefu Zhang
On Sept. 2, 2015, 7:16 a.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38044/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 7:16 a.m.)
>
>
> Review request for hive and Xuefu Zhang.
>
>
> Bugs: HIVE-11640
> https://issues.apache.org/jira/browse/HIVE-11640
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Fixed issues includes:
> * Support the case that one line contains multi-commands with ";" seperated
> * Fix the nullempty string for new cli mode
> * Resolve command doesn't support issue when executing files
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b
> beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea
>
> Diff: https://reviews.apache.org/r/38044/diff/
>
>
> Testing
> -------
>
> Local smoke test passed and unit test passed locally
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for
new CLI[Beeline-cli branch]
Posted by cheng xu <ch...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38044/#review97878
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 1091)
<https://reviews.apache.org/r/38044/#comment153947>
That's possible. You can deep into the code of AbstractCommandHandler. The condition of match is whether the names is starting with the first part of the line. It means the command "!l;" is valid since there's only one match "list" exists.
- cheng xu
On Sept. 2, 2015, 3:16 p.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38044/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 3:16 p.m.)
>
>
> Review request for hive and Xuefu Zhang.
>
>
> Bugs: HIVE-11640
> https://issues.apache.org/jira/browse/HIVE-11640
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Fixed issues includes:
> * Support the case that one line contains multi-commands with ";" seperated
> * Fix the nullempty string for new cli mode
> * Resolve command doesn't support issue when executing files
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b
> beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea
>
> Diff: https://reviews.apache.org/r/38044/diff/
>
>
> Testing
> -------
>
> Local smoke test passed and unit test passed locally
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for
new CLI[Beeline-cli branch]
Posted by cheng xu <ch...@intel.com>.
> On Sept. 3, 2015, 1:34 a.m., Xuefu Zhang wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 1094
> > <https://reviews.apache.org/r/38044/diff/1/?file=1062103#file1062103line1094>
> >
> > with line 1081-1085, is this still possible? I understand this is just moving code around.
That's possible. You can deep into the code of AbstractCommandHandler. The condition of match is whether the names is starting with the first part of the line. It means the command "!l;" is valid since there's only one match "list" exists.
- cheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38044/#review97494
-----------------------------------------------------------
On Sept. 2, 2015, 3:16 p.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38044/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 3:16 p.m.)
>
>
> Review request for hive and Xuefu Zhang.
>
>
> Bugs: HIVE-11640
> https://issues.apache.org/jira/browse/HIVE-11640
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Fixed issues includes:
> * Support the case that one line contains multi-commands with ";" seperated
> * Fix the nullempty string for new cli mode
> * Resolve command doesn't support issue when executing files
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b
> beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea
>
> Diff: https://reviews.apache.org/r/38044/diff/
>
>
> Testing
> -------
>
> Local smoke test passed and unit test passed locally
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for
new CLI[Beeline-cli branch]
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38044/#review97494
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 1091)
<https://reviews.apache.org/r/38044/#comment153386>
with line 1081-1085, is this still possible? I understand this is just moving code around.
- Xuefu Zhang
On Sept. 2, 2015, 7:16 a.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38044/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 7:16 a.m.)
>
>
> Review request for hive and Xuefu Zhang.
>
>
> Bugs: HIVE-11640
> https://issues.apache.org/jira/browse/HIVE-11640
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Fixed issues includes:
> * Support the case that one line contains multi-commands with ";" seperated
> * Fix the nullempty string for new cli mode
> * Resolve command doesn't support issue when executing files
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b
> beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea
>
> Diff: https://reviews.apache.org/r/38044/diff/
>
>
> Testing
> -------
>
> Local smoke test passed and unit test passed locally
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for
new CLI[Beeline-cli branch]
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38044/#review97958
-----------------------------------------------------------
Ship it!
Ship It!
- Xuefu Zhang
On Sept. 8, 2015, 1:45 a.m., cheng xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38044/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2015, 1:45 a.m.)
>
>
> Review request for hive and Xuefu Zhang.
>
>
> Bugs: HIVE-11640
> https://issues.apache.org/jira/browse/HIVE-11640
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Fixed issues includes:
> * Support the case that one line contains multi-commands with ";" seperated
> * Fix the nullempty string for new cli mode
> * Resolve command doesn't support issue when executing files
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b
> beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea
>
> Diff: https://reviews.apache.org/r/38044/diff/
>
>
> Testing
> -------
>
> Local smoke test passed and unit test passed locally
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for
new CLI[Beeline-cli branch]
Posted by cheng xu <ch...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38044/
-----------------------------------------------------------
(Updated Sept. 8, 2015, 9:45 a.m.)
Review request for hive and Xuefu Zhang.
Bugs: HIVE-11640
https://issues.apache.org/jira/browse/HIVE-11640
Repository: hive-git
Description
-------
Fixed issues includes:
* Support the case that one line contains multi-commands with ";" seperated
* Fix the nullempty string for new cli mode
* Resolve command doesn't support issue when executing files
Diffs (updated)
-----
beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b
beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec
beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea
Diff: https://reviews.apache.org/r/38044/diff/
Testing
-------
Local smoke test passed and unit test passed locally
Thanks,
cheng xu