You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2021/04/27 02:40:31 UTC

[calcite] 01/01: [CALCITE-4591] RelRunner should throw SQLException if prepare fails

This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit de847c38f3544f9c7282984f32dc1093bdb2fb60
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Mon Apr 26 12:04:00 2021 -0700

    [CALCITE-4591] RelRunner should throw SQLException if prepare fails
---
 .../apache/calcite/jdbc/CalciteConnectionImpl.java | 28 +++++++++++++++-------
 .../java/org/apache/calcite/tools/RelRunner.java   | 12 +++++++++-
 .../java/org/apache/calcite/tools/RelRunners.java  |  5 ++--
 .../apache/calcite/test/ExceptionMessageTest.java  | 11 +++++----
 .../apache/calcite/test/ExtensionDdlExecutor.java  |  5 ++--
 .../org/apache/calcite/test/RelBuilderTest.java    |  2 +-
 .../org/apache/calcite/tools/FrameworksTest.java   |  6 ++---
 .../calcite/adapter/elasticsearch/MatchTest.java   |  3 +--
 .../apache/calcite/server/ServerDdlExecutor.java   |  5 ++--
 9 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/jdbc/CalciteConnectionImpl.java b/core/src/main/java/org/apache/calcite/jdbc/CalciteConnectionImpl.java
index 9d785d5..147602a 100644
--- a/core/src/main/java/org/apache/calcite/jdbc/CalciteConnectionImpl.java
+++ b/core/src/main/java/org/apache/calcite/jdbc/CalciteConnectionImpl.java
@@ -46,6 +46,7 @@ import org.apache.calcite.materialize.Lattice;
 import org.apache.calcite.materialize.MaterializationService;
 import org.apache.calcite.plan.RelOptUtil;
 import org.apache.calcite.prepare.CalciteCatalogReader;
+import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.type.DelegatingTypeSystem;
 import org.apache.calcite.rel.type.RelDataTypeSystem;
 import org.apache.calcite.runtime.Hook;
@@ -74,6 +75,7 @@ import com.google.common.collect.ImmutableMap;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 import java.lang.reflect.Type;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.HashMap;
@@ -94,7 +96,7 @@ import static java.util.Objects.requireNonNull;
  * Implementation of JDBC connection
  * in the Calcite engine.
  *
- * <p>Abstract to allow newer versions of JDBC to add methods.</p>
+ * <p>Abstract to allow newer versions of JDBC to add methods.
  */
 abstract class CalciteConnectionImpl
     extends AvaticaConnection
@@ -111,7 +113,7 @@ abstract class CalciteConnectionImpl
   /**
    * Creates a CalciteConnectionImpl.
    *
-   * <p>Not public; method is called only from the driver.</p>
+   * <p>Not public; method is called only from the driver.
    *
    * @param driver Driver
    * @param factory Factory for JDBC objects
@@ -180,13 +182,21 @@ abstract class CalciteConnectionImpl
 
   @Override public <T> T unwrap(Class<T> iface) throws SQLException {
     if (iface == RelRunner.class) {
-      return iface.cast((RelRunner) rel -> {
-        try {
-          return prepareStatement_(CalcitePrepare.Query.of(rel),
-              ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY,
-              getHoldability());
-        } catch (SQLException e) {
-          throw new RuntimeException(e);
+      return iface.cast(new RelRunner() {
+        @Override public PreparedStatement prepareStatement(RelNode rel)
+            throws SQLException {
+            return prepareStatement_(CalcitePrepare.Query.of(rel),
+                ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY,
+                getHoldability());
+        }
+
+        @SuppressWarnings("deprecation")
+        @Override public PreparedStatement prepare(RelNode rel) {
+          try {
+            return prepareStatement(rel);
+          } catch (SQLException e) {
+            throw Util.throwAsRuntime(e);
+          }
         }
       });
     }
diff --git a/core/src/main/java/org/apache/calcite/tools/RelRunner.java b/core/src/main/java/org/apache/calcite/tools/RelRunner.java
index e7f1d69..b1dad24 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelRunner.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelRunner.java
@@ -19,6 +19,7 @@ package org.apache.calcite.tools;
 import org.apache.calcite.rel.RelNode;
 
 import java.sql.PreparedStatement;
+import java.sql.SQLException;
 
 /**
  * Runs a relational expression.
@@ -28,6 +29,15 @@ import java.sql.PreparedStatement;
  * @see RelRunners
  */
 public interface RelRunner {
-  /** Runs a relational expression. */
+  /** Prepares a statement based on a relational expression. */
+  @Deprecated // to be removed before 1.28
   PreparedStatement prepare(RelNode rel);
+
+  /** Prepares a statement based on a relational expression.
+   *
+   * @param rel Relational expression
+   * @throws SQLException on error */
+  default PreparedStatement prepareStatement(RelNode rel) throws SQLException {
+    return prepare(rel);
+  }
 }
diff --git a/core/src/main/java/org/apache/calcite/tools/RelRunners.java b/core/src/main/java/org/apache/calcite/tools/RelRunners.java
index 27e1f19..e89dae5 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelRunners.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelRunners.java
@@ -23,6 +23,7 @@ import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.RelShuttle;
 import org.apache.calcite.rel.core.TableScan;
 import org.apache.calcite.rel.logical.LogicalTableScan;
+import org.apache.calcite.util.Util;
 
 import java.sql.Connection;
 import java.sql.DriverManager;
@@ -50,9 +51,9 @@ public class RelRunners {
     rel = rel.accept(shuttle);
     try (Connection connection = DriverManager.getConnection("jdbc:calcite:")) {
       final RelRunner runner = connection.unwrap(RelRunner.class);
-      return runner.prepare(rel);
+      return runner.prepareStatement(rel);
     } catch (SQLException e) {
-      throw new RuntimeException(e);
+      throw Util.throwAsRuntime(e);
     }
   }
 }
diff --git a/core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java b/core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
index 590cbd1..31b09ba 100644
--- a/core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
+++ b/core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
@@ -120,14 +120,15 @@ public class ExceptionMessageTest {
     final FrameworkConfig config = Frameworks.newConfigBuilder()
         .defaultSchema(rootSchema)
         .build();
-    RelBuilder relBuilder = RelBuilder.create(config);
+    final RelBuilder relBuilder = RelBuilder.create(config);
     return fn.apply(relBuilder);
   }
 
   private void runQuery(Function<RelBuilder, RelNode> relFn) throws SQLException {
     final RelRunner relRunner = conn.unwrap(RelRunner.class);
     final RelNode relNode = withRelBuilder(relFn);
-    PreparedStatement preparedStatement = relRunner.prepare(relNode);
+    final PreparedStatement preparedStatement =
+        relRunner.prepareStatement(relNode);
     try {
       preparedStatement.executeQuery();
     } finally {
@@ -203,7 +204,7 @@ public class ExceptionMessageTest {
    * <a href="https://issues.apache.org/jira/browse/CALCITE-4585">[CALCITE-4585]
    * If a query is executed via RelRunner.prepare(RelNode) and fails, the
    * exception should report the RelNode plan, not the SQL</a>. */
-  @Test void testRelNodeQueryException() throws SQLException {
+  @Test void testRelNodeQueryException() {
     try {
       final Function<RelBuilder, RelNode> relFn = b ->
           b.scan("test", "entries")
@@ -211,8 +212,8 @@ public class ExceptionMessageTest {
               .build();
       runQuery(relFn);
       fail("RelNode query about entries should result in an exception");
-    } catch (RuntimeException e) {
-      String message = "java.sql.SQLException: Error while preparing plan ["
+    } catch (SQLException e) {
+      String message = "Error while preparing plan ["
           + "LogicalProject($f0=[ABS($1)])\n"
           + "  LogicalTableScan(table=[[test, entries]])\n"
           + "]";
diff --git a/core/src/test/java/org/apache/calcite/test/ExtensionDdlExecutor.java b/core/src/test/java/org/apache/calcite/test/ExtensionDdlExecutor.java
index 7170c6e..22a7707 100644
--- a/core/src/test/java/org/apache/calcite/test/ExtensionDdlExecutor.java
+++ b/core/src/test/java/org/apache/calcite/test/ExtensionDdlExecutor.java
@@ -164,13 +164,14 @@ public class ExtensionDdlExecutor extends DdlExecutorImpl {
       final SqlNode query1 = planner.parse(sql);
       final SqlNode query2 = planner.validate(query1);
       final RelRoot r = planner.rel(query2);
-      final PreparedStatement prepare = context.getRelRunner().prepare(r.rel);
+      final PreparedStatement prepare =
+          context.getRelRunner().prepareStatement(r.rel);
       int rowCount = prepare.executeUpdate();
       Util.discard(rowCount);
       prepare.close();
     } catch (SqlParseException | ValidationException
         | RelConversionException | SQLException e) {
-      throw new RuntimeException(e);
+      throw Util.throwAsRuntime(e);
     }
   }
 
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index b2995c6..1d062f4 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -3770,7 +3770,7 @@ public class RelBuilderTest {
 
       int count = 0;
       try (PreparedStatement statement =
-               connection.unwrap(RelRunner.class).prepare(node);
+               connection.unwrap(RelRunner.class).prepareStatement(node);
            ResultSet resultSet = statement.executeQuery()) {
         while (resultSet.next()) {
           count++;
diff --git a/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java b/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java
index 601ea37..dbfb2d0 100644
--- a/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java
+++ b/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java
@@ -359,16 +359,16 @@ public class FrameworksTest {
             // everything works fine. JdbcValues is never instantiated in any
             // of the 3 queries.
             if (false) {
-              runner.prepare(values).executeQuery();
+              runner.prepareStatement(values).executeQuery();
             }
 
             final RelNode scan = builder.scan("JDBC_SCOTT", "EMP").build();
-            runner.prepare(scan).executeQuery();
+            runner.prepareStatement(scan).executeQuery();
             builder.clear();
 
             // running this after the scott query causes the exception
             RelRunner runner2 = connection.unwrap(RelRunner.class);
-            runner2.prepare(values).executeQuery();
+            runner2.prepareStatement(values).executeQuery();
           } catch (Exception e) {
             throw TestUtil.rethrow(e);
           }
diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/MatchTest.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/MatchTest.java
index 42b3f98..fbf126a 100644
--- a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/MatchTest.java
+++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/MatchTest.java
@@ -208,8 +208,7 @@ class MatchTest {
     RelNode root = builder.build();
 
     RelRunner ru = (RelRunner) con.unwrap(Class.forName("org.apache.calcite.tools.RelRunner"));
-    try (PreparedStatement preparedStatement = ru.prepare(root)) {
-
+    try (PreparedStatement preparedStatement = ru.prepareStatement(root)) {
       String s = CalciteAssert.toString(preparedStatement.executeQuery());
       final String result = ""
           + "_MAP={id=02154, city=NORTH WALTHAM, loc=[-71.236497, 42.382492], "
diff --git a/server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java b/server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java
index 51c5b5f..6a0d1a0 100644
--- a/server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java
+++ b/server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java
@@ -202,13 +202,14 @@ public class ServerDdlExecutor extends DdlExecutorImpl {
       final SqlNode query1 = planner.parse(sql);
       final SqlNode query2 = planner.validate(query1);
       final RelRoot r = planner.rel(query2);
-      final PreparedStatement prepare = context.getRelRunner().prepare(r.rel);
+      final PreparedStatement prepare =
+          context.getRelRunner().prepareStatement(r.rel);
       int rowCount = prepare.executeUpdate();
       Util.discard(rowCount);
       prepare.close();
     } catch (SqlParseException | ValidationException
         | RelConversionException | SQLException e) {
-      throw new RuntimeException(e);
+      throw Util.throwAsRuntime(e);
     }
   }