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/11/01 22:02:40 UTC

[2/3] calcite git commit: Upgrade Quidem

Upgrade Quidem

Allow Hook instances to return values.

Fix some javadoc problems under JDK 1.7.


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

Branch: refs/heads/master
Commit: bfcd4980d2cec1b0d7d998190510c3604e261d4b
Parents: 6378fa6
Author: Julian Hyde <jh...@apache.org>
Authored: Mon Oct 31 15:58:24 2016 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Tue Nov 1 13:30:45 2016 -0700

----------------------------------------------------------------------
 .../calcite/prepare/CalcitePrepareImpl.java     | 32 ++++++++++++--------
 .../org/apache/calcite/prepare/Prepare.java     |  8 ++---
 .../apache/calcite/prepare/RelOptTableImpl.java |  3 +-
 .../java/org/apache/calcite/runtime/Hook.java   | 17 +++++++++++
 .../java/org/apache/calcite/util/Closer.java    |  4 +--
 .../org/apache/calcite/test/CalciteAssert.java  | 15 +--------
 .../java/org/apache/calcite/test/JdbcTest.java  |  7 ++---
 .../org/apache/calcite/test/QuidemTest.java     | 24 ++++++++++++---
 .../org/apache/calcite/test/RelOptTestBase.java | 15 +--------
 .../calcite/adapter/druid/DruidTable.java       |  1 -
 pom.xml                                         |  2 +-
 .../apache/calcite/test/SparkAdapterTest.java   |  8 +++++
 12 files changed, 76 insertions(+), 60 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java b/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
