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

Reply via email to