calcite-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jh...@apache.org
Subject [1/3] calcite git commit: [CALCITE-1416] Make classes implement AutoCloseable where possible (Chinmay Kolhatkar)
Date Tue, 01 Nov 2016 22:02:39 GMT
Repository: calcite
Updated Branches:
  refs/heads/master 35c417527 -> 03d6b00cd


[CALCITE-1416] Make classes implement AutoCloseable where possible (Chinmay Kolhatkar)

Classes include Planner, Interpreter, Source, Hook.Closeable, SqlTesterImpl.

Add class Closer, which closes AutoCloseable resources (Julian Hyde).

Close apache/calcite#316


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

Branch: refs/heads/master
Commit: 6378fa6889a2527f13ef70a223ee4ee305bb51fb
Parents: 35c4175
Author: Chinmay Kolhatkar <chinmay@datatorrent.com>
Authored: Sat Oct 22 11:47:28 2016 +0530
Committer: Julian Hyde <jhyde@apache.org>
Committed: Tue Nov 1 01:54:04 2016 -0700

----------------------------------------------------------------------
 .../apache/calcite/interpreter/Interpreter.java |  9 ++--
 .../org/apache/calcite/interpreter/Source.java  |  2 +-
 .../java/org/apache/calcite/runtime/Hook.java   | 16 +++---
 .../java/org/apache/calcite/tools/Planner.java  |  2 +-
 .../java/org/apache/calcite/util/Closer.java    | 50 ++++++++++++++++++
 .../apache/calcite/sql/test/SqlTesterImpl.java  |  8 +--
 .../org/apache/calcite/test/CalciteAssert.java  | 53 ++++++++++++--------
 .../org/apache/calcite/test/RelOptTestBase.java | 11 ++--
 .../org/apache/calcite/tools/PlannerTest.java   | 40 ++++++++-------
 9 files changed, 121 insertions(+), 70 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/6378fa68/core/src/main/java/org/apache/calcite/interpreter/Interpreter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/interpreter/Interpreter.java b/core/src/main/java/org/apache/calcite/interpreter/Interpreter.java
index bd76efd..5b2752c 100644
--- a/core/src/main/java/org/apache/calcite/interpreter/Interpreter.java
+++ b/core/src/main/java/org/apache/calcite/interpreter/Interpreter.java
@@ -57,15 +57,17 @@ import java.util.NoSuchElementException;
  *
  * <p>Contains the context for interpreting relational expressions. In
  * particular it holds working state while the data flow graph is being
- * assembled.</p>
+ * assembled.
  */
-public class Interpreter extends AbstractEnumerable<Object[]> {
+public class Interpreter extends AbstractEnumerable<Object[]>
+    implements AutoCloseable {
   final Map<RelNode, NodeInfo> nodes = Maps.newLinkedHashMap();
   private final DataContext dataContext;
   private final RelNode rootRel;
   private final Map<RelNode, List<RelNode>> relInputs = Maps.newHashMap();
   protected final ScalarCompiler scalarCompiler;
 
+  /** Creates an Interpreter. */
   public Interpreter(DataContext dataContext, RelNode rootRel) {
     this.dataContext = Preconditions.checkNotNull(dataContext);
     this.scalarCompiler =
@@ -118,8 +120,7 @@ public class Interpreter extends AbstractEnumerable<Object[]> {
     }
   }
 
-  private void close() {
-    // TODO:
+  public void close() {
   }
 
   /** Compiles an expression to an executable form. */

http://git-wip-us.apache.org/repos/asf/calcite/blob/6378fa68/core/src/main/java/org/apache/calcite/interpreter/Source.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/interpreter/Source.java b/core/src/main/java/org/apache/calcite/interpreter/Source.java
index 75b6c1b..1371819 100644
--- a/core/src/main/java/org/apache/calcite/interpreter/Source.java
+++ b/core/src/main/java/org/apache/calcite/interpreter/Source.java
@@ -21,7 +21,7 @@ package org.apache.calcite.interpreter;
  *
  * <p>Corresponds to an input of a relational expression.
  */
-public interface Source {
+public interface Source extends AutoCloseable {
   /** Reads a row. Null means end of data. */
   Row receive();
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/6378fa68/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 a97a406..6257f4a 100644
--- a/core/src/main/java/org/apache/calcite/runtime/Hook.java
+++ b/core/src/main/java/org/apache/calcite/runtime/Hook.java
@@ -71,12 +71,12 @@ public enum Hook {
   QUERY_PLAN;
 
   private final List<Function<Object, Object>> handlers =
-      new CopyOnWriteArrayList<Function<Object, Object>>();
+      new CopyOnWriteArrayList<>();
 
   private final ThreadLocal<List<Function<Object, Object>>> threadHandlers
=
       new ThreadLocal<List<Function<Object, Object>>>() {
         protected List<Function<Object, Object>> initialValue() {
-          return new ArrayList<Function<Object, Object>>();
+          return new ArrayList<>();
         }
       };
 
@@ -143,20 +143,16 @@ public enum Hook {
     return holder.get();
   }
 
-  /** Removes a Hook after use.
-   *
-   * <p>Note: Although it would be convenient, this interface cannot extend
-   * {@code AutoCloseable} while Calcite maintains compatibility with
-   * JDK 1.6.</p>
-   */
-  public interface Closeable /*extends AutoCloseable*/ {
+  /** Removes a Hook after use. */
+  public interface Closeable extends AutoCloseable {
     /** Closeable that does nothing. */
     Closeable EMPTY =
         new Closeable() {
           public void close() {}
         };
 
-    void close(); // override, removing "throws"
+    // override, removing "throws"
+    @Override void close();
   }
 }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/6378fa68/core/src/main/java/org/apache/calcite/tools/Planner.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/tools/Planner.java b/core/src/main/java/org/apache/calcite/tools/Planner.java
index 8b2b25d..2d9cd82 100644
--- a/core/src/main/java/org/apache/calcite/tools/Planner.java
+++ b/core/src/main/java/org/apache/calcite/tools/Planner.java
@@ -35,7 +35,7 @@ import org.apache.calcite.util.Pair;
  * reset() after each use of Planner that corresponds to a different
  * query.
  */
-public interface Planner {
+public interface Planner extends AutoCloseable {
   /**
    * Parses and validates a SQL statement.
    *

http://git-wip-us.apache.org/repos/asf/calcite/blob/6378fa68/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
new file mode 100644
index 0000000..e82fdbc
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/util/Closer.java
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.util;
+
+import com.google.common.base.Throwables;
+
+import java.io.IOException;
+import java.util.ArrayList;
+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}. */
+public final class Closer implements AutoCloseable {
+  private final List<AutoCloseable> list = new ArrayList<>();
+
+  /** Registers a resource. */
+  public <E extends AutoCloseable> E add(E e) {
+    list.add(e);
+    return e;
+  }
+
+  public void close() {
+    for (AutoCloseable closeable : list) {
+      try {
+        closeable.close();
+      } catch (Exception e) {
+        throw Throwables.propagate(e);
+      }
+    }
+  }
+}
+
+// End Closer.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/6378fa68/core/src/test/java/org/apache/calcite/sql/test/SqlTesterImpl.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlTesterImpl.java b/core/src/test/java/org/apache/calcite/sql/test/SqlTesterImpl.java
index fdf08f8..663714d 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlTesterImpl.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlTesterImpl.java
@@ -74,7 +74,7 @@ import static org.junit.Assert.fail;
  * Implementation of {@link org.apache.calcite.test.SqlValidatorTestCase.Tester}
  * that talks to a mock catalog.
  */
-public class SqlTesterImpl implements SqlTester {
+public class SqlTesterImpl implements SqlTester, AutoCloseable {
   protected final SqlTestFactory factory;
 
   public SqlTesterImpl(SqlTestFactory factory) {
@@ -385,11 +385,7 @@ public class SqlTesterImpl implements SqlTester {
     for (String sql : buildQueries(expression)) {
       TypeChecker typeChecker =
           new SqlTests.StringTypeChecker(expectedType);
-      check(
-          sql,
-          typeChecker,
-          new Double(expectedResult),
-          delta);
+      check(sql, typeChecker, expectedResult, delta);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/6378fa68/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 db8e654..c877d00 100644
--- a/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
+++ b/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
@@ -33,6 +33,8 @@ import org.apache.calcite.schema.Schema;
 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;
@@ -458,8 +460,7 @@ public class CalciteAssert {
     final String message =
         "With materializationsEnabled=" + materializationsEnabled
             + ", limit=" + limit;
-    final List<Hook.Closeable> closeableList = Lists.newArrayList();
-    try {
+    try (final Closer closer = new Closer()) {
       if (connection instanceof CalciteConnection) {
         CalciteConnection calciteConnection = (CalciteConnection) connection;
         calciteConnection.getProperties().setProperty(
@@ -470,7 +471,7 @@ public class CalciteAssert {
             Boolean.toString(materializationsEnabled));
       }
       for (Pair<Hook, Function> hook : hooks) {
-        closeableList.add(hook.left.addThread(hook.right));
+        closer.add(hook.left.addThread(hook.right));
       }
       Statement statement = connection.createStatement();
       statement.setMaxRows(limit <= 0 ? limit : Math.max(limit, 1));
@@ -511,10 +512,6 @@ public class CalciteAssert {
       throw e;
     } catch (Throwable e) {
       throw new RuntimeException(message, e);
-    } finally {
-      for (Hook.Closeable closeable : closeableList) {
-        closeable.close();
-      }
     }
   }
 
@@ -526,27 +523,27 @@ public class CalciteAssert {
       final Function<RelNode, Void> substitutionChecker) throws Exception {
     final String message =
         "With materializationsEnabled=" + materializationsEnabled;
-    final Hook.Closeable closeable =
-        convertChecker == null
-            ? Hook.Closeable.EMPTY
-            : Hook.TRIMMED.addThread(
+    try (Closer closer = new Closer()) {
+      if (convertChecker != null) {
+        closer.add(
+            Hook.TRIMMED.addThread(
                 new Function<RelNode, Void>() {
                   public Void apply(RelNode rel) {
                     convertChecker.apply(rel);
                     return null;
                   }
-                });
-    final Hook.Closeable closeable2 =
-        substitutionChecker == null
-            ? Hook.Closeable.EMPTY
-            : Hook.SUB.addThread(
+                }));
+      }
+      if (substitutionChecker != null) {
+        closer.add(
+            Hook.SUB.addThread(
                 new Function<RelNode, Void>() {
                   public Void apply(RelNode rel) {
                     substitutionChecker.apply(rel);
                     return null;
                   }
-                });
-    try {
+                }));
+      }
       ((CalciteConnection) connection).getProperties().setProperty(
           CalciteConnectionProperty.MATERIALIZATIONS_ENABLED.camelName(),
           Boolean.toString(materializationsEnabled));
@@ -558,9 +555,6 @@ public class CalciteAssert {
       connection.close();
     } catch (Throwable e) {
       throw new RuntimeException(message, e);
-    } finally {
-      closeable.close();
-      closeable2.close();
     }
   }
 
@@ -1422,6 +1416,23 @@ public class CalciteAssert {
     private <T> void addHook(Hook hook, Function<T, Void> handler) {
       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));
+    }
   }
 
   /** Fluent interface for building a metadata query to be tested. */

http://git-wip-us.apache.org/repos/asf/calcite/blob/6378fa68/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 1556094..e405874 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java
@@ -28,13 +28,13 @@ import org.apache.calcite.rel.metadata.ChainedRelMetadataProvider;
 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;
 import com.google.common.collect.Lists;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
@@ -241,17 +241,12 @@ abstract class RelOptTestBase extends SqlToRelTestBase {
     }
 
     private void check(boolean unchanged) {
-      final List<Hook.Closeable> closeables = new ArrayList<>();
-      try {
+      try (final Closer closer = new Closer()) {
         for (Map.Entry<Hook, Function> entry : hooks.entrySet()) {
-          closeables.add(entry.getKey().addThread(entry.getValue()));
+          closer.add(entry.getKey().addThread(entry.getValue()));
         }
         checkPlanning(tester.withExpand(expand), null, hepPlanner, sql,
             unchanged);
-      } finally {
-        for (Hook.Closeable closeable : closeables) {
-          closeable.close();
-        }
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/calcite/blob/6378fa68/core/src/test/java/org/apache/calcite/tools/PlannerTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/tools/PlannerTest.java b/core/src/test/java/org/apache/calcite/tools/PlannerTest.java
index 648786b..01c8f94 100644
--- a/core/src/test/java/org/apache/calcite/tools/PlannerTest.java
+++ b/core/src/test/java/org/apache/calcite/tools/PlannerTest.java
@@ -1021,12 +1021,13 @@ public class PlannerTest {
         .defaultSchema(schema)
         .programs(Programs.ofRules(Programs.RULE_SET))
         .build();
-    Planner p = Frameworks.getPlanner(config);
-    SqlNode n = p.parse(tpchTestQuery);
-    n = p.validate(n);
-    RelNode r = p.rel(n).project();
-    String plan = RelOptUtil.toString(r);
-    p.close();
+    String plan;
+    try (Planner p = Frameworks.getPlanner(config)) {
+      SqlNode n = p.parse(tpchTestQuery);
+      n = p.validate(n);
+      RelNode r = p.rel(n).project();
+      plan = RelOptUtil.toString(r);
+    }
     return plan;
   }
 
@@ -1073,19 +1074,20 @@ public class PlannerTest {
     traitDefs.add(RelCollationTraitDef.INSTANCE);
     final SqlParser.Config parserConfig =
         SqlParser.configBuilder().setLex(Lex.MYSQL).build();
-    Planner p = Frameworks.getPlanner(
-        Frameworks.newConfigBuilder()
-            .parserConfig(parserConfig)
-            .defaultSchema(schema)
-            .traitDefs(traitDefs)
-            .programs(Programs.ofRules(Programs.RULE_SET))
-            .build());
-    SqlNode n = p.parse(query);
-    n = p.validate(n);
-    RelNode r = p.rel(n).project();
-    String plan = RelOptUtil.toString(r);
-    plan = Util.toLinux(plan);
-    p.close();
+    FrameworkConfig config = Frameworks.newConfigBuilder()
+      .parserConfig(parserConfig)
+      .defaultSchema(schema)
+      .traitDefs(traitDefs)
+      .programs(Programs.ofRules(Programs.RULE_SET))
+      .build();
+    String plan;
+    try (Planner p = Frameworks.getPlanner(config)) {
+      SqlNode n = p.parse(query);
+      n = p.validate(n);
+      RelNode r = p.rel(n).project();
+      plan = RelOptUtil.toString(r);
+      plan = Util.toLinux(plan);
+    }
     assertThat(plan,
         equalTo("LogicalSort(sort0=[$0], dir0=[ASC])\n"
         + "  LogicalProject(psPartkey=[$0])\n"


Mime
View raw message