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