drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From par...@apache.org
Subject [1/2] drill git commit: DRILL-2932: Fix: Error reported via System.out rather than exception message
Date Thu, 07 May 2015 05:36:41 GMT
Repository: drill
Updated Branches:
  refs/heads/master 31e51832d -> 3ba374a53


DRILL-2932: Fix: Error reported via System.out rather than exception message

Main:
- Removed the System.out.println(...) from submissionFailed(...).
- Put UserException's message text in SQLException's message (didn't just wrap).
- Added undoing of extra wrapping by AvaticaStatement.execute...(...).
- Created unit test for execute...(...) exceptions.
- Refined related exception handling (handled cases separately, narrowed
  throws declarations and catches).

JDBC cleanup:
- Renamed ex -> executionFailureException
- Added getNext() method doc. comment.
- Removed some obsolete alignment spaces.

Added "review this" TODOs re DRILL-2933 (probably-obsolete SchemaChangeException):
- DrillCursor;
- MergingRecordBatch, ParquetResultListener, PrintingResultsListener, QueryWrapper, RecordBatchLoader,
UnorderedReceiverBatch;
- TestDrillbitResilience, TestTextColumn, BaseTestQuery, DrillTestWrapper.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/c4d864d7
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/c4d864d7
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/c4d864d7

Branch: refs/heads/master
Commit: c4d864d7c84dbb1fb9e2cff8da0bdf9899a0720b
Parents: 31e5183
Author: dbarclay <dbarclay@maprtech.com>
Authored: Thu Apr 30 23:34:03 2015 -0700
Committer: Parth Chandra <parthc@apache.org>
Committed: Wed May 6 21:41:19 2015 -0700

----------------------------------------------------------------------
 .../exec/client/PrintingResultsListener.java    |   2 +
 .../impl/mergereceiver/MergingRecordBatch.java  |   6 +
 .../UnorderedReceiverBatch.java                 |   2 +
 .../drill/exec/record/RecordBatchLoader.java    |   5 +-
 .../drill/exec/rpc/user/QueryResultHandler.java |   2 +-
 .../drill/exec/server/rest/QueryWrapper.java    |   2 +
 .../java/org/apache/drill/BaseTestQuery.java    |   2 +
 .../java/org/apache/drill/DrillTestWrapper.java |   4 +
 .../exec/server/TestDrillbitResilience.java     |   2 +
 .../store/parquet/ParquetResultListener.java    |   2 +
 .../drill/exec/store/text/TestTextColumn.java   |   2 +
 .../java/org/apache/drill/jdbc/DrillCursor.java |  36 +++-
 .../org/apache/drill/jdbc/DrillStatement.java   |  38 +++-
 .../drill/jdbc/impl/DrillResultSetImpl.java     |  23 ++-
 .../test/TestExecutionExceptionsToClient.java   | 186 +++++++++++++++++++
 15 files changed, 295 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java
index 64e7266..875160b 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java
@@ -76,6 +76,8 @@ public class PrintingResultsListener implements UserResultsListener {
       count.addAndGet(header.getRowCount());
       try {
         loader.load(header.getDef(), data);
+        // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+        // SchemaChangeException, so check/clean catch clause below.
       } catch (SchemaChangeException e) {
         submissionFailed(UserException.systemError(e).build());
       }

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
index c36b0d3..ce683cb 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
@@ -243,6 +243,8 @@ public class MergingRecordBatch extends AbstractRecordBatch<MergingReceiverPOP>
         final UserBitShared.RecordBatchDef rbd = batch.getHeader().getDef();
         try {
           batchLoaders[i].load(rbd, batch.getBody());
+          // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+          // SchemaChangeException, so check/clean catch clause below.
         } catch(final SchemaChangeException e) {
           logger.error("MergingReceiver failed to load record batch from remote host.  {}",
e);
           context.fail(e);
@@ -313,6 +315,8 @@ public class MergingRecordBatch extends AbstractRecordBatch<MergingReceiverPOP>
             incomingBatches[b] = batch;
             if (batch != null) {
               batchLoaders[b].load(batch.getHeader().getDef(), batch.getBody());
+              // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+              // SchemaChangeException, so check/clean catch clause below.
             } else {
               batchLoaders[b].clear();
               batchLoaders[b] = null;
@@ -399,6 +403,8 @@ public class MergingRecordBatch extends AbstractRecordBatch<MergingReceiverPOP>
         final UserBitShared.RecordBatchDef rbd = incomingBatches[node.batchId].getHeader().getDef();
         try {
           batchLoaders[node.batchId].load(rbd, incomingBatches[node.batchId].getBody());
+          // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+          // SchemaChangeException, so check/clean catch clause below.
         } catch(final SchemaChangeException ex) {
           context.fail(ex);
           return IterOutcome.STOP;

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java
index 09cb7ad..c9d9c11 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java
@@ -169,6 +169,8 @@ public class UnorderedReceiverBatch implements CloseableRecordBatch {
 
       final RecordBatchDef rbd = batch.getHeader().getDef();
       final boolean schemaChanged = batchLoader.load(rbd, batch.getBody());
+      // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+      // SchemaChangeException, so check/clean catch clause below.
       stats.addLongStat(Metric.BYTES_RECEIVED, batch.getByteCount());
 
       batch.release();

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
index 088630c..1b8b7ce 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
@@ -54,9 +54,10 @@ public class RecordBatchLoader implements VectorAccessible, Iterable<VectorWrapp
    * @param def
    *          The definition for the record batch.
    * @param buf
-   *          The buffer that holds the data associated with the record batch
-   * @return Whether or not the schema changed since the previous load.
+   *          The buffer that holds the data associated with the record batch.
+   * @return Whether the schema changed since the previous load.
    * @throws SchemaChangeException
+   *   TODO:  Clean:  DRILL-2933  load(...) never actually throws SchemaChangeException.
    */
   public boolean load(RecordBatchDef def, DrillBuf buf) throws SchemaChangeException {
 //    logger.debug("Loading record batch with def {} and data {}", def, buf);

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
index 3beae89..5e3e937 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
@@ -81,7 +81,7 @@ public class QueryResultHandler {
 
     assert queryResult.hasQueryState() : "received query result without QueryState";
 
-    final boolean isFailureResult    = QueryState.FAILED    == queryState;
+    final boolean isFailureResult = QueryState.FAILED == queryState;
     // CANCELED queries are handled the same way as COMPLETED
     final boolean isTerminalResult;
     switch ( queryState ) {

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java
index aa43aa9..4629dd0 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java
@@ -148,6 +148,8 @@ public class QueryWrapper {
         if (result.hasData()) {
           final RecordBatchLoader loader = new RecordBatchLoader(allocator);
           loader.load(result.getHeader().getDef(), result.getData());
+          // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+          // SchemaChangeException, so check/clean catch clause below.
           for (int i = 0; i < loader.getSchema().getFieldCount(); ++i) {
             columns.add(loader.getSchema().getColumn(i).getPath().getAsUnescapedPath());
           }

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
index d6e6d08..200bbc6 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
@@ -423,6 +423,8 @@ public class BaseTestQuery extends ExecTest {
     for(QueryDataBatch result : results) {
       rowCount += result.getHeader().getRowCount();
       loader.load(result.getHeader().getDef(), result.getData());
+      // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+      // SchemaChangeException, so check/clean throw clause above.
       if (loader.getRecordCount() <= 0) {
         continue;
       }

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java b/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java
index 856dfa5..d4e7ed6 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java
@@ -225,6 +225,8 @@ public class DrillTestWrapper {
     for (int i = 0; i < size; i++) {
       batch = records.get(0);
       loader.load(batch.getHeader().getDef(), batch.getData());
+      // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+      // SchemaChangeException, so check/clean throws clause above.
       if (schema == null) {
         schema = loader.getSchema();
         for (MaterializedField mf : schema) {
@@ -410,6 +412,8 @@ public class DrillTestWrapper {
     for (int i = 0; i < size; i++) {
       batch = records.get(0);
       loader.load(batch.getHeader().getDef(), batch.getData());
+      // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+      // SchemaChangeException, so check/clean throws clause above.
       if (schema == null) {
         schema = loader.getSchema();
       }

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java
index 2698701..da69e9e 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java
@@ -208,6 +208,8 @@ public class TestDrillbitResilience {
             final QueryData queryData = queryResultBatch.getHeader();
             try {
               loader.load(queryData.getDef(), queryResultBatch.getData());
+              // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+              // SchemaChangeException, so check/clean catch clause below.
             } catch (final SchemaChangeException e) {
               fail(e.toString());
             }

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java
index 0e80f91..6326478 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java
@@ -109,6 +109,8 @@ public class ParquetResultListener implements UserResultsListener {
     RecordBatchLoader batchLoader = new RecordBatchLoader(allocator);
     try {
       schemaChanged = batchLoader.load(result.getHeader().getDef(), result.getData());
+      // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+      // SchemaChangeException, so check/clean catch clause below.
     } catch (SchemaChangeException e) {
       throw new RuntimeException(e);
     }

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestTextColumn.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestTextColumn.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestTextColumn.java
index 3c1a38a..f07cf3b 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestTextColumn.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestTextColumn.java
@@ -75,6 +75,8 @@ public class TestTextColumn extends BaseTestQuery{
       int rows = batch.getHeader().getRowCount();
       if(batch.getData() != null) {
         loader.load(batch.getHeader().getDef(), batch.getData());
+        // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
+        // SchemaChangeException, so check/clean throws clause above.
         for (int i = 0; i < rows; ++i) {
           output.add(new ArrayList<String>());
           for (VectorWrapper<?> vw: loader) {

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java
index 6bad3ce..30c85eb 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java
@@ -25,6 +25,9 @@ import net.hydromatic.avatica.ArrayImpl.Factory;
 import net.hydromatic.avatica.ColumnMetaData;
 import net.hydromatic.avatica.Cursor;
 
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.exception.SchemaChangeException;
 import org.apache.drill.exec.record.BatchSchema;
 import org.apache.drill.exec.record.RecordBatchLoader;
 import org.apache.drill.exec.rpc.user.QueryDataBatch;
@@ -132,8 +135,13 @@ public class DrillCursor implements Cursor {
           return false;
         } else {
           currentRecordNumber = 0;
-          boolean changed = currentBatch.load(qrb.getHeader().getDef(), qrb.getData());
-          qrb.release();
+          final boolean changed;
+          try {
+            changed = currentBatch.load(qrb.getHeader().getDef(), qrb.getData());
+          }
+          finally {
+            qrb.release();
+          }
           schema = currentBatch.getSchema();
           if (changed) {
             updateColumns();
@@ -143,8 +151,28 @@ public class DrillCursor implements Cursor {
           }
           return true;
         }
-      } catch (Exception e) {
-        throw new SQLException("Failure while executing query.", e);
+      }
+      catch ( UserException e ) {
+        // A normally expected case--for any server-side error (e.g., syntax
+        // error in SQL statement).
+        // Contruct SQLException with message text from the UserException.
+        // TODO:  Map UserException error type to SQLException subclass (once
+        // error type is accessible, of course. :-( )
+        throw new SQLException( e.getMessage(), e );
+      }
+      catch ( InterruptedException e ) {
+        // Not normally expected--Drill doesn't interrupt in this area (right?)--
+        // but JDBC client certainly could.
+        throw new SQLException( "Interrupted.", e );
+      }
+      catch ( SchemaChangeException e ) {
+        // TODO:  Clean:  DRILL-2933:  RecordBatchLoader.load(...) no longer
+        // throws SchemaChangeException, so check/clean catch clause.
+        throw new SQLException(
+            "Unexpected SchemaChangeException from RecordBatchLoader.load(...)" );
+      }
+      catch ( RuntimeException e ) {
+        throw new SQLException( "Unexpected exception: " + e.toString(), e );
       }
 
     }

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/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 7fc79be..a609bb1 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
@@ -47,23 +47,53 @@ public abstract class DrillStatement extends AvaticaStatement
     return (DrillConnectionImpl) connection;
   }
 
+  // WORKAROUND:  Work around AvaticaStatement's code that wraps _any_ exception,
+  // even if SQLException, by unwrapping to get cause exception so caller can
+  // throw it directly if it's a SQLException:
+  // TODO:  Any ideas for a better name?
+  private SQLException unwrapIfExtra( final SQLException superMethodException ) {
+    final SQLException result;
+    final Throwable cause = superMethodException.getCause();
+    if ( null != cause && cause instanceof SQLException ) {
+      result = (SQLException) cause;
+    }
+    else {
+      result = superMethodException;
+    }
+    return result;
+  }
 
   @Override
   public boolean execute( String sql ) throws SQLException {
     checkNotClosed();
-    return super.execute( sql );
+    try {
+      return super.execute( sql );
+    }
+    catch ( final SQLException possiblyExtraWrapperException ) {
+      throw unwrapIfExtra( possiblyExtraWrapperException );
+    }
   }
 
   @Override
   public ResultSet executeQuery( String sql ) throws SQLException {
-    checkNotClosed();
-    return super.executeQuery( sql );
+    try {
+       checkNotClosed();
+       return super.executeQuery( sql );
+    }
+    catch ( final SQLException possiblyExtraWrapperException ) {
+      throw unwrapIfExtra( possiblyExtraWrapperException );
+    }
   }
 
   @Override
   public int executeUpdate( String sql ) throws SQLException {
     checkNotClosed();
-    return super.executeUpdate( sql );
+    try {
+      return super.executeUpdate( sql );
+    }
+    catch ( final SQLException possiblyExtraWrapperException ) {
+      throw unwrapIfExtra( possiblyExtraWrapperException );
+    }
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
index a7cc0c1..8ef2af3 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
@@ -155,12 +155,12 @@ public class DrillResultSetImpl extends AvaticaResultSet implements
DrillResultS
       // TODO:  Revisit:  Why reaching directly into ResultsListener rather than
       // calling some wait method?
       resultsListener.latch.await();
-      cursor.next();
-    } catch (InterruptedException e) {
-      // TODO:  Check:  Should this call Thread.currentThread.interrupt()?   If
-      // not, at least document why this is empty.
-      // TODO:  Check:  Does anything ever interrupt this?  (Is catch needed?)
+    } catch ( InterruptedException e ) {
+      // Not normally expected--Drill doesn't interrupt in this area (right?)--
+      // but JDBC client certainly could.
+      throw new SQLException( "Interrupted", e );
     }
+    cursor.next();
 
     return this;
   }
@@ -181,7 +181,6 @@ public class DrillResultSetImpl extends AvaticaResultSet implements DrillResultS
 
     private volatile QueryId queryId;
 
-
     private volatile UserException executionFailureException;
     volatile boolean completed = false;
     private volatile boolean autoread = true;
@@ -259,8 +258,16 @@ public class DrillResultSetImpl extends AvaticaResultSet implements DrillResultS
     }
 
 
-    // TODO:  Doc.:  Specify whether result can be null and what that means.
-    public QueryDataBatch getNext() throws Exception {
+    /**
+     * Gets the next batch of query results from the queue.
+     * @return  the next batch, or {@code null} after last batch has been returned
+     * @throws UserException
+     *         if the query failed
+     * @throws InterruptedException
+     *         if waiting on the queue was interrupted
+     */
+    public QueryDataBatch getNext() throws UserException,
+                                           InterruptedException {
       while (true) {
         if (executionFailureException != null) {
           logger.debug( "Dequeued query failure exception: {}.", executionFailureException
);

http://git-wip-us.apache.org/repos/asf/drill/blob/c4d864d7/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestExecutionExceptionsToClient.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestExecutionExceptionsToClient.java
b/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestExecutionExceptionsToClient.java
new file mode 100644
index 0000000..d845a38
--- /dev/null
+++ b/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestExecutionExceptionsToClient.java
@@ -0,0 +1,186 @@
+/**
+ * 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.test;
+
+import static org.junit.Assert.fail;
+import static org.junit.Assert.assertThat;
+import static org.hamcrest.CoreMatchers.*;
+
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.exceptions.UserRemoteException;
+import org.apache.drill.jdbc.Driver;
+import org.apache.drill.jdbc.JdbcTestBase;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+
+public class TestExecutionExceptionsToClient extends JdbcTestBase {
+
+  private static Connection connection;
+
+  @BeforeClass
+  public static void setUpConnection() throws Exception {
+    connection = new Driver().connect( "jdbc:drill:zk=local", null );
+  }
+
+  @AfterClass
+  public static void tearDownConnection() throws SQLException {
+    connection.close();
+  }
+
+  @Test
+  public void testExecuteQueryThrowsRight1() throws Exception {
+    final Statement statement = connection.createStatement();
+    try {
+      statement.executeQuery( "SELECT one case of syntax error" );
+    }
+    catch ( SQLException e ) {
+      assertThat( "Null getCause(); missing expected wrapped exception",
+                  e.getCause(), notNullValue() );
+
+      assertThat( "Unexpectedly wrapped another SQLException",
+                  e.getCause(), not( instanceOf( SQLException.class ) ) );
+
+      assertThat( "getCause() not UserRemoteException as expected",
+                  e.getCause(), instanceOf( UserRemoteException.class ) );
+
+      assertThat( "No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+                  e.getMessage(), anyOf( startsWith( "SYSTEM ERROR" ),
+                                         startsWith( "PARSE ERROR" ) ) );
+    }
+  }
+
+  @Test
+  public void testExecuteThrowsRight1() throws Exception {
+    final Statement statement = connection.createStatement();
+    try {
+      statement.execute( "SELECT one case of syntax error" );
+    }
+    catch ( SQLException e ) {
+      assertThat( "Null getCause(); missing expected wrapped exception",
+                  e.getCause(), notNullValue() );
+
+      assertThat( "Unexpectedly wrapped another SQLException",
+                  e.getCause(), not( instanceOf( SQLException.class ) ) );
+
+      assertThat( "getCause() not UserRemoteException as expected",
+                  e.getCause(), instanceOf( UserRemoteException.class ) );
+
+      assertThat( "No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+                  e.getMessage(), anyOf( startsWith( "SYSTEM ERROR" ),
+                                         startsWith( "PARSE ERROR" ) ) );
+    }
+  }
+
+  @Test
+  public void testExecuteUpdateThrowsRight1() throws Exception {
+    final Statement statement = connection.createStatement();
+    try {
+      statement.executeUpdate( "SELECT one case of syntax error" );
+    }
+    catch ( SQLException e ) {
+      assertThat( "Null getCause(); missing expected wrapped exception",
+                  e.getCause(), notNullValue() );
+
+      assertThat( "Unexpectedly wrapped another SQLException",
+                  e.getCause(), not( instanceOf( SQLException.class ) ) );
+
+      assertThat( "getCause() not UserRemoteException as expected",
+                  e.getCause(), instanceOf( UserRemoteException.class ) );
+
+      assertThat( "No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+                  e.getMessage(), anyOf( startsWith( "SYSTEM ERROR" ),
+                                         startsWith( "PARSE ERROR" ) ) );
+    }
+  }
+
+  @Test
+  public void testExecuteQueryThrowsRight2() throws Exception {
+    final Statement statement = connection.createStatement();
+    try {
+      statement.executeQuery( "BAD QUERY 1" );
+    }
+    catch ( SQLException e ) {
+      assertThat( "Null getCause(); missing expected wrapped exception",
+                  e.getCause(), notNullValue() );
+
+      assertThat( "Unexpectedly wrapped another SQLException",
+                  e.getCause(), not( instanceOf( SQLException.class ) ) );
+
+      assertThat( "getCause() not UserRemoteException as expected",
+                  e.getCause(), instanceOf( UserRemoteException.class ) );
+
+      assertThat( "No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+                  e.getMessage(), anyOf( startsWith( "SYSTEM ERROR" ),
+                                         startsWith( "PARSE ERROR" ) ) );
+    }
+  }
+
+  @Test
+  public void testExecuteThrowsRight2() throws Exception {
+    final Statement statement = connection.createStatement();
+    try {
+      statement.execute( "worse query 2" );
+    }
+    catch ( SQLException e ) {
+      assertThat( "Null getCause(); missing expected wrapped exception",
+                  e.getCause(), notNullValue() );
+
+      assertThat( "Unexpectedly wrapped another SQLException",
+                  e.getCause(), not( instanceOf( SQLException.class ) ) );
+
+      assertThat( "getCause() not UserRemoteException as expected",
+                  e.getCause(), instanceOf( UserRemoteException.class ) );
+
+      assertThat( "No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+                  e.getMessage(), anyOf( startsWith( "SYSTEM ERROR" ),
+                                         startsWith( "PARSE ERROR" ) ) );
+    }
+  }
+
+  @Test
+  public void testExecuteUpdateThrowsRight2() throws Exception {
+    final Statement statement = connection.createStatement();
+    try {
+      statement.executeUpdate( "naughty, naughty query 3" );
+    }
+    catch ( SQLException e ) {
+      assertThat( "Null getCause(); missing expected wrapped exception",
+                  e.getCause(), notNullValue() );
+
+      assertThat( "Unexpectedly wrapped another SQLException",
+                  e.getCause(), not( instanceOf( SQLException.class ) ) );
+
+      assertThat( "getCause() not UserRemoteException as expected",
+                  e.getCause(), instanceOf( UserRemoteException.class ) );
+
+      assertThat( "No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+                  e.getMessage(), anyOf( startsWith( "SYSTEM ERROR" ),
+                                         startsWith( "PARSE ERROR" ) ) );
+    }
+  }
+
+}


Mime
View raw message