You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by pa...@apache.org on 2015/04/07 04:14:59 UTC
[5/9] drill git commit: DRILL-2565: Add some key "already-closed"
checks, with test for future DRILL-2489 work.
DRILL-2565: Add some key "already-closed" checks, with test for future DRILL-2489 work.
- 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' tests disabled with @Ignore("...DRILL-2489...").)
- 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...(...)).
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/f066786e
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/f066786e
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/f066786e
Branch: refs/heads/master
Commit: f066786e45b1c3ce93fafd339f0ad9c7bf4904bc
Parents: b2e9cd0
Author: dbarclay <db...@maprtech.com>
Authored: Tue Mar 24 22:58:33 2015 -0700
Committer: Parth Chandra <pc...@maprtech.com>
Committed: Mon Apr 6 18:24:06 2015 -0700
----------------------------------------------------------------------
.../drill/jdbc/AlreadyClosedSqlException.java | 95 +
.../apache/drill/jdbc/DrillConnectionImpl.java | 13 +
.../drill/jdbc/DrillDatabaseMetaData.java | 67 +-
.../org/apache/drill/jdbc/DrillResultSet.java | 21 +
.../org/apache/drill/jdbc/DrillStatement.java | 46 +
.../jdbc/InvalidCursorStateSqlException.java | 3 +-
.../apache/drill/jdbc/JdbcApiSqlException.java | 11 +-
...l2489CallsAfterCloseThrowExceptionsTest.java | 1862 ++++++++++++++++++
8 files changed, 2108 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/f066786e/exec/jdbc/src/main/java/org/apache/drill/jdbc/AlreadyClosedSqlException.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/AlreadyClosedSqlException.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/AlreadyClosedSqlException.java
new file mode 100644
index 0000000..6e41bb4
--- /dev/null
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/AlreadyClosedSqlException.java
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.jdbc;
+
+import java.sql.ResultSet;
+
+
+/**
+ * SQLException for object-already-closed conditions, e.g., calling a method
+ * on a closed {@link Statement}.
+ */
+public class AlreadyClosedSqlException extends JdbcApiSqlException {
+
+ private static final long serialVersionUID = 2015_03_25L;
+
+ /**
+ * See {@link JdbcApiSqlException#JdbcApiSqlException(String, String, int)}.
+ * /
+ public AlreadyClosedSqlException( String reason,
+ String SQLState,
+ int vendorCode ) {
+ super( reason, SQLState, vendorCode );
+ }
+
+ /**
+ * See {@link JdbcApiSqlException#JdbcApiSqlException(String, String)}.
+ * /
+ public AlreadyClosedSqlException( String reason, String SQLState ) {
+ super( reason, SQLState );
+ }
+
+ /**
+ * See {@link JdbcApiSqlException#JdbcApiSqlException(String)}.
+ */
+ public AlreadyClosedSqlException( String reason ) {
+ super( reason );
+ }
+
+ /**
+ * See {@link JdbcApiSqlException#JdbcApiSqlException()}.
+ */
+ public AlreadyClosedSqlException() {
+ super();
+ }
+
+ /**
+ * See {@link JdbcApiSqlException#JdbcApiSqlException(Throwable cause)}.
+ */
+ public AlreadyClosedSqlException( Throwable cause ) {
+ super( cause );
+ }
+
+ /**
+ * See {@link JdbcApiSqlException#JdbcApiSqlException(String, Throwable)}.
+ */
+ public AlreadyClosedSqlException( String reason, Throwable cause ) {
+ super( reason, cause );
+ }
+
+ /**
+ * See
+ * {@link JdbcApiSqlException#JdbcApiSqlException(String, String, Throwable)}.
+ */
+ public AlreadyClosedSqlException( String reason, String sqlState,
+ Throwable cause ) {
+ super( reason, sqlState, cause );
+ }
+
+ /**
+ * See
+ * {@link JdbcApiSqlException#JdbcApiSqlException(String, String, int, Throwable)}.
+ */
+ public AlreadyClosedSqlException( String reason,
+ String sqlState,
+ int vendorCode,
+ Throwable cause ) {
+ super( reason, sqlState, vendorCode, cause );
+ }
+
+}
http://git-wip-us.apache.org/repos/asf/drill/blob/f066786e/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java
index 8caeb3f..170495e 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java
@@ -55,6 +55,17 @@ abstract class DrillConnectionImpl extends AvaticaConnection implements DrillCon
private Drillbit bit;
private RemoteServiceSet serviceSet;
+ /**
+ * Throws AlreadyClosedSqlException if this Connection is closed.
+ *
+ * @throws AlreadyClosedSqlException if Connection is closed
+ * @throws SQLException if error in calling {@link #isClosed()}
+ */
+ private void checkNotClosed() throws SQLException {
+ if ( isClosed() ) {
+ throw new AlreadyClosedSqlException( "Connection is already closed." );
+ }
+ }
protected DrillConnectionImpl(Driver driver, AvaticaFactory factory, String url, Properties info) throws SQLException {
super(driver, factory, url, info);
@@ -128,6 +139,7 @@ abstract class DrillConnectionImpl extends AvaticaConnection implements DrillCon
@Override
public DrillStatement createStatement(int resultSetType, int resultSetConcurrency,
int resultSetHoldability) throws SQLException {
+ checkNotClosed();
DrillStatement statement =
(DrillStatement) super.createStatement(resultSetType, resultSetConcurrency,
resultSetHoldability);
@@ -138,6 +150,7 @@ abstract class DrillConnectionImpl extends AvaticaConnection implements DrillCon
public PreparedStatement prepareStatement(String sql, int resultSetType,
int resultSetConcurrency,
int resultSetHoldability) throws SQLException {
+ checkNotClosed();
try {
DrillPrepareResult prepareResult = new DrillPrepareResult(sql);
DrillPreparedStatement statement =
http://git-wip-us.apache.org/repos/asf/drill/blob/f066786e/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillDatabaseMetaData.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillDatabaseMetaData.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillDatabaseMetaData.java
index ca1648d..4b2d694 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillDatabaseMetaData.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillDatabaseMetaData.java
@@ -17,6 +17,9 @@
*/
package org.apache.drill.jdbc;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
import net.hydromatic.avatica.AvaticaConnection;
import net.hydromatic.avatica.AvaticaDatabaseMetaData;
@@ -26,28 +29,84 @@ public class DrillDatabaseMetaData extends AvaticaDatabaseMetaData {
super( connection );
}
+ /**
+ * Throws AlreadyClosedSqlException if the associated Connection is closed.
+ *
+ * @throws AlreadyClosedSqlException if Connection is closed
+ * @throws SQLException if error in calling {@link Connection#isClosed()}
+ */
+ private void checkNotClosed() throws AlreadyClosedSqlException,
+ SQLException {
+ if ( getConnection().isClosed() ) {
+ throw new AlreadyClosedSqlException(
+ "DatabaseMetaData's Connection is already closed." );
+ }
+ }
+
// For omitted NULLS FIRST/NULLS HIGH, Drill sort NULL sorts as highest value:
@Override
- public boolean nullsAreSortedHigh() {
+ public boolean nullsAreSortedHigh() throws SQLException {
+ checkNotClosed();
return true;
}
@Override
- public boolean nullsAreSortedLow() {
+ public boolean nullsAreSortedLow() throws SQLException {
+ checkNotClosed();
return false;
}
@Override
- public boolean nullsAreSortedAtStart() {
+ public boolean nullsAreSortedAtStart() throws SQLException {
+ checkNotClosed();
return false;
}
@Override
- public boolean nullsAreSortedAtEnd() {
+ public boolean nullsAreSortedAtEnd() throws SQLException {
+ checkNotClosed();
return false;
}
+ // For now, check whether connection is closed for most important methods
+ // (DRILL-2565 (partial fix for DRILL-2489)):
+
+
+ @Override
+ public ResultSet getCatalogs() throws SQLException {
+ checkNotClosed();
+ return super.getCatalogs();
+ }
+
+ @Override
+ public ResultSet getSchemas() throws SQLException {
+ checkNotClosed();
+ return super.getSchemas();
+ }
+
+ @Override
+ public ResultSet getSchemas( String catalog, String schemaPattern ) throws SQLException {
+ checkNotClosed();
+ return super.getSchemas( catalog, schemaPattern );
+ }
+
+ @Override
+ public ResultSet getTables( String catalog,
+ String schemaPattern,
+ String tableNamePattern,
+ String[] types ) throws SQLException {
+ checkNotClosed();
+ return super.getTables( catalog, schemaPattern,tableNamePattern, types );
+ }
+
+ @Override
+ public ResultSet getColumns( String catalog, String schema, String table,
+ String columnNamePattern ) throws SQLException {
+ checkNotClosed();
+ return super.getColumns( catalog, schema, table, columnNamePattern );
+ }
+
}
http://git-wip-us.apache.org/repos/asf/drill/blob/f066786e/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java
index fb27d2d..74900bc 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java
@@ -62,6 +62,25 @@ public class DrillResultSet extends AvaticaResultSet {
cursor = new DrillCursor(this);
}
+ /**
+ * Throws AlreadyClosedSqlException if this ResultSet is closed.
+ *
+ * @throws AlreadyClosedSqlException if ResultSe is closed
+ * @throws SQLException if error in calling {@link #isClosed()}
+ */
+ private void checkNotClosed() throws SQLException {
+ if ( isClosed() ) {
+ throw new AlreadyClosedSqlException( "ResultSet is already closed." );
+ }
+ }
+
+ @Override
+ public ResultSetMetaData getMetaData() throws SQLException {
+ checkNotClosed();
+ return super.getMetaData();
+ }
+
+
@Override
protected void cancel() {
cleanup();
@@ -78,6 +97,7 @@ public class DrillResultSet extends AvaticaResultSet {
@Override
public boolean next() throws SQLException {
+ checkNotClosed();
// Next may be called after close has been called (for example after a user cancel) which in turn
// sets the cursor to null. So we must check before we call next.
// TODO: handle next() after close is called in the Avatica code.
@@ -90,6 +110,7 @@ public class DrillResultSet extends AvaticaResultSet {
@Override
protected DrillResultSet execute() throws SQLException{
+ checkNotClosed();
// Call driver's callback. It is permitted to throw a RuntimeException.
DrillConnectionImpl connection = (DrillConnectionImpl) statement.getConnection();
http://git-wip-us.apache.org/repos/asf/drill/blob/f066786e/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java
index d934c7c..ba265e6 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java
@@ -17,6 +17,9 @@
*/
package org.apache.drill.jdbc;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
import net.hydromatic.avatica.AvaticaStatement;
public abstract class DrillStatement extends AvaticaStatement
@@ -27,11 +30,54 @@ public abstract class DrillStatement extends AvaticaStatement
connection.openStatementsRegistry.addStatement(this);
}
+ /**
+ * Throws AlreadyClosedSqlException if this Statement is closed.
+ *
+ * @throws AlreadyClosedSqlException if Statement is closed
+ * @throws SQLException if error in calling {@link #isClosed()}
+ */
+ private void checkNotClosed() throws SQLException {
+ if ( isClosed() ) {
+ throw new AlreadyClosedSqlException( "Statement is already closed." );
+ }
+ }
+
@Override
public DrillConnectionImpl getConnection() {
return (DrillConnectionImpl) connection;
}
+
+ @Override
+ public boolean execute( String sql ) throws SQLException {
+ checkNotClosed();
+ return super.execute( sql );
+ }
+
+ @Override
+ public ResultSet executeQuery( String sql ) throws SQLException {
+ checkNotClosed();
+ return super.executeQuery( sql );
+ }
+
+ @Override
+ public int executeUpdate( String sql ) throws SQLException {
+ checkNotClosed();
+ return super.executeUpdate( sql );
+ }
+
+ @Override
+ public int executeUpdate( String sql, int[] columnIndexes ) throws SQLException {
+ checkNotClosed();
+ return super.executeUpdate( sql, columnIndexes );
+ }
+
+ @Override
+ public int executeUpdate( String sql, String[] columnNames ) throws SQLException {
+ checkNotClosed();
+ return super.executeUpdate( sql, columnNames );
+ }
+
@Override
public void cleanup() {
final DrillConnectionImpl connection1 = (DrillConnectionImpl) connection;
http://git-wip-us.apache.org/repos/asf/drill/blob/f066786e/exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java
index 7b04371..8d882e9 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java
@@ -23,7 +23,6 @@ import java.sql.ResultSet;
* SQLException for invalid-cursor-state conditions, e.g., calling a column
* accessor method before calling {@link ResultSet#next()} or after
* {@link ResultSet#next()} returns false.
- *
*/
class InvalidCursorStateSqlException extends JdbcApiSqlException {
@@ -55,7 +54,7 @@ class InvalidCursorStateSqlException extends JdbcApiSqlException {
/**
* See {@link JdbcApiSqlException#JdbcApiSqlException()}.
- * */
+ */
public InvalidCursorStateSqlException() {
super();
}
http://git-wip-us.apache.org/repos/asf/drill/blob/f066786e/exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java
index d6b05fb..a7e6d98 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java
@@ -18,9 +18,10 @@
package org.apache.drill.jdbc;
import java.sql.SQLException;
+import java.sql.SQLNonTransientException;
/**
- * SQLException for JDBC API calling sequence/state problems.
+ * SQLException for JDBC API calling-sequence/state problems.
*
* <p>
* {@code JdbcApiSqlException} is intended for errors in using the JDBC API,
@@ -30,8 +31,10 @@ import java.sql.SQLException;
* <p>
* ({@code JdbcApiSqlException} is not for errors that are not under direct
* control of the programmer writing JDBC API calls, for example, invalid SQL
- * syntax, errors from SQL-vs.-data mismatches, data file format errors,
- * resource availability errors, or internal Drill errors.)
+ * syntax errors (which should use {@link SQLSyntaxErrorException}), errors
+ * from SQL-vs.-data mismatches (which likely should use {@link SQLDataException}),
+ * data file format errors, resource availability errors (which might use
+ * {@link SQLTransientException}), or internal Drill errors.)
* </p>
* <p>
* TODO: Consider having a DrillSqlException (in part for reviewing,
@@ -88,7 +91,7 @@ import java.sql.SQLException;
* etc.)
* </p>
*/
-class JdbcApiSqlException extends SQLException {
+class JdbcApiSqlException extends SQLNonTransientException {
private static final long serialVersionUID = 2014_12_12L;