drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel Barclay" <dbarc...@maprtech.com>
Subject Re: Review Request 33651: DRILL-2884: Have cancel() cause "query canceled" rather than just "ResultSet closed".
Date Sat, 02 May 2015 00:21:49 GMT


> On May 1, 2015, 11:52 p.m., Parth Chandra wrote:
> > exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java, line
145
> > <https://reviews.apache.org/r/33651/diff/1/?file=944582#file944582line145>
> >
> >     I don't get why you removed the call to checkNotClosed here.

The main reason is that that enclosing execute() method is not a JDBC-defined method (it's
an Avatica-defined method), so it's not at the JDBC boundary where methods are defined to
throw SQLException if the ResultSet is closed, and where I had intended to use checkNotClosed().

(A secondary reasons is that it doesn't otherwise seem to be needed (cancelation works fine
and the JDBC client still gets one "canceled" exception (via ResultSet method calls).)


- Daniel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33651/#review82294
-----------------------------------------------------------


On April 29, 2015, 4:26 p.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33651/
> -----------------------------------------------------------
> 
> (Updated April 29, 2015, 4:26 p.m.)
> 
> 
> Review request for drill, Mehant Baid and Parth Chandra.
> 
> 
> Bugs: drill-2884
>     https://issues.apache.org/jira/browse/drill-2884
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Changed DrillResultSetImpl to throw new ExecutionCanceledSqlException, rather than (previously
existing) AlreadyClosedSqlException, at the first call to the result set after the associated
SQL statement's execution is canceled (except that currently only some methods check and throw
those exceptions).
> 
> Also fixed two little documentation errors in DrillResultSet.
> 
> 
> Diffs
> -----
> 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 7a4f426 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java ba265e6 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/ExecutionCanceledSqlException.java PRE-CREATION

>   exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java 484a5e5

> 
> Diff: https://reviews.apache.org/r/33651/diff/
> 
> 
> Testing
> -------
> 
> Manually tested in SQLLine, straight and in debugger (to have control-C INT thread run
at different times relative to query execution flow).
> 
> 
> Ran regular tests.
> 
> 
> Thanks,
> 
> Daniel Barclay
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message