drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sudhe...@apache.org
Subject [13/14] drill git commit: DRILL-4938: Report UserException when constant expression reduction fails
Date Tue, 13 Dec 2016 00:40:10 GMT
DRILL-4938: Report UserException when constant expression reduction fails

closes #689


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

Branch: refs/heads/master
Commit: cf2b7c70ea47e604364ccb5e8076bbd7eae310c3
Parents: 97da019
Author: Serhii-Harnyk <serhii.harnyk@gmail.com>
Authored: Fri Nov 18 16:02:04 2016 +0000
Committer: Sudheesh Katkam <sudheesh@apache.org>
Committed: Mon Dec 12 15:59:59 2016 -0800

----------------------------------------------------------------------
 .../planner/logical/DrillConstExecutor.java     |  13 +-
 .../test/TestExecutionExceptionsToClient.java   | 177 ++++++++++---------
 2 files changed, 100 insertions(+), 90 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/cf2b7c70/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java
index 829db2a..4a94c71 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java
@@ -20,7 +20,7 @@ package org.apache.drill.exec.planner.logical;
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
 import io.netty.buffer.DrillBuf;
-import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.common.expression.ErrorCollectorImpl;
 import org.apache.drill.common.expression.ExpressionStringBuilder;
 import org.apache.drill.common.expression.LogicalExpression;
@@ -68,7 +68,6 @@ import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rex.RexBuilder;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.sql.type.SqlTypeName;
-import org.apache.calcite.util.NlsString;
 import org.apache.drill.exec.planner.physical.PlannerSettings;
 import org.apache.drill.exec.planner.sql.TypeInferenceUtils;
 import org.joda.time.DateTime;
@@ -129,8 +128,9 @@ public class DrillConstExecutor implements RelOptPlanner.Executor {
         String message = String.format(
             "Failure while materializing expression in constant expression evaluator [%s].
 Errors: %s",
             newCall.toString(), errors.toString());
-        logger.error(message);
-        throw new DrillRuntimeException(message);
+        throw UserException.planError()
+          .message(message)
+          .build(logger);
       }
 
       if (NON_REDUCIBLE_TYPES.contains(materializedExpr.getMajorType().getMinorType())) {
@@ -149,8 +149,9 @@ public class DrillConstExecutor implements RelOptPlanner.Executor {
         if (sqlTypeName == null) {
           String message = String.format("Error reducing constant expression, unsupported
type: %s.",
               materializedExpr.getMajorType().getMinorType());
-          logger.error(message);
-          throw new DrillRuntimeException(message);
+          throw UserException.unsupportedError()
+            .message(message)
+            .build(logger);
         }
         reducedValues.add(rexBuilder.makeNullLiteral(sqlTypeName));
         continue;

http://git-wip-us.apache.org/repos/asf/drill/blob/cf2b7c70/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
index d845a38..749312f 100644
--- 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
@@ -17,26 +17,20 @@
  */
 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;
@@ -51,136 +45,151 @@ public class TestExecutionExceptionsToClient extends JdbcTestBase {
     connection.close();
   }
 
-  @Test
+  @Test(expected = SQLException.class)
   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() );
+      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("Unexpectedly wrapped another SQLException",
+        e.getCause(), not(instanceOf(SQLException.class)));
 
-      assertThat( "getCause() not UserRemoteException as expected",
-                  e.getCause(), instanceOf( UserRemoteException.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" ) ) );
+      assertThat("No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+        e.getMessage(), anyOf(startsWith("SYSTEM ERROR"), startsWith("PARSE ERROR")));
+      throw e;
     }
   }
 
-  @Test
+  @Test(expected = SQLException.class)
   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() );
+      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("Unexpectedly wrapped another SQLException",
+        e.getCause(), not(instanceOf(SQLException.class)));
 
-      assertThat( "getCause() not UserRemoteException as expected",
-                  e.getCause(), instanceOf( UserRemoteException.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" ) ) );
+      assertThat("No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+        e.getMessage(), anyOf(startsWith("SYSTEM ERROR"), startsWith("PARSE ERROR")));
+      throw e;
     }
   }
 
-  @Test
+  @Test(expected = SQLException.class)
   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() );
+      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("Unexpectedly wrapped another SQLException",
+        e.getCause(), not(instanceOf(SQLException.class)));
 
-      assertThat( "getCause() not UserRemoteException as expected",
-                  e.getCause(), instanceOf( UserRemoteException.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" ) ) );
+      assertThat("No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+        e.getMessage(), anyOf(startsWith("SYSTEM ERROR"), startsWith("PARSE ERROR")));
+      throw e;
     }
   }
 
-  @Test
+  @Test(expected = SQLException.class)
   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() );
+      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("Unexpectedly wrapped another SQLException",
+        e.getCause(), not(instanceOf(SQLException.class)));
 
-      assertThat( "getCause() not UserRemoteException as expected",
-                  e.getCause(), instanceOf( UserRemoteException.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" ) ) );
+      assertThat("No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+        e.getMessage(), anyOf(startsWith("SYSTEM ERROR"), startsWith("PARSE ERROR")));
+      throw e;
     }
   }
 
-  @Test
+  @Test(expected = SQLException.class)
   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() );
+      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("Unexpectedly wrapped another SQLException",
+        e.getCause(), not(instanceOf(SQLException.class)));
 
-      assertThat( "getCause() not UserRemoteException as expected",
-                  e.getCause(), instanceOf( UserRemoteException.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" ) ) );
+      assertThat("No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+        e.getMessage(), anyOf(startsWith("SYSTEM ERROR"), startsWith("PARSE ERROR")));
+      throw e;
     }
   }
 
-  @Test
+  @Test(expected = SQLException.class)
   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() );
+      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("Unexpectedly wrapped another SQLException",
+        e.getCause(), not(instanceOf(SQLException.class)));
 
-      assertThat( "getCause() not UserRemoteException as expected",
-                  e.getCause(), instanceOf( UserRemoteException.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" ) ) );
+      assertThat("No expected current \"SYSTEM ERROR\"/eventual \"PARSE ERROR\"",
+        e.getMessage(), anyOf(startsWith("SYSTEM ERROR"), startsWith("PARSE ERROR")));
+      throw e;
     }
   }
 
+  @Test(expected = SQLException.class)
+  public void testMaterializingError() throws Exception {
+    final Statement statement = connection.createStatement();
+    try {
+      statement.executeUpdate("select (res1 = 2016/09/22) res2 from (select (case when (false)
then null else "
+        + "cast('2016/09/22' as date) end) res1 from (values(1)) foo) foobar");
+    } 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 \"PLAN ERROR\"",
+        e.getMessage(), startsWith("PLAN ERROR"));
+      throw e;
+    }
+  }
 }


Mime
View raw message