You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Dong Chen <do...@intel.com> on 2014/09/12 09:26:11 UTC
Review Request 25575: HIVE-7615: Beeline should have an option for
user to see the query progress
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/
-----------------------------------------------------------
Review request for hive.
Repository: hive-git
Description
-------
When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
Diffs
-----
beeline/pom.xml 45fa02b
beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
Diff: https://reviews.apache.org/r/25575/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
> On Sept. 15, 2014, 6:56 p.m., Brock Noland wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java, line 93
> > <https://reviews.apache.org/r/25575/diff/1/?file=687534#file687534line93>
> >
> > Used internally can be private
>
> Dong Chen wrote:
> Class Commands also use it. Maybe we have to keep it public.
This enum is removed from patch
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review53376
-----------------------------------------------------------
On Sept. 24, 2014, 9:17 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2014, 9:17 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
> jdbc/src/java/org/apache/hive/jdbc/ClosedOrCancelledStatementException.java PRE-CREATION
> jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 86bc580
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
> On Sept. 15, 2014, 6:56 p.m., Brock Noland wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java, line 93
> > <https://reviews.apache.org/r/25575/diff/1/?file=687534#file687534line93>
> >
> > Used internally can be private
Class Commands also use it. Maybe we have to keep it public.
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review53376
-----------------------------------------------------------
On Sept. 18, 2014, 6:30 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2014, 6:30 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review53376
-----------------------------------------------------------
Looks great!! I have a few comments.
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment93025>
Let's make this 1 second.
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment93021>
I think the + 100000 portion needs to be a constant. Let's make that a large number, like 30 minutes.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93022>
1) let's not import *
2) remove x_xbitmap
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93023>
Used internally can be private
- Brock Noland
On Sept. 12, 2014, 7:26 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 12, 2014, 7:26 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review54058
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment94024>
It would be better to call logThread.interrupt() before stmt.close(). That way getLog is less likely to get called when the handle is null.
I think we should also synchronously get any remaining logs before closing the statement. That will ensure any last remaining log lines have been fetched.
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment94023>
I agree with Brock. It would be better to just return empty list in this case the query has not started. This can happen in normal operation.
For the case where statement has been closed/cancelled, I think it makes sense to throw a distinct exception. Say a 'ClosedOrCancelledStatement extends SQLException'.
I think we should throw the exception irrespective of the query suceeding or failing. QUery succeeding or failing is not relavent for the getLog api.
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java
<https://reviews.apache.org/r/25575/#comment94029>
synchronously getting any last few lines would make this test case more robust. Otherwhise, it is possible that test fails on some slow virtual machines.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93996>
Brock,
That order that is better is highly subjective IMO.
For me this is more natural
if(valid common case) {
} else {
}
For me "if( not null)" is actually checking for a valid case, and seems more natural to me.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment94003>
In this case, the if-else looks quite readable to me.
Should we be removing else where ever possible ?
In my opinion, we can leave such very subjective options to the author, as long as it does not violate the coding standards of hive/oracle-java (or another well known coding guildline that is compatible with Hive's), and does not deviate from the style followed in hive or locally in that class.
- Thejas Nair
On Sept. 19, 2014, 9:22 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2014, 9:22 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
> On Sept. 19, 2014, 9:32 p.m., Swarnim Kulkarni wrote:
> > beeline/src/java/org/apache/hive/beeline/Commands.java, line 833
> > <https://reviews.apache.org/r/25575/diff/5/?file=694548#file694548line833>
> >
> > Also show we be checking on the SQL error code vs the message. Grepping on the error seems a little risky.
Thanks for pointing out this risky code. The new patch return an empty list instead.
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review54027
-----------------------------------------------------------
On Sept. 24, 2014, 9:17 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2014, 9:17 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
> jdbc/src/java/org/apache/hive/jdbc/ClosedOrCancelledStatementException.java PRE-CREATION
> jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 86bc580
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Swarnim Kulkarni <ku...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review54027
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment93921>
Nit: Can move the logic inline.
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment93910>
Also show we be checking on the SQL error code vs the message. Grepping on the error seems a little risky.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93925>
Nit: Could skip else
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93924>
Nit: Could skip the else.
- Swarnim Kulkarni
On Sept. 19, 2014, 9:22 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2014, 9:22 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
> On Sept. 19, 2014, 8:52 p.m., Brock Noland wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java, line 768
> > <https://reviews.apache.org/r/25575/diff/5/?file=694551#file694551line768>
> >
> > if ( not null) {
> >
> > } else {
> >
> >
> > is simpler as:
> >
> > if (null) {
> >
> > } else {
I think it is better to put the valid common case at first, like Thejas metioned in below comments. Shall we keep the order here?
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review54023
-----------------------------------------------------------
On Sept. 24, 2014, 9:17 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2014, 9:17 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
> jdbc/src/java/org/apache/hive/jdbc/ClosedOrCancelledStatementException.java PRE-CREATION
> jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 86bc580
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review54023
-----------------------------------------------------------
Excellent work as usual! I have a few items below but I think is really close!
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment93899>
I think in this case it makes sense to turn an empty list as opposed to throwing an exception. All clients will have to do this same thing we are doing.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93901>
if ( not null) {
} else {
is simpler as:
if (null) {
} else {
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93900>
As mentioned above, in this case let's return an empty list.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93902>
Let's change this to:
throw new SQLException("Error when getting query log: " + e, e);
- Brock Noland
On Sept. 19, 2014, 9:22 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2014, 9:22 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review54464
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment94611>
I think it better to change this to a warning,as it is only error in getting logs, not query execution.
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment94612>
same here. beeLine.warn would be better, it is less likely to scare users.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment94610>
We should indicate that the getQueryLog is what failed, and it was not the query execution that afailed (as you already do in other exceptions)
- Thejas Nair
On Sept. 24, 2014, 9:51 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2014, 9:51 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
> jdbc/src/java/org/apache/hive/jdbc/ClosedOrCancelledStatementException.java PRE-CREATION
> jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 86bc580
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review54586
-----------------------------------------------------------
Ship it!
Ship It!
- Thejas Nair
On Sept. 25, 2014, 8:43 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2014, 8:43 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
> jdbc/src/java/org/apache/hive/jdbc/ClosedOrCancelledStatementException.java PRE-CREATION
> jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 86bc580
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/
-----------------------------------------------------------
(Updated Sept. 25, 2014, 8:43 a.m.)
Review request for hive.
Changes
-------
Update patch V4 to address error logging comments.
Repository: hive-git
Description
-------
When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
Diffs (updated)
-----
beeline/pom.xml 45fa02b
beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
jdbc/src/java/org/apache/hive/jdbc/ClosedOrCancelledStatementException.java PRE-CREATION
jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 86bc580
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
Diff: https://reviews.apache.org/r/25575/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/
-----------------------------------------------------------
(Updated Sept. 24, 2014, 9:51 a.m.)
Review request for hive.
Changes
-------
a little change on the updated patch. Move the place of cancelled state checking in getQueryLog().
Repository: hive-git
Description
-------
When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
Diffs (updated)
-----
beeline/pom.xml 45fa02b
beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
jdbc/src/java/org/apache/hive/jdbc/ClosedOrCancelledStatementException.java PRE-CREATION
jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 86bc580
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
Diff: https://reviews.apache.org/r/25575/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/
-----------------------------------------------------------
(Updated Sept. 24, 2014, 9:17 a.m.)
Review request for hive.
Changes
-------
Update patch based on the comments from Thejas, Brock, and Swarnim. This patch mainly add a public API hasMoreLogs() and modify the internal status tracking of HiveStatement.
Repository: hive-git
Description
-------
When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
Diffs (updated)
-----
beeline/pom.xml 45fa02b
beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
jdbc/src/java/org/apache/hive/jdbc/ClosedOrCancelledStatementException.java PRE-CREATION
jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 86bc580
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
Diff: https://reviews.apache.org/r/25575/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/
-----------------------------------------------------------
(Updated Sept. 19, 2014, 9:22 a.m.)
Review request for hive.
Changes
-------
Update the patch based on comments. Mainly change the HiveStatement exposed public API to minimal. So remove the QueryState.
Repository: hive-git
Description
-------
When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
Diffs (updated)
-----
beeline/pom.xml 45fa02b
beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
Diff: https://reviews.apache.org/r/25575/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review53789
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment93592>
It is safer to also set logThread.setDaemon(true)
beeline/src/java/org/apache/hive/beeline/Commands.java
<https://reviews.apache.org/r/25575/#comment93591>
Lets print debug instead of error here. It would be good to keep beeline usable with other jdbc drivers as well.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93622>
I think we need a public method in HiveStatement for use by other custom 'jdbc' applications.
I think the interface of the function should be -
public List<String> getQueryLog(boolean incremental, int fetchSize).
Can you change this function to get fetchSize as argument ? And also change javadoc of this method to say that this method is a public api (for use outside of hive), but other methods in the class that are not part of java.sql.Statement are considered internal hive apis.
- Thejas Nair
On Sept. 18, 2014, 6:30 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2014, 6:30 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
> On Sept. 18, 2014, 8:07 a.m., Thejas Nair wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java, line 98
> > <https://reviews.apache.org/r/25575/diff/4/?file=693372#file693372line98>
> >
> > Looks like we just need to check isRunning (ie true or false) for running.
> > We don't need the whole set of enums, and complexity associated with getting those values right.
> > Can we just use a boolean instead of the enum ?
> >
> > Apart from complexity, the reason for using boolean instead of enum - I am also concerned about people starting to use these enums and then complain if we change them in future. Even though we haven't documented it as a public api, people might start using them. So keeping the functionality we promise through methods here to minimal is safer.
Use the method in below comments to track HiveStatement status.
> On Sept. 18, 2014, 8:07 a.m., Thejas Nair wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java, line 247
> > <https://reviews.apache.org/r/25575/diff/4/?file=693372#file693372line247>
> >
> > set isRunning=true here and set isRunning=false in other place of other calls to setQueryStatus
Use the method in below comments to track HiveStatement status.
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review53809
-----------------------------------------------------------
On Sept. 24, 2014, 9:17 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2014, 9:17 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 1e66542
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java daf8e9e
> jdbc/src/java/org/apache/hive/jdbc/ClosedOrCancelledStatementException.java PRE-CREATION
> jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 86bc580
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/#review53809
-----------------------------------------------------------
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java
<https://reviews.apache.org/r/25575/#comment93627>
We can keep the logic here (and any other application using getQueryLog method) simpler by checking the query status (running or not) within statement.getQueryLog, instead of doing it here.
That way we can keep the method get state of HiveStatement a private api.
I don't think we need this timeout loop. As long as execute call hasn't returned we won't be able to make any calls to get log because of the transportLock.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93624>
Looks like we just need to check isRunning (ie true or false) for running.
We don't need the whole set of enums, and complexity associated with getting those values right.
Can we just use a boolean instead of the enum ?
Apart from complexity, the reason for using boolean instead of enum - I am also concerned about people starting to use these enums and then complain if we change them in future. Even though we haven't documented it as a public api, people might start using them. So keeping the functionality we promise through methods here to minimal is safer.
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
<https://reviews.apache.org/r/25575/#comment93625>
set isRunning=true here and set isRunning=false in other place of other calls to setQueryStatus
- Thejas Nair
On Sept. 18, 2014, 6:30 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25575/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2014, 6:30 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
>
>
> Diffs
> -----
>
> beeline/pom.xml 45fa02b
> beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
>
> Diff: https://reviews.apache.org/r/25575/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/
-----------------------------------------------------------
(Updated Sept. 18, 2014, 6:30 a.m.)
Review request for hive.
Changes
-------
Remove the TODO comment.
Repository: hive-git
Description
-------
When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
Diffs (updated)
-----
beeline/pom.xml 45fa02b
beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
Diff: https://reviews.apache.org/r/25575/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/
-----------------------------------------------------------
(Updated Sept. 18, 2014, 6:24 a.m.)
Review request for hive.
Repository: hive-git
Description
-------
When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
Diffs (updated)
-----
beeline/pom.xml 45fa02b
beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
Diff: https://reviews.apache.org/r/25575/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen
Re: Review Request 25575: HIVE-7615: Beeline should have an option
for user to see the query progress
Posted by Dong Chen <do...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25575/
-----------------------------------------------------------
(Updated Sept. 18, 2014, 6:20 a.m.)
Review request for hive.
Repository: hive-git
Description
-------
When executing query in Beeline, user should have a option to see the progress through the outputs. Beeline could use the API introduced in HIVE-4629 to get and display the logs to the client.
Diffs (updated)
-----
beeline/pom.xml 45fa02b
beeline/src/java/org/apache/hive/beeline/Commands.java a92d69f
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 2cbf58c
Diff: https://reviews.apache.org/r/25575/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen