drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From par...@apache.org
Subject [5/9] drill git commit: DRILL-2565: Add some key "already-closed" checks, with test for future DRILL-2489 work.
Date Tue, 07 Apr 2015 02:14:59 GMT
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 <dbarclay@maprtech.com>
Authored: Tue Mar 24 22:58:33 2015 -0700
Committer: Parth Chandra <pchandra@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;
 


Mime
View raw message