[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 <[email protected]> Authored: Sun Dec 20 12:07:33 2015 -0800 Committer: Julian Hyde <[email protected]> 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
