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 2019/07/15 16:39:00 UTC

[calcite] 04/06: [CALCITE-3166] Make RelBuilder configurable

This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit 15e63787a6c2ccaa01582991068d8e74daf93d1f
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Mon Jul 1 17:14:26 2019 -0700

    [CALCITE-3166] Make RelBuilder configurable
    
    Add immutable class RelBuilder.Config, and a builder for it.
    
    Disable elimination of duplicate aggregate calls (added in
    [CALCITE-3123]), until [CALCITE-3145] is fixed.
---
 .../java/org/apache/calcite/tools/RelBuilder.java  | 96 ++++++++++++++++++++--
 .../org/apache/calcite/test/RelBuilderTest.java    | 53 +++++++++++-
 2 files changed, 141 insertions(+), 8 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index 983fa35..079c3de 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -154,8 +154,8 @@ public class RelBuilder {
   private final RelFactories.SpoolFactory spoolFactory;
   private final RelFactories.RepeatUnionFactory repeatUnionFactory;
   private final Deque<Frame> stack = new ArrayDeque<>();
-  private final boolean simplify;
   private final RexSimplify simplifier;
+  private final Config config;
 
   protected RelBuilder(Context context, RelOptCluster cluster,
       RelOptSchema relOptSchema) {
@@ -164,7 +164,7 @@ public class RelBuilder {
     if (context == null) {
       context = Contexts.EMPTY_CONTEXT;
     }
-    this.simplify = Hook.REL_BUILDER_SIMPLIFY.get(true);
+    this.config = getConfig(context);
     this.aggregateFactory =
         Util.first(context.unwrap(RelFactories.AggregateFactory.class),
             RelFactories.DEFAULT_AGGREGATE_FACTORY);
@@ -221,6 +221,21 @@ public class RelBuilder {
         new RexSimplify(cluster.getRexBuilder(), predicates, executor);
   }
 
+  /** Derives the Config to be used for this RelBuilder.
+   *
+   * <p>Overrides {@link RelBuilder.Config#simplify} if
+   * {@link Hook#REL_BUILDER_SIMPLIFY} is set.
+   */
+  private Config getConfig(Context context) {
+    final Config config =
+        Util.first(context.unwrap(Config.class), Config.DEFAULT);
+    boolean simplify = Hook.REL_BUILDER_SIMPLIFY.get(config.simplify);
+    if (simplify == config.simplify) {
+      return config;
+    }
+    return config.toBuilder().withSimplify(simplify).build();
+  }
+
   /** Creates a RelBuilder. */
   public static RelBuilder create(FrameworkConfig config) {
     return Frameworks.withPrepare(config,
@@ -1310,7 +1325,7 @@ public class RelBuilder {
     }
 
     // Simplify expressions.
-    if (simplify) {
+    if (config.simplify) {
       for (int i = 0; i < nodeList.size(); i++) {
         nodeList.set(i, simplifier.simplifyPreservingType(nodeList.get(i)));
       }
@@ -1616,7 +1631,7 @@ public class RelBuilder {
       assert groupSet.contains(set);
     }
 
-    if (Util.isDistinct(aggregateCalls)) {
+    if (!config.dedupAggregateCalls || Util.isDistinct(aggregateCalls)) {
       return aggregate_(groupSet, groupSets, r, aggregateCalls,
           registrar.extraNodes, frame.fields);
     } else {
@@ -1887,7 +1902,7 @@ public class RelBuilder {
     final RelNode join;
     final boolean correlate = variablesSet.size() == 1;
     RexNode postCondition = literal(true);
-    if (simplify) {
+    if (config.simplify) {
       // Normalize expanded versions IS NOT DISTINCT FROM so that simplifier does not
       // transform the expression to something unrecognizable
       if (condition instanceof RexCall) {
@@ -2760,6 +2775,77 @@ public class RelBuilder {
       }
     }
   }
+
+  /** Configuration of RelBuilder.
+   *
+   * <p>It is immutable, and all fields are public.
+   *
+   * <p>Use the {@link #DEFAULT} instance,
+   * or call {@link #builder()} to create a builder then
+   * {@link RelBuilder.ConfigBuilder#build()}. You can also use
+   * {@link #toBuilder()} to modify a few properties of an existing Config. */
+  public static class Config {
+    /** Default configuration. */
+    public static final Config DEFAULT =
+        new Config(false, true);
+
+    /** Whether {@link RelBuilder#aggregate} should eliminate duplicate
+     * aggregate calls; default true but currently disabled. */
+    public final boolean dedupAggregateCalls;
+
+    /** Whether to simplify expressions; default true. */
+    public final boolean simplify;
+
+    // called only from ConfigBuilder and when creating DEFAULT;
+    // parameters and fields must be in alphabetical order
+    private Config(boolean dedupAggregateCalls,
+        boolean simplify) {
+      this.dedupAggregateCalls = dedupAggregateCalls;
+      this.simplify = simplify;
+    }
+
+    /** Creates a ConfigBuilder with all properties set to their default
+     * values. */
+    public static ConfigBuilder builder() {
+      return DEFAULT.toBuilder();
+    }
+
+    /** Creates a ConfigBuilder with properties set to the values in this
+     * Config. */
+    public ConfigBuilder toBuilder() {
+      return new ConfigBuilder()
+          .withDedupAggregateCalls(dedupAggregateCalls)
+          .withSimplify(simplify);
+    }
+  }
+
+  /** Creates a {@link RelBuilder.Config}. */
+  public static class ConfigBuilder {
+    private boolean dedupAggregateCalls;
+    private boolean simplify;
+
+    private ConfigBuilder() {
+    }
+
+    /** Creates a {@link RelBuilder.Config}. */
+    public Config build() {
+      return new Config(dedupAggregateCalls, simplify);
+    }
+
+    /** Sets the value that will become
+     * {@link org.apache.calcite.tools.RelBuilder.Config#dedupAggregateCalls}. */
+    public ConfigBuilder withDedupAggregateCalls(boolean dedupAggregateCalls) {
+      this.dedupAggregateCalls = dedupAggregateCalls;
+      return this;
+    }
+
+    /** Sets the value that will become
+     * {@link org.apache.calcite.tools.RelBuilder.Config#simplify}. */
+    public ConfigBuilder withSimplify(boolean simplify) {
+      this.simplify = simplify;
+      return this;
+    }
+  }
 }
 
 // End RelBuilder.java
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index 29e42c2..1178a30 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -79,6 +79,8 @@ import java.util.List;
 import java.util.Locale;
 import java.util.NoSuchElementException;
 import java.util.TreeSet;
+import java.util.function.Function;
+import javax.annotation.Nonnull;
 
 import static org.apache.calcite.test.Matchers.hasTree;
 
@@ -157,6 +159,15 @@ public class RelBuilderTest {
     return Frameworks.newConfigBuilder().defaultSchema(root);
   }
 
+  @Nonnull static RelBuilder createBuilder(
+      Function<RelBuilder.ConfigBuilder, RelBuilder.ConfigBuilder> transform) {
+    final Frameworks.ConfigBuilder configBuilder = config();
+    configBuilder.context(
+        Contexts.of(transform.apply(RelBuilder.Config.builder())
+            .build()));
+    return RelBuilder.create(configBuilder.build());
+  }
+
   @Test public void testScan() {
     // Equivalent SQL:
     //   SELECT *
@@ -377,6 +388,31 @@ public class RelBuilderTest {
     assertThat(root, hasTree(expected));
   }
 
+  /** Tests that {@link RelBuilder#project} simplifies expressions if and only if
+   * {@link RelBuilder.Config#simplify}. */
+  @Test public void testSimplify() {
+    checkSimplify(configBuilder -> configBuilder.withSimplify(true),
+        hasTree("LogicalProject($f0=[true])\n"
+            + "  LogicalTableScan(table=[[scott, EMP]])\n"));
+    checkSimplify(configBuilder -> configBuilder,
+        hasTree("LogicalProject($f0=[true])\n"
+            + "  LogicalTableScan(table=[[scott, EMP]])\n"));
+    checkSimplify(configBuilder -> configBuilder.withSimplify(false),
+        hasTree("LogicalProject($f0=[IS NOT NULL($0)])\n"
+            + "  LogicalTableScan(table=[[scott, EMP]])\n"));
+  }
+
+  private void checkSimplify(
+      Function<RelBuilder.ConfigBuilder, RelBuilder.ConfigBuilder> transform,
+      Matcher<RelNode> matcher) {
+    final RelBuilder builder = createBuilder(transform);
+    final RelNode root =
+        builder.scan("EMP")
+            .project(builder.isNotNull(builder.field("EMPNO")))
+            .build();
+    assertThat(root, matcher);
+  }
+
   @Test public void testScanFilterOr() {
     // Equivalent SQL:
     //   SELECT *
@@ -931,8 +967,7 @@ public class RelBuilderTest {
   /** Tests that {@link RelBuilder#aggregate} eliminates duplicate aggregate
    * calls and creates a {@code Project} to compensate. */
   @Test public void testAggregateEliminatesDuplicateCalls() {
-    final RelBuilder builder = RelBuilder.create(config().build());
-    RelNode root =
+    final Function<RelBuilder, RelNode> fn = builder ->
         builder.scan("EMP")
             .aggregate(builder.groupKey(),
                 builder.sum(builder.field(1)).as("S1"),
@@ -940,11 +975,23 @@ public class RelBuilderTest {
                 builder.sum(builder.field(2)).as("S2"),
                 builder.sum(builder.field(1)).as("S1b"))
             .build();
+    final RelBuilder builder =
+        createBuilder(configBuilder ->
+            configBuilder.withDedupAggregateCalls(true));
     final String expected = ""
         + "LogicalProject(S1=[$0], C=[$1], S2=[$2], S1b=[$0])\n"
         + "  LogicalAggregate(group=[{}], S1=[SUM($1)], C=[COUNT()], S2=[SUM($2)])\n"
         + "    LogicalTableScan(table=[[scott, EMP]])\n";
-    assertThat(root, hasTree(expected));
+    assertThat(fn.apply(builder), hasTree(expected));
+
+    // Now, disable the rewrite
+    final RelBuilder builder2 =
+        createBuilder(configBuilder ->
+            configBuilder.withDedupAggregateCalls(false));
+    final String expected2 = ""
+        + "LogicalAggregate(group=[{}], S1=[SUM($1)], C=[COUNT()], S2=[SUM($2)], S1b=[SUM($1)])\n"
+        + "  LogicalTableScan(table=[[scott, EMP]])\n";
+    assertThat(fn.apply(builder2), hasTree(expected2));
   }
 
   @Test public void testAggregateFilter() {