[CALCITE-4585] Improve error message from RelRunner (NobiGo)

If a query is executed via RelRunner.prepare(RelNode) and
fails, the exception should report the RelNode plan.
Currently it reports the SQL, and for this kind of query,
there is no SQL.

Close apache#2402

core/src/main/java/org/apache/calcite/jdbc/CalciteConnectionImpl.java

@@ -44,6 +44,7 @@
 import org.apache.calcite.linq4j.tree.Expressions;
 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.type.DelegatingTypeSystem;
 import org.apache.calcite.rel.type.RelDataTypeSystem;
@@ -222,8 +223,10 @@ private CalcitePreparedStatement prepareStatement_(
       server.getStatement(calcitePreparedStatement.handle).setSignature(signature);
       return calcitePreparedStatement;
     } catch (Exception e) {
-      throw Helper.INSTANCE.createException(
-          "Error while preparing statement [" + query.sql + "]", e);
+      String message = query.rel == null
+          ? "Error while preparing statement [" + query.sql + "]"
+          : "Error while preparing plan [" + RelOptUtil.toString(query.rel) + "]";
+      throw Helper.INSTANCE.createException(message, e);
     }
   }
 

core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java

@@ -18,15 +18,24 @@
 
 import org.apache.calcite.adapter.java.ReflectiveSchema;
 import org.apache.calcite.jdbc.CalciteConnection;
+import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.calcite.tools.Frameworks;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelRunner;
 
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import java.sql.Connection;
 import java.sql.DriverManager;
+import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import java.sql.Statement;
+import java.util.function.Function;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -79,6 +88,15 @@ public void setUp() throws SQLException {
     this.conn = calciteConnection;
   }
 
+  @AfterEach
+  public void tearDown() throws SQLException {
+    if (conn != null) {
+      Connection c = conn;
+      conn = null;
+      c.close();
+    }
+  }
+
   private void runQuery(String sql) throws SQLException {
     Statement stmt = conn.createStatement();
     try {
@@ -94,6 +112,33 @@ private void runQuery(String sql) throws SQLException {
     }
   }
 
+  /** Performs an action that requires a {@link RelBuilder}, and returns the
+   * result. */
+  private <T> T withRelBuilder(Function<RelBuilder, T> fn) throws SQLException {
+    final SchemaPlus rootSchema =
+        conn.unwrap(CalciteConnection.class).getRootSchema();
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .defaultSchema(rootSchema)
+        .build();
+    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);
+    try {
+      preparedStatement.executeQuery();
+    } finally {
+      try {
+        preparedStatement.close();
+      } catch (Exception e) {
+        fail("Error on close");
+      }
+    }
+  }
+
   @Test void testValidQuery() throws SQLException {
     // Just ensure that we're actually dealing with a valid connection
     // to be sure that the results of the other tests can be trusted
@@ -141,4 +186,37 @@ private void runQuery(String sql) throws SQLException {
           containsString("Object 'nonexistentTable' not found"));
     }
   }
+
+  /** Runs a query via {@link RelRunner}. */
+  @Test void testValidRelNodeQuery() throws SQLException {
+    final Function<RelBuilder, RelNode> relFn = b ->
+        b.scan("test", "entries")
+            .project(b.field("name"))
+            .build();
+    runQuery(relFn);
+  }
+
+  /** Runs a query via {@link RelRunner} that is expected to fail,
+   * and checks that the exception correctly describes the RelNode tree.
+   *
+   * <p>Test case for
+   * <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 {
+    try {
+      final Function<RelBuilder, RelNode> relFn = b ->
+          b.scan("test", "entries")
+              .project(b.call(SqlStdOperatorTable.ABS, b.field("name")))
+              .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 ["
+          + "LogicalProject($f0=[ABS($1)])\n"
+          + "  LogicalTableScan(table=[[test, entries]])\n"
+          + "]";
+      assertThat(e.getMessage(), Matchers.isLinux(message));
+    }
+  }
 }