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 <[email protected]> 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); } }
