> On April 2, 2015, 8:56 p.m., Parth Chandra wrote:
> > exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java, line 105
> > <https://reviews.apache.org/r/32494/diff/1/?file=911345#file911345line105>
> >
> >     Did you check this with SQLLine? If a query is returning a large result 
> > set and the user cancels, the result set is closed as part of the main 
> > thread but another thread will still call next(). This will cause an ugly 
> > exception to be printed even though this is a normal case.

Isn't that SQLLine scenario a JDBC client (SQLLine) problem, rather than a JDBC 
driver problem? 

SQLLine's main thread should prevent the other thread from making future calls 
to next before that main thread closes the ResultSet, Statement, or Connection.

Note that:

1:  The behavior I have implemented is specified by JDBC.  Many methods are 
specified to "[throw] SQLException - if ... called on a closed result set" (or 
statement or connection).

2.  This DRILL-2565 fix is just a partial fix, for higher-priority parts of 
DRILL-2489.


> On April 2, 2015, 8:56 p.m., Parth Chandra wrote:
> > exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java, line 65
> > <https://reviews.apache.org/r/32494/diff/1/?file=911346#file911346line65>
> >
> >     I think the parent throws an UnsupportedOperationException for the 
> > update methods. Don't see why we need to override this.

As per the other comment, I implemented the behavior specified by JDBC.  

(Throwing unchecked exception UnsupportedOperationException is not sanctioned 
by the JDBC documentation, and JDBC already provides (checked) exception 
SQLFeatureNotSupportedException (though it's stated to be for _optional_ JDBC 
features, not non-optional features Drill doesn't/won't support).)


- Daniel


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


On March 31, 2015, 9:32 p.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32494/
> -----------------------------------------------------------
> 
> (Updated March 31, 2015, 9:32 p.m.)
> 
> 
> Review request for drill, Mehant Baid and Parth Chandra.
> 
> 
> Bugs: DRILL-2565
>     https://issues.apache.org/jira/browse/DRILL-2565
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> - Created AlreadyClosedSqlException.
> - (Moved JdbcApiSqlException to be subclass of SQLNonTransientException.)
> - Created test Drill2489CallsAfterCloseThrowExceptionsTest for eventual 
> fixing of DRILL-2489.
>    - (Is partial:  Covers Connection, Statement, and ResultSet.)
>    - (Is interim:  Most methods' test disabled with @Ignore.)
> - Added already-closed checking in key places, especially those that involve 
> communication and could hang for a while rather than dying quickly (e.g., 
> Statement.execute...(...)).
> 
> 
> Diffs
> -----
> 
>   
> exec/jdbc/src/main/java/org/apache/drill/jdbc/AlreadyClosedSqlException.java 
> PRE-CREATION 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 
> e590778 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillDatabaseMetaData.java 
> ca1648d 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 0ce33f4 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java d934c7c 
>   
> exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java
>  7b04371 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java 
> d6b05fb 
>   
> exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2489CallsAfterCloseThrowExceptionsTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/32494/diff/
> 
> 
> Testing
> -------
> 
> Ran new specific tests.
> 
> Run existing tests; no new problems.
> 
> 
> Thanks,
> 
> Daniel Barclay
> 
>

Reply via email to