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 2020/02/04 00:15:56 UTC
[calcite] 02/02: Switch RelBuilder.Config to an interface,
and deprecate RelBuilder.ConfigBuilder
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 c086877a8b7f7b47526d55e9e18dcb3576cdbaab
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Fri Jan 31 16:57:31 2020 -0800
Switch RelBuilder.Config to an interface, and deprecate RelBuilder.ConfigBuilder
---
.../java/org/apache/calcite/tools/RelBuilder.java | 106 ++++++++++++---------
.../org/apache/calcite/test/PigRelBuilderTest.java | 12 +++
.../org/apache/calcite/test/RelBuilderTest.java | 45 ++++-----
3 files changed, 93 insertions(+), 70 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 7b240e6..b4455a0 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -85,6 +85,7 @@ import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.type.TableFunctionReturnTypeInference;
import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.util.Holder;
+import org.apache.calcite.util.ImmutableBeans;
import org.apache.calcite.util.ImmutableBitSet;
import org.apache.calcite.util.ImmutableIntList;
import org.apache.calcite.util.ImmutableNullableList;
@@ -234,11 +235,8 @@ public class RelBuilder {
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();
+ boolean simplify = Hook.REL_BUILDER_SIMPLIFY.get(config.simplify());
+ return config.withSimplify(simplify);
}
/** Creates a RelBuilder. */
@@ -1418,7 +1416,7 @@ public class RelBuilder {
}
// Simplify expressions.
- if (config.simplify) {
+ if (config.simplify()) {
for (int i = 0; i < nodeList.size(); i++) {
nodeList.set(i, simplifier.simplifyPreservingType(nodeList.get(i)));
}
@@ -1735,7 +1733,7 @@ public class RelBuilder {
assert groupSet.contains(set);
}
- if (!config.dedupAggregateCalls || Util.isDistinct(aggregateCalls)) {
+ if (!config.dedupAggregateCalls() || Util.isDistinct(aggregateCalls)) {
return aggregate_(groupSet, groupSets, r, aggregateCalls,
registrar.extraNodes, frame.fields);
}
@@ -2038,7 +2036,7 @@ public class RelBuilder {
final RelNode join;
final boolean correlate = variablesSet.size() == 1;
RexNode postCondition = literal(true);
- if (config.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) {
@@ -2751,6 +2749,21 @@ public class RelBuilder {
this.filter = filter;
}
+ @Override public String toString() {
+ final StringBuilder b = new StringBuilder();
+ b.append(aggFunction.getName())
+ .append('(');
+ if (distinct) {
+ b.append("DISTINCT ");
+ }
+ b.append(operands)
+ .append(')');
+ if (filter != null) {
+ b.append(" FILTER (WHERE" + filter + ")");
+ }
+ return b.toString();
+ }
+
public AggCall sort(Iterable<RexNode> orderKeys) {
final ImmutableList<RexNode> orderKeyList =
ImmutableList.copyOf(orderKeys);
@@ -2813,6 +2826,10 @@ public class RelBuilder {
this.aggregateCall = Objects.requireNonNull(aggregateCall);
}
+ @Override public String toString() {
+ return aggregateCall.toString();
+ }
+
public AggCall sort(Iterable<RexNode> orderKeys) {
throw new UnsupportedOperationException();
}
@@ -3001,69 +3018,68 @@ public class 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 {
+ * <p>Start with the {@link #DEFAULT} instance,
+ * and call {@code withXxx} methods to set its properties. */
+ public interface Config {
/** Default configuration. */
- public static final Config DEFAULT =
- new Config(true, true);
+ Config DEFAULT = ImmutableBeans.create(Config.class);
+
+ @Deprecated // to be removed before 2.0
+ static ConfigBuilder builder() {
+ return DEFAULT.toBuilder();
+ }
+
+ @Deprecated // to be removed before 2.0
+ default ConfigBuilder toBuilder() {
+ return new ConfigBuilder(this);
+ }
/** Whether {@link RelBuilder#aggregate} should eliminate duplicate
* aggregate calls; default true. */
- public final boolean dedupAggregateCalls;
+ @ImmutableBeans.Property
+ @ImmutableBeans.BooleanDefault(true)
+ boolean dedupAggregateCalls();
- /** Whether to simplify expressions; default true. */
- public final boolean simplify;
+ /** Sets {@link #dedupAggregateCalls}. */
+ Config withDedupAggregateCalls(boolean dedupAggregateCalls);
- // 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();
- }
+ /** Whether to simplify expressions; default true. */
+ @ImmutableBeans.Property
+ @ImmutableBeans.BooleanDefault(true)
+ boolean simplify();
- /** Creates a ConfigBuilder with properties set to the values in this
- * Config. */
- public ConfigBuilder toBuilder() {
- return new ConfigBuilder()
- .withDedupAggregateCalls(dedupAggregateCalls)
- .withSimplify(simplify);
- }
+ /** Sets {@link #simplify}. */
+ Config withSimplify(boolean simplify);
}
- /** Creates a {@link RelBuilder.Config}. */
+ /** Creates a {@link RelBuilder.Config}.
+ *
+ * @deprecated Use the {@code withXxx} methods in
+ * {@link RelBuilder.Config}. */
+ @Deprecated // to be removed before 2.0
public static class ConfigBuilder {
- private boolean dedupAggregateCalls;
- private boolean simplify;
+ private Config config;
- private ConfigBuilder() {
+ private ConfigBuilder(@Nonnull Config config) {
+ this.config = config;
}
/** Creates a {@link RelBuilder.Config}. */
public Config build() {
- return new Config(dedupAggregateCalls, simplify);
+ return config;
}
/** Sets the value that will become
* {@link org.apache.calcite.tools.RelBuilder.Config#dedupAggregateCalls}. */
public ConfigBuilder withDedupAggregateCalls(boolean dedupAggregateCalls) {
- this.dedupAggregateCalls = dedupAggregateCalls;
+ config = config.withDedupAggregateCalls(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;
+ config = config.withSimplify(simplify);
return this;
}
}
diff --git a/core/src/test/java/org/apache/calcite/test/PigRelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/PigRelBuilderTest.java
index ab5e7cd..43b6bbe 100644
--- a/core/src/test/java/org/apache/calcite/test/PigRelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/PigRelBuilderTest.java
@@ -16,14 +16,18 @@
*/
package org.apache.calcite.test;
+import org.apache.calcite.plan.Contexts;
import org.apache.calcite.plan.RelOptUtil;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.tools.Frameworks;
import org.apache.calcite.tools.PigRelBuilder;
+import org.apache.calcite.tools.RelBuilder;
import org.apache.calcite.util.Util;
import org.junit.jupiter.api.Test;
+import java.util.function.UnaryOperator;
+
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
@@ -36,6 +40,14 @@ public class PigRelBuilderTest {
return RelBuilderTest.config();
}
+ static PigRelBuilder createBuilder(
+ UnaryOperator<RelBuilder.Config> transform) {
+ final Frameworks.ConfigBuilder configBuilder = config();
+ configBuilder.context(
+ Contexts.of(transform.apply(RelBuilder.Config.DEFAULT)));
+ return PigRelBuilder.create(configBuilder.build());
+ }
+
/** Converts a relational expression to a sting with linux line-endings. */
private String str(RelNode r) {
return Util.toLinux(RelOptUtil.toString(r));
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 c91d3c1..210b398 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -81,8 +81,7 @@ 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 java.util.function.UnaryOperator;
import static org.apache.calcite.test.Matchers.hasHints;
import static org.apache.calcite.test.Matchers.hasTree;
@@ -90,6 +89,7 @@ import static org.apache.calcite.test.Matchers.hasTree;
import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -164,12 +164,10 @@ public class RelBuilderTest {
return Frameworks.newConfigBuilder().defaultSchema(root);
}
- @Nonnull static RelBuilder createBuilder(
- Function<RelBuilder.ConfigBuilder, RelBuilder.ConfigBuilder> transform) {
+ static RelBuilder createBuilder(UnaryOperator<RelBuilder.Config> transform) {
final Frameworks.ConfigBuilder configBuilder = config();
configBuilder.context(
- Contexts.of(transform.apply(RelBuilder.Config.builder())
- .build()));
+ Contexts.of(transform.apply(RelBuilder.Config.DEFAULT)));
return RelBuilder.create(configBuilder.build());
}
@@ -396,19 +394,18 @@ public class RelBuilderTest {
/** Tests that {@link RelBuilder#project} simplifies expressions if and only if
* {@link RelBuilder.Config#simplify}. */
@Test public void testSimplify() {
- checkSimplify(configBuilder -> configBuilder.withSimplify(true),
+ checkSimplify(c -> c.withSimplify(true),
hasTree("LogicalProject($f0=[true])\n"
+ " LogicalTableScan(table=[[scott, EMP]])\n"));
- checkSimplify(configBuilder -> configBuilder,
+ checkSimplify(c -> c,
hasTree("LogicalProject($f0=[true])\n"
+ " LogicalTableScan(table=[[scott, EMP]])\n"));
- checkSimplify(configBuilder -> configBuilder.withSimplify(false),
+ checkSimplify(c -> c.withSimplify(false),
hasTree("LogicalProject($f0=[IS NOT NULL($0)])\n"
+ " LogicalTableScan(table=[[scott, EMP]])\n"));
}
- private void checkSimplify(
- Function<RelBuilder.ConfigBuilder, RelBuilder.ConfigBuilder> transform,
+ private void checkSimplify(UnaryOperator<RelBuilder.Config> transform,
Matcher<RelNode> matcher) {
final RelBuilder builder = createBuilder(transform);
final RelNode root =
@@ -728,7 +725,7 @@ public class RelBuilderTest {
assertTrue(root instanceof Project);
Project project = (Project) root;
Mappings.TargetMapping mapping = project.getMapping();
- assertTrue(mapping == null);
+ assertThat(mapping, nullValue());
}
private void project1(int value, SqlTypeName sqlTypeName, String message, String expected) {
@@ -999,30 +996,27 @@ 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 =
- 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(buildRelWithDuplicateAggregates(builder), hasTree(expected));
+ assertThat(
+ buildRelWithDuplicateAggregates(c -> c.withDedupAggregateCalls(true)),
+ 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(buildRelWithDuplicateAggregates(builder2), hasTree(expected2));
+ assertThat(
+ buildRelWithDuplicateAggregates(c -> c.withDedupAggregateCalls(false)),
+ hasTree(expected2));
}
/** As {@link #testAggregateEliminatesDuplicateCalls()} but with a
* single-column GROUP BY clause. */
@Test public void testAggregateEliminatesDuplicateCalls2() {
- final RelBuilder builder = RelBuilder.create(config().build());
- RelNode root = buildRelWithDuplicateAggregates(builder, 0);
+ RelNode root = buildRelWithDuplicateAggregates(c -> c, 0);
final String expected = ""
+ "LogicalProject(EMPNO=[$0], S1=[$1], C=[$2], S2=[$3], S1b=[$1])\n"
+ " LogicalAggregate(group=[{0}], S1=[SUM($1)], C=[COUNT()], S2=[SUM($2)])\n"
@@ -1033,8 +1027,7 @@ public class RelBuilderTest {
/** As {@link #testAggregateEliminatesDuplicateCalls()} but with a
* multi-column GROUP BY clause. */
@Test public void testAggregateEliminatesDuplicateCalls3() {
- final RelBuilder builder = RelBuilder.create(config().build());
- RelNode root = buildRelWithDuplicateAggregates(builder, 2, 0, 4, 3);
+ RelNode root = buildRelWithDuplicateAggregates(c -> c, 2, 0, 4, 3);
final String expected = ""
+ "LogicalProject(EMPNO=[$0], JOB=[$1], MGR=[$2], HIREDATE=[$3], S1=[$4], C=[$5], S2=[$6], S1b=[$4])\n"
+ " LogicalAggregate(group=[{0, 2, 3, 4}], S1=[SUM($1)], C=[COUNT()], S2=[SUM($2)])\n"
@@ -1042,8 +1035,10 @@ public class RelBuilderTest {
assertThat(root, hasTree(expected));
}
- private RelNode buildRelWithDuplicateAggregates(RelBuilder builder,
+ private RelNode buildRelWithDuplicateAggregates(
+ UnaryOperator<RelBuilder.Config> transform,
int... groupFieldOrdinals) {
+ final RelBuilder builder = createBuilder(transform);
return builder.scan("EMP")
.aggregate(builder.groupKey(groupFieldOrdinals),
builder.sum(builder.field(1)).as("S1"),