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 2016/02/14 20:49:28 UTC

[2/2] calcite git commit: [CALCITE-1031] In prepared statement, CsvScannableTable.scan is called twice

[CALCITE-1031] In prepared statement, CsvScannableTable.scan is called twice

Close some unrelated PRs:
Close apache/calcite#190
Close apache/calcite#192


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

Branch: refs/heads/master
Commit: 8bbef2d6fbba8547adc55dcbb7d5b07a7a5437bc
Parents: f650bcc
Author: Julian Hyde <jh...@apache.org>
Authored: Sun Dec 20 12:07:33 2015 -0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Sun Feb 14 10:44:57 2016 -0800

----------------------------------------------------------------------
 .../calcite/avatica/AvaticaConnection.java      |  7 +-
 .../java/org/apache/calcite/avatica/Meta.java   |  2 +-
 .../org/apache/calcite/avatica/MetaImpl.java    |  2 +-
 .../apache/calcite/jdbc/CalciteMetaImpl.java    | 31 ++++----
 .../org/apache/calcite/runtime/Enumerables.java | 16 +++-
 .../apache/calcite/test/ScannableTableTest.java | 78 ++++++++++++++++++++
 .../java/org/apache/calcite/test/CsvTest.java   | 40 +++++++++-
 7 files changed, 150 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/8bbef2d6/avatica/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java
----------------------------------------------------------------------
diff --git a/avatica/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java b/avatica/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java
index 7336f01..4076c4e 100644
--- a/avatica/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java
+++ b/avatica/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java
@@ -115,7 +115,7 @@ public abstract class AvaticaConnection implements Connection {
   }
 
   /** Computes the number of retries
-   * {@link #executeQueryInternal(AvaticaStatement, Meta.Signature, Meta.Frame, QueryState)}
+   * {@link AvaticaStatement#executeInternal(Meta.Signature)}
    * should retry before failing. */
   long getNumStatementRetries(Properties props) {
     return Long.valueOf(Objects.requireNonNull(props)
@@ -517,8 +517,9 @@ public abstract class AvaticaConnection implements Connection {
       if (obj instanceof Number) {
         statement.updateCount = ((Number) obj).intValue();
       } else if (obj instanceof List) {
-        statement.updateCount =
-            ((Number) ((List<Object>) obj).get(0)).intValue();
+        @SuppressWarnings("unchecked")
+        final List<Number> numbers = (List<Number>) obj;
+        statement.updateCount = numbers.get(0).intValue();
       } else {
         throw helper.createException("Not a valid return result.");
       }

http://git-wip-us.apache.org/repos/asf/calcite/blob/8bbef2d6/avatica/src/main/java/org/apache/calcite/avatica/Meta.java
----------------------------------------------------------------------
diff --git a/avatica/src/main/java/org/apache/calcite/avatica/Meta.java b/avatica/src/main/java/org/apache/calcite/avatica/Meta.java
index 392086c..4f1d56a 100644
--- a/avatica/src/main/java/org/apache/calcite/avatica/Meta.java
+++ b/avatica/src/main/java/org/apache/calcite/avatica/Meta.java
@@ -253,7 +253,7 @@ public interface Meta {
    * @param parameterValues A list of parameter values; may be empty, not null
    * @param maxRowCount Maximum number of rows to return; negative means
    * no limit
-   * @return Frame, or null if there are no more
+   * @return Execute result
    */
   ExecuteResult execute(StatementHandle h, List<TypedValue> parameterValues,
       long maxRowCount) throws NoSuchStatementException;

http://git-wip-us.apache.org/repos/asf/calcite/blob/8bbef2d6/avatica/src/main/java/org/apache/calcite/avatica/MetaImpl.java
----------------------------------------------------------------------
diff --git a/avatica/src/main/java/org/apache/calcite/avatica/MetaImpl.java b/avatica/src/main/java/org/apache/calcite/avatica/MetaImpl.java
index f373273..7bb99fc 100644
--- a/avatica/src/main/java/org/apache/calcite/avatica/MetaImpl.java
+++ b/avatica/src/main/java/org/apache/calcite/avatica/MetaImpl.java
@@ -118,7 +118,7 @@ public abstract class MetaImpl implements Meta {
   }
 
   public static List<List<Object>> collect(CursorFactory cursorFactory,
-      Iterable<Object> iterable, List<List<Object>> list) {
+      Iterable<Object> iterable, final List<List<Object>> list) {
     switch (cursorFactory.style) {
     case OBJECT:
       for (Object o : iterable) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/8bbef2d6/core/src/main/java/org/apache/calcite/jdbc/CalciteMetaImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/jdbc/CalciteMetaImpl.java b/core/src/main/java/org/apache/calcite/jdbc/CalciteMetaImpl.java
index 01b07bc..fec7346 100644
--- a/core/src/main/java/org/apache/calcite/jdbc/CalciteMetaImpl.java
+++ b/core/src/main/java/org/apache/calcite/jdbc/CalciteMetaImpl.java
@@ -598,12 +598,13 @@ public class CalciteMetaImpl extends MetaImpl {
     } else {
       iterator = stmt.getResultSet();
     }
-    final List<List<Object>> list = new ArrayList<>();
-    List<List<Object>> rows =
+    final List rows =
         MetaImpl.collect(signature.cursorFactory,
-            LimitIterator.of(iterator, fetchMaxRowCount), list);
-    boolean done = fetchMaxRowCount == 0 || list.size() < fetchMaxRowCount;
-    return new Meta.Frame(offset, done, (List<Object>) (List) rows);
+            LimitIterator.of(iterator, fetchMaxRowCount),
+            new ArrayList<List<Object>>());
+    boolean done = fetchMaxRowCount == 0 || rows.size() < fetchMaxRowCount;
+    @SuppressWarnings("unchecked") List<Object> rows1 = (List<Object>) rows;
+    return new Meta.Frame(offset, done, rows1);
   }
 
   @Override public ExecuteResult execute(StatementHandle h,
@@ -611,26 +612,20 @@ public class CalciteMetaImpl extends MetaImpl {
     final CalciteConnectionImpl calciteConnection = getConnection();
     CalciteServerStatement stmt = calciteConnection.server.getStatement(h);
     final Signature signature = stmt.getSignature();
-    final Iterator<Object> iterator;
-
-    final Iterable<Object> iterable =
-        _createIterable(h, signature, parameterValues, null);
-    iterator = iterable.iterator();
-    stmt.setResultSet(iterator);
 
     MetaResultSet metaResultSet;
     if (signature.statementType.canUpdate()) {
+      final Iterable<Object> iterable =
+          _createIterable(h, signature, parameterValues, null);
+      final Iterator<Object> iterator = iterable.iterator();
+      stmt.setResultSet(iterator);
       metaResultSet = MetaResultSet.count(h.connectionId, h.id,
           ((Number) iterator.next()).intValue());
     } else {
-      final List<List<Object>> list = new ArrayList<>();
-      List<List<Object>> rows =
-          MetaImpl.collect(signature.cursorFactory,
-              LimitIterator.of(iterator, maxRowCount), list);
-      final boolean done = maxRowCount == 0 || list.size() < maxRowCount;
+      // Don't populate the first frame.
+      // It's not worth saving a round-trip, since we're local.
       final Meta.Frame frame =
-          new Meta.Frame(0, done, (List<Object>) (List) rows);
-
+          new Meta.Frame(0, false, Collections.emptyList());
       metaResultSet =
           MetaResultSet.create(h.connectionId, h.id, false, signature, frame);
     }

http://git-wip-us.apache.org/repos/asf/calcite/blob/8bbef2d6/core/src/main/java/org/apache/calcite/runtime/Enumerables.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/runtime/Enumerables.java b/core/src/main/java/org/apache/calcite/runtime/Enumerables.java
index c8ebe4d..22a85c7 100644
--- a/core/src/main/java/org/apache/calcite/runtime/Enumerables.java
+++ b/core/src/main/java/org/apache/calcite/runtime/Enumerables.java
@@ -28,6 +28,7 @@ import org.apache.calcite.linq4j.function.Predicate1;
 import org.apache.calcite.linq4j.function.Predicate2;
 import org.apache.calcite.util.Bug;
 
+import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
@@ -199,8 +200,19 @@ public class Enumerables {
 
   /** Converts an {@link Enumerable} over object arrays into an
    * {@link Enumerable} over {@link Row} objects. */
-  public static Enumerable<Row> toRow(final Enumerable<Object[]> enumerator) {
-    return enumerator.select(ARRAY_TO_ROW);
+  public static Enumerable<Row> toRow(final Enumerable<Object[]> enumerable) {
+    return enumerable.select(ARRAY_TO_ROW);
+  }
+
+  /** Converts a supplier of an {@link Enumerable} over object arrays into a
+   * supplier of an {@link Enumerable} over {@link Row} objects. */
+  public static Supplier<Enumerable<Row>>
+  toRow(final Supplier<Enumerable<Object[]>> supplier) {
+    return new Supplier<Enumerable<Row>>() {
+      public Enumerable<Row> get() {
+        return toRow(supplier.get());
+      }
+    };
   }
 
   /** Joins two inputs that are sorted on the key. */

http://git-wip-us.apache.org/repos/asf/calcite/blob/8bbef2d6/core/src/test/java/org/apache/calcite/test/ScannableTableTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/ScannableTableTest.java b/core/src/test/java/org/apache/calcite/test/ScannableTableTest.java
index 2c24295..05e40c7 100644
--- a/core/src/test/java/org/apache/calcite/test/ScannableTableTest.java
+++ b/core/src/test/java/org/apache/calcite/test/ScannableTableTest.java
@@ -35,25 +35,33 @@ import org.apache.calcite.schema.Schema;
 import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.schema.Statistic;
 import org.apache.calcite.schema.Statistics;
+import org.apache.calcite.schema.Table;
 import org.apache.calcite.schema.impl.AbstractSchema;
 import org.apache.calcite.schema.impl.AbstractTable;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.type.SqlTypeName;
 
+import com.google.common.collect.ImmutableMap;
+
 import org.junit.Assert;
 import org.junit.Test;
 
 import java.math.BigDecimal;
 import java.sql.Connection;
 import java.sql.DriverManager;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
@@ -283,6 +291,76 @@ public class ScannableTableTest {
       equalTo("returnCount=4, projects=[2]"));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-1031">[CALCITE-1031]
+   * In prepared statement, CsvScannableTable.scan is called twice</a>. */
+  @Test public void testPrepared2() throws SQLException {
+    final Properties properties = new Properties();
+    properties.setProperty("caseSensitive", "true");
+    try (final Connection connection =
+             DriverManager.getConnection("jdbc:calcite:", properties)) {
+      final CalciteConnection calciteConnection = connection.unwrap(
+          CalciteConnection.class);
+
+      final AtomicInteger scanCount = new AtomicInteger();
+      final AtomicInteger enumerateCount = new AtomicInteger();
+      final Schema schema =
+          new AbstractSchema() {
+            @Override protected Map<String, Table> getTableMap() {
+              return ImmutableMap.<String, Table>of("TENS",
+                  new SimpleTable() {
+                    private Enumerable<Object[]> superScan(DataContext root) {
+                      return super.scan(root);
+                    }
+
+                    @Override public Enumerable<Object[]>
+                    scan(final DataContext root) {
+                      scanCount.incrementAndGet();
+                      return new AbstractEnumerable<Object[]>() {
+                        public Enumerator<Object[]> enumerator() {
+                          enumerateCount.incrementAndGet();
+                          return superScan(root).enumerator();
+                        }
+                      };
+                    }
+                  });
+            }
+          };
+      calciteConnection.getRootSchema().add("TEST", schema);
+      final String sql = "select * from \"TEST\".\"TENS\" where \"i\" < ?";
+      final PreparedStatement statement =
+          calciteConnection.prepareStatement(sql);
+      assertThat(scanCount.get(), is(0));
+      assertThat(enumerateCount.get(), is(0));
+
+      // First execute
+      statement.setInt(1, 20);
+      assertThat(scanCount.get(), is(0));
+      ResultSet resultSet = statement.executeQuery();
+      assertThat(scanCount.get(), is(1));
+      assertThat(enumerateCount.get(), is(1));
+      assertThat(resultSet,
+          Matchers.returnsUnordered("i=0", "i=10"));
+      assertThat(scanCount.get(), is(1));
+      assertThat(enumerateCount.get(), is(1));
+
+      // Second execute
+      resultSet = statement.executeQuery();
+      assertThat(scanCount.get(), is(2));
+      assertThat(resultSet,
+          Matchers.returnsUnordered("i=0", "i=10"));
+      assertThat(scanCount.get(), is(2));
+
+      // Third execute
+      statement.setInt(1, 30);
+      resultSet = statement.executeQuery();
+      assertThat(scanCount.get(), is(3));
+      assertThat(resultSet,
+          Matchers.returnsUnordered("i=0", "i=10", "i=20"));
+      assertThat(scanCount.get(), is(3));
+    }
+  }
+
   /** Table that returns one column via the {@link ScannableTable} interface. */
   public static class SimpleTable implements ScannableTable {
     public RelDataType getRowType(RelDataTypeFactory typeFactory) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/8bbef2d6/example/csv/src/test/java/org/apache/calcite/test/CsvTest.java
----------------------------------------------------------------------
diff --git a/example/csv/src/test/java/org/apache/calcite/test/CsvTest.java b/example/csv/src/test/java/org/apache/calcite/test/CsvTest.java
index 4040867..173994b 100644
--- a/example/csv/src/test/java/org/apache/calcite/test/CsvTest.java
+++ b/example/csv/src/test/java/org/apache/calcite/test/CsvTest.java
@@ -16,10 +16,14 @@
  */
 package org.apache.calcite.test;
 
+import org.apache.calcite.adapter.csv.CsvSchemaFactory;
+import org.apache.calcite.jdbc.CalciteConnection;
 import org.apache.calcite.linq4j.function.Function1;
+import org.apache.calcite.schema.Schema;
 import org.apache.calcite.sql2rel.SqlToRelConverter;
 
 import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableMap;
 
 import org.junit.Assert;
 import org.junit.Ignore;
@@ -29,6 +33,7 @@ import java.io.PrintStream;
 import java.net.URL;
 import java.sql.Connection;
 import java.sql.DriverManager;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
@@ -271,7 +276,11 @@ public class CsvTest {
   }
 
   private String jsonPath(String model) {
-    final URL url = CsvTest.class.getResource("/" + model + ".json");
+    return resourcePath(model + ".json");
+  }
+
+  private String resourcePath(String path) {
+    final URL url = CsvTest.class.getResource("/" + path);
     String s = url.toString();
     if (s.startsWith("file:")) {
       s = s.substring("file:".length());
@@ -465,6 +474,35 @@ public class CsvTest {
     }
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-1031">[CALCITE-1031]
+   * In prepared statement, CsvScannableTable.scan is called twice</a>. To see
+   * the bug, place a breakpoint in CsvScannableTable.scan, and note that it is
+   * called twice. It should only be called once. */
+  @Test public void testPrepared() throws SQLException {
+    final Properties properties = new Properties();
+    properties.setProperty("caseSensitive", "true");
+    try (final Connection connection =
+        DriverManager.getConnection("jdbc:calcite:", properties)) {
+      final CalciteConnection calciteConnection = connection.unwrap(
+          CalciteConnection.class);
+
+      final Schema schema =
+          new CsvSchemaFactory()
+              .create(calciteConnection.getRootSchema(), null,
+                  ImmutableMap.<String, Object>of("directory",
+                      resourcePath("sales"), "flavor", "scannable"));
+      calciteConnection.getRootSchema().add("TEST", schema);
+      final String sql = "select * from \"TEST\".\"DEPTS\" where \"NAME\" = ?";
+      final PreparedStatement statement2 =
+          calciteConnection.prepareStatement(sql);
+
+      statement2.setString(1, "Sales");
+      final ResultSet resultSet1 = statement2.executeQuery();
+      Function1<ResultSet, Void> expect = expect("DEPTNO=10; NAME=Sales");
+      expect.apply(resultSet1);
+    }
+  }
 }
 
 // End CsvTest.java