index fcbee23..cd2ad50 100644
--- a/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
+++ b/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
@@ -172,7 +172,7 @@ public class CalcitePrepareImpl implements CalcitePrepare {
 
   /** Whether the bindable convention should be the root convention of any
    * plan. If not, enumerable convention is the default. */
-  public static final boolean ENABLE_BINDABLE = false;
+  public final boolean enableBindable = Hook.ENABLE_BINDABLE.get(false);
 
   /** Whether the enumerable convention is enabled. */
   public static final boolean ENABLE_ENUMERABLE = true;
@@ -294,7 +294,7 @@ public class CalcitePrepareImpl implements CalcitePrepare {
       SqlNode sqlNode1) {
     final JavaTypeFactory typeFactory = context.getTypeFactory();
     final Convention resultConvention =
-        ENABLE_BINDABLE ? BindableConvention.INSTANCE
+        enableBindable ? BindableConvention.INSTANCE
             : EnumerableConvention.INSTANCE;
     final HepPlanner planner = new HepPlanner(new HepProgramBuilder().build());
     planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
@@ -503,7 +503,7 @@ public class CalcitePrepareImpl implements CalcitePrepare {
     if (prepareContext.config().materializationsEnabled()) {
       planner.addRule(MaterializedViewFilterScanRule.INSTANCE);
     }
-    if (ENABLE_BINDABLE) {
+    if (enableBindable) {
       for (RelOptRule rule : Bindables.RULES) {
         planner.addRule(rule);
       }
@@ -519,7 +519,7 @@ public class CalcitePrepareImpl implements CalcitePrepare {
       planner.addRule(EnumerableInterpreterRule.INSTANCE);
     }
 
-    if (ENABLE_BINDABLE && ENABLE_ENUMERABLE) {
+    if (enableBindable && ENABLE_ENUMERABLE) {
       planner.addRule(
           EnumerableBindable.EnumerableToBindableConverterRule.INSTANCE);
     }
@@ -683,7 +683,7 @@ public class CalcitePrepareImpl implements CalcitePrepare {
       prefer = EnumerableRel.Prefer.CUSTOM;
     }
     final Convention resultConvention =
-        ENABLE_BINDABLE ? BindableConvention.INSTANCE
+        enableBindable ? BindableConvention.INSTANCE
             : EnumerableConvention.INSTANCE;
     final CalcitePreparingStmt preparingStmt =
         new CalcitePreparingStmt(this, context, catalogReader, typeFactory,
@@ -779,17 +779,19 @@ public class CalcitePrepareImpl implements CalcitePrepare {
     if (preparedResult instanceof Typed) {
       resultClazz = (Class) ((Typed) preparedResult).getElementType();
     }
+    final Meta.CursorFactory cursorFactory =
+        preparingStmt.resultConvention == BindableConvention.INSTANCE
+            ? Meta.CursorFactory.ARRAY
+            : Meta.CursorFactory.deduce(columns, resultClazz);
     //noinspection unchecked
-    final Bindable<T> bindable = preparedResult.getBindable();
+    final Bindable<T> bindable = preparedResult.getBindable(cursorFactory);
     return new CalciteSignature<>(
         query.sql,
         parameters,
         preparingStmt.internalParameters,
         jdbcType,
         columns,
-        preparingStmt.resultConvention == BindableConvention.INSTANCE
-            ? Meta.CursorFactory.ARRAY
-            : Meta.CursorFactory.deduce(columns, resultClazz),
+        cursorFactory,
         preparedResult instanceof Prepare.PreparedResultImpl
             ? ((Prepare.PreparedResultImpl) preparedResult).collations
             : ImmutableList.<RelCollation>of(),
@@ -1226,7 +1228,7 @@ public class CalcitePrepareImpl implements CalcitePrepare {
           throw new UnsupportedOperationException();
         }
 
-        public Bindable getBindable() {
+        public Bindable getBindable(Meta.CursorFactory cursorFactory) {
           return bindable;
         }
 
@@ -1263,11 +1265,17 @@ public class CalcitePrepareImpl implements CalcitePrepare {
       super(resultType, parameterRowType, root, format, detailLevel);
     }
 
-    public Bindable getBindable() {
+    public Bindable getBindable(final Meta.CursorFactory cursorFactory) {
       final String explanation = getCode();
       return new Bindable() {
         public Enumerable bind(DataContext dataContext) {
-          return Linq4j.singletonEnumerable(explanation);
+          switch (cursorFactory.style) {
+          case ARRAY:
+            return Linq4j.singletonEnumerable(new String[] {explanation});
+          case OBJECT:
+          default:
+            return Linq4j.singletonEnumerable(explanation);
+          }
         }
       };
     }

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/core/src/main/java/org/apache/calcite/prepare/Prepare.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/prepare/Prepare.java b/core/src/main/java/org/apache/calcite/prepare/Prepare.java
index 0860d16..3532ed3 100644
--- a/core/src/main/java/org/apache/calcite/prepare/Prepare.java
+++ b/core/src/main/java/org/apache/calcite/prepare/Prepare.java
@@ -18,6 +18,7 @@ package org.apache.calcite.prepare;
 
 import org.apache.calcite.DataContext;
 import org.apache.calcite.adapter.java.JavaTypeFactory;
+import org.apache.calcite.avatica.Meta;
 import org.apache.calcite.jdbc.CalcitePrepare;
 import org.apache.calcite.jdbc.CalciteSchema;
 import org.apache.calcite.jdbc.CalciteSchema.LatticeEntry;
@@ -441,8 +442,6 @@ public abstract class Prepare {
       return Collections.singletonList(
           Collections.<String>nCopies(4, null));
     }
-
-    public abstract Bindable getBindable();
   }
 
   /**
@@ -481,9 +480,10 @@ public abstract class Prepare {
     /**
      * Executes the prepared result.
      *
+     * @param cursorFactory How to map values into a cursor
      * @return producer of rows resulting from execution
      */
-    Bindable getBindable();
+    Bindable getBindable(Meta.CursorFactory cursorFactory);
   }
 
   /**
@@ -546,8 +546,6 @@ public abstract class Prepare {
     public RelNode getRootRel() {
       return rootRel;
     }
-
-    public abstract Bindable getBindable();
   }
 
   /** Describes that a given SQL query is materialized by a given table.

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java b/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java
index 969fa99..6d7ffac 100644
--- a/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java
+++ b/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java
@@ -32,6 +32,7 @@ import org.apache.calcite.rel.logical.LogicalTableScan;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeField;
 import org.apache.calcite.rel.type.RelRecordType;
+import org.apache.calcite.runtime.Hook;
 import org.apache.calcite.schema.ExtensibleTable;
 import org.apache.calcite.schema.FilterableTable;
 import org.apache.calcite.schema.ModifiableTable;
@@ -251,7 +252,7 @@ public class RelOptTableImpl implements Prepare.PreparingTable {
       return ((TranslatableTable) table).toRel(context, this);
     }
     final RelOptCluster cluster = context.getCluster();
-    if (CalcitePrepareImpl.ENABLE_BINDABLE) {
+    if (Hook.ENABLE_BINDABLE.get(false)) {
       return LogicalTableScan.create(cluster, this);
     }
     if (CalcitePrepareImpl.ENABLE_ENUMERABLE

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/core/src/main/java/org/apache/calcite/runtime/Hook.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/runtime/Hook.java b/core/src/main/java/org/apache/calcite/runtime/Hook.java
index 6257f4a..efa9004 100644
--- a/core/src/main/java/org/apache/calcite/runtime/Hook.java
+++ b/core/src/main/java/org/apache/calcite/runtime/Hook.java
@@ -39,6 +39,11 @@ public enum Hook {
    * Default true. */
   REL_BUILDER_SIMPLIFY,
 
+  /** Returns a boolean value, whether the return convention should be
+   * {@link org.apache.calcite.interpreter.BindableConvention}.
+   * Default false. */
+  ENABLE_BINDABLE,
+
   /** Called with the SQL string and parse tree, in an array. */
   PARSE_TREE,
 
@@ -125,6 +130,18 @@ public enum Hook {
     return threadHandlers.get().remove(handler);
   }
 
+  /** Returns a function that, when a hook is called, will "return" a given
+   * value. (Because of the way hooks work, it "returns" the value by writing
+   * into a {@link Holder}. */
+  public static <V> Function<Holder<V>, Void> property(final V v) {
+    return new Function<Holder<V>, Void>() {
+      public Void apply(Holder<V> holder) {
+        holder.set(v);
+        return null;
+      }
+    };
+  }
+
   /** Runs all handlers registered for this Hook, with the given argument. */
   public void run(Object arg) {
     for (Function<Object, Object> handler : handlers) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/core/src/main/java/org/apache/calcite/util/Closer.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/util/Closer.java b/core/src/main/java/org/apache/calcite/util/Closer.java
index e82fdbc..b816d35 100644
--- a/core/src/main/java/org/apache/calcite/util/Closer.java
+++ b/core/src/main/java/org/apache/calcite/util/Closer.java
@@ -25,8 +25,8 @@ import java.util.List;
 /** Helper that holds onto {@link AutoCloseable} resources and releases them
  * when its {@code #close} method is called.
  *
- * <p>Similar to {@link com.google.common.io.Closer} but can deal with
- * {@link AutoCloseable} and doesn't throw {@link IOException}. */
+ * <p>Similar to {@code com.google.common.io.Closer} but can deal with
+ * {@link AutoCloseable}, and doesn't throw {@link IOException}. */
 public final class Closer implements AutoCloseable {
   private final List<AutoCloseable> list = new ArrayList<>();
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/CalciteAssert.java b/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
index c877d00..c5efd87 100644
--- a/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
+++ b/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
@@ -34,7 +34,6 @@ import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.schema.impl.AbstractSchema;
 import org.apache.calcite.schema.impl.ViewTable;
 import org.apache.calcite.util.Closer;
-import org.apache.calcite.util.Holder;
 import org.apache.calcite.util.JsonBuilder;
 import org.apache.calcite.util.Pair;
 import org.apache.calcite.util.Util;
@@ -1417,21 +1416,9 @@ public class CalciteAssert {
       hooks.add(Pair.of(hook, (Function) handler));
     }
 
-    /** Returns a function that, when a hook is called, will "return" a given
-     * value. (Because of the way hooks work, it "returns" the value by writing
-     * into a {@link Holder}. */
-    private <V> Function<Holder<V>, Void> propertyHook(final V v) {
-      return new Function<Holder<V>, Void>() {
-        public Void apply(Holder<V> holder) {
-          holder.set(v);
-          return null;
-        }
-      };
-    }
-
     /** Adds a property hook. */
     public <V> AssertQuery withProperty(Hook hook, V value) {
-      return withHook(hook, propertyHook(value));
+      return withHook(hook, Hook.property(value));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/core/src/test/java/org/apache/calcite/test/JdbcTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index 0eede43..eaa026b 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -6094,7 +6094,7 @@ public class JdbcTest {
   /** Tests that {@link Hook#PARSE_TREE} works. */
   @Test public void testHook() {
     final int[] callCount = {0};
-    final Hook.Closeable hook = Hook.PARSE_TREE.addThread(
+    try (Hook.Closeable hook = Hook.PARSE_TREE.addThread(
         new Function<Object[], Object>() {
           public Void apply(Object[] args) {
             assertThat(args.length, equalTo(2));
@@ -6107,8 +6107,7 @@ public class JdbcTest {
             ++callCount[0];
             return null;
           }
-        });
-    try {
+        })) {
       // Simple query does not run the hook.
       testSimple();
       assertThat(callCount[0], equalTo(0));
@@ -6116,8 +6115,6 @@ public class JdbcTest {
       // Non-trivial query runs hook once.
       testGroupByNull();
       assertThat(callCount[0], equalTo(1));
-    } finally {
-      hook.close();
     }
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/core/src/test/java/org/apache/calcite/test/QuidemTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/QuidemTest.java b/core/src/test/java/org/apache/calcite/test/QuidemTest.java
index a3dafba..78b202c 100644
--- a/core/src/test/java/org/apache/calcite/test/QuidemTest.java
+++ b/core/src/test/java/org/apache/calcite/test/QuidemTest.java
@@ -22,11 +22,13 @@ import org.apache.calcite.jdbc.CalciteConnection;
 import org.apache.calcite.prepare.Prepare;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.runtime.Hook;
 import org.apache.calcite.schema.Schema;
 import org.apache.calcite.schema.impl.AbstractSchema;
 import org.apache.calcite.schema.impl.AbstractTable;
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.util.Bug;
+import org.apache.calcite.util.Closer;
 import org.apache.calcite.util.TryThreadLocal;
 import org.apache.calcite.util.Util;
 
@@ -144,11 +146,22 @@ public class QuidemTest {
       outFile = new File(base, u2n("/surefire/") + path);
     }
     Util.discard(outFile.getParentFile().mkdirs());
-    final FileReader fileReader = new FileReader(inFile);
-    final BufferedReader bufferedReader = new BufferedReader(fileReader);
-    final FileWriter writer = new FileWriter(outFile);
-    new Quidem(bufferedReader, writer, env(), new QuidemConnectionFactory())
-        .execute();
+    try (final FileReader fileReader = new FileReader(inFile);
+         final BufferedReader bufferedReader = new BufferedReader(fileReader);
+         final FileWriter writer = new FileWriter(outFile);
+         final Closer closer = new Closer()) {
+      new Quidem(bufferedReader, writer, env(), new QuidemConnectionFactory())
+          .withPropertyHandler(new Quidem.PropertyHandler() {
+            public void onSet(String propertyName, Object value) {
+              if (propertyName.equals("bindable")) {
+                final boolean b = value instanceof Boolean
+                    && (Boolean) value;
+                closer.add(Hook.ENABLE_BINDABLE.addThread(Hook.property(b)));
+              }
+            }
+          })
+          .execute();
+    }
     final String diff = DiffTestCase.diff(inFile, outFile);
     if (!diff.isEmpty()) {
       fail("Files differ: " + outFile + " " + inFile + "\n"
@@ -314,6 +327,7 @@ public class QuidemTest {
       throw new RuntimeException("unknown connection '" + name + "'");
     }
   }
+
 }
 
 // End QuidemTest.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java b/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java
index e405874..e01fd5b 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java
@@ -29,7 +29,6 @@ import org.apache.calcite.rel.metadata.DefaultRelMetadataProvider;
 import org.apache.calcite.rel.metadata.RelMetadataProvider;
 import org.apache.calcite.runtime.Hook;
 import org.apache.calcite.util.Closer;
-import org.apache.calcite.util.Holder;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableMap;
@@ -212,20 +211,8 @@ abstract class RelOptTestBase extends SqlToRelTestBase {
               .put(hook, handler).build());
     }
 
-    /** Returns a function that, when a hook is called, will "return" a given
-     * value. (Because of the way hooks work, it "returns" the value by writing
-     * into a {@link Holder}. */
-    private <V> Function<Holder<V>, Void> propertyHook(final V v) {
-      return new Function<Holder<V>, Void>() {
-        public Void apply(Holder<V> holder) {
-          holder.set(v);
-          return null;
-        }
-      };
-    }
-
     public <V> Sql withProperty(Hook hook, V value) {
-      return withHook(hook, propertyHook(value));
+      return withHook(hook, Hook.property(value));
     }
 
     public Sql expand(boolean expand) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/druid/src/main/java/org/apache/calcite/adapter/druid/DruidTable.java
----------------------------------------------------------------------
diff --git a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidTable.java b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidTable.java
index 816bec4..182e03a 100644
--- a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidTable.java
+++ b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidTable.java
@@ -69,7 +69,6 @@ public class DruidTable extends AbstractTable implements TranslatableTable {
    * @param metricFieldNames Names of fields that are metrics
    * @param intervals Default interval if query does not constrain the time, or null
    * @param timestampFieldName Name of the column that contains the time
-   * @param intervals Intervals for the given table
    */
   public DruidTable(DruidSchema schema, String dataSource,
       RelProtoDataType protoRowType, Set<String> metricFieldNames,

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 2e46dcb..45d1fdb 100644
--- a/pom.xml
+++ b/pom.xml
@@ -107,7 +107,7 @@ limitations under the License.
     <oracle-jdbc6-driver.version>11.2.0.2.0</oracle-jdbc6-driver.version>
     <aggdesigner.version>6.0</aggdesigner.version>
     <postgresql.version>9.3-1102-jdbc3</postgresql.version>
-    <quidem.version>0.7</quidem.version>
+    <quidem.version>0.8</quidem.version>
     <scala.version>2.10.3</scala.version>
     <scott-data-hsqldb.version>0.1</scott-data-hsqldb.version>
     <servlet.version>3.0.1</servlet.version>

http://git-wip-us.apache.org/repos/asf/calcite/blob/bfcd4980/spark/src/test/java/org/apache/calcite/test/SparkAdapterTest.java
----------------------------------------------------------------------
diff --git a/spark/src/test/java/org/apache/calcite/test/SparkAdapterTest.java b/spark/src/test/java/org/apache/calcite/test/SparkAdapterTest.java
index 300801b..8f415fb 100644
--- a/spark/src/test/java/org/apache/calcite/test/SparkAdapterTest.java
+++ b/spark/src/test/java/org/apache/calcite/test/SparkAdapterTest.java
@@ -16,6 +16,9 @@
  */
 package org.apache.calcite.test;
 
+import org.apache.calcite.adapter.spark.SparkRel;
+import org.apache.calcite.util.Util;
+
 import org.junit.Test;
 
 import java.sql.SQLException;
@@ -30,6 +33,11 @@ public class SparkAdapterTest {
    * There are no data sources.
    */
   @Test public void testValues() throws SQLException {
+    // Insert a spurious reference to a class in Calcite's Spark adapter.
+    // Otherwise this test doesn't depend on the Spark module at all, and
+    // Javadoc gets confused.
+    Util.discard(SparkRel.class);
+
     CalciteAssert.that()
         .with(CalciteAssert.Config.SPARK)
         .query("select *\n"