You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Parth Chandra <pc...@maprtech.com> on 2015/05/02 01:52:17 UTC
Re: Review Request 33651: DRILL-2884: Have cancel() cause "query
canceled" rather than just "ResultSet closed".
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33651/#review82294
-----------------------------------------------------------
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
<https://reviews.apache.org/r/33651/#comment133031>
I don't get why you removed the call to checkNotClosed here.
- Parth Chandra
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
>
>
Re: Review Request 33651: DRILL-2884: Have cancel() cause "query
canceled" rather than just "ResultSet closed".
Posted by Daniel Barclay <db...@maprtech.com>.
> 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
>
>