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:38:56 UTC

[calcite] branch master updated (0732283 -> 0cce229)

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

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


    from 0732283  [CALCITE-3189] Multiple fixes for Oracle SQL dialect
     new bb0bae9  [CALCITE-3183] During field trimming, Filter is copied with wrong traitSet (Juhwan Kim)
     new 1c5de1c  [CALCITE-3196] In Frameworks, add interface BasePrepareAction (a functional interface) and deprecate abstract class PrepareAction
     new 687b7d8  [CALCITE-3144] Add rule, AggregateCaseToFilterRule, that converts "SUM(CASE WHEN b THEN x END)" to "SUM(x) FILTER (WHERE b)"
     new 15e6378  [CALCITE-3166] Make RelBuilder configurable
     new ab1f777  Change type of SqlStdOperatorTable.GROUPING field to public class
     new 0cce229  [CALCITE-3145] RelBuilder.aggregate throws IndexOutOfBoundsException if groupKey is non-empty and there are duplicate aggregate functions

The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../calcite/adapter/enumerable/EnumerableRel.java  |   9 +-
 .../org/apache/calcite/adapter/jdbc/JdbcRules.java |   9 +-
 .../java/org/apache/calcite/plan/RelOptRules.java  |   2 +
 .../java/org/apache/calcite/plan/RelOptUtil.java   |  11 +-
 .../apache/calcite/prepare/CalcitePrepareImpl.java |  17 +-
 .../org/apache/calcite/rel/core/AggregateCall.java |   2 +
 .../org/apache/calcite/rel/core/RelFactories.java  |  30 ++-
 .../rel/rules/AggregateCaseToFilterRule.java       | 268 +++++++++++++++++++++
 .../rel/rules/SemiJoinFilterTransposeRule.java     |   3 +-
 .../calcite/sql/fun/SqlSingleValueAggFunction.java |   4 +
 .../calcite/sql/fun/SqlStdOperatorTable.java       |   6 +-
 .../apache/calcite/sql2rel/RelFieldTrimmer.java    |   6 +-
 .../apache/calcite/sql2rel/SqlToRelConverter.java  |   5 +-
 .../java/org/apache/calcite/tools/Frameworks.java  |  59 +++--
 .../java/org/apache/calcite/tools/RelBuilder.java  | 190 +++++++++++----
 .../org/apache/calcite/rex/RexExecutorTest.java    |  58 ++---
 .../org/apache/calcite/test/RelBuilderTest.java    | 141 ++++++++++-
 .../org/apache/calcite/test/RelOptRulesTest.java   |  18 +-
 .../org/apache/calcite/test/RelOptTestBase.java    |   8 +-
 .../calcite/test/RexImplicationCheckerTest.java    |  26 +-
 .../apache/calcite/test/SqlToRelConverterTest.java |  64 +++++
 .../org/apache/calcite/tools/FrameworksTest.java   |  32 +--
 .../org/apache/calcite/test/RelOptRulesTest.xml    |  30 +++
 core/src/test/resources/sql/agg.iq                 |  32 +++
 core/src/test/resources/sql/misc.iq                |   4 +-
 .../adapter/geode/rel/GeodeAllDataTypesTest.java   |   4 +-
 .../main/java/org/apache/calcite/linq4j/Ord.java   |   2 +-
 .../calcite/adapter/pig/PigRelFactories.java       |  12 +-
 site/_docs/algebra.md                              |   4 +-
 29 files changed, 869 insertions(+), 187 deletions(-)
 create mode 100644 core/src/main/java/org/apache/calcite/rel/rules/AggregateCaseToFilterRule.java


[calcite] 05/06: Change type of SqlStdOperatorTable.GROUPING field to public class

Posted by jh...@apache.org.
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 ab1f777edb617ed9cf1c2f9b9a99c4dd0202a36b
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Wed Jun 12 18:56:38 2019 -0700

    Change type of SqlStdOperatorTable.GROUPING field to public class
---
 .../main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java   | 6 +++---
 linq4j/src/main/java/org/apache/calcite/linq4j/Ord.java             | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
index b033ea2..dae3b00 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
@@ -205,11 +205,11 @@ public class SqlStdOperatorTable extends ReflectiveSqlOperatorTable {
    * <p>Occurs in similar places to an aggregate
    * function ({@code SELECT}, {@code HAVING} clause, etc. of an aggregate
    * query), but not technically an aggregate function. */
-  public static final SqlGroupingFunction GROUPING =
+  public static final SqlAggFunction GROUPING =
       new SqlGroupingFunction("GROUPING");
 
   /** {@code GROUP_ID()} function. (Oracle-specific.) */
-  public static final SqlGroupIdFunction GROUP_ID =
+  public static final SqlAggFunction GROUP_ID =
       new SqlGroupIdFunction();
 
   /** {@code GROUPING_ID} function is a synonym for {@code GROUPING}.
@@ -222,7 +222,7 @@ public class SqlStdOperatorTable extends ReflectiveSqlOperatorTable {
    * <p>The SQL standard has changed to allow {@code GROUPING} to have multiple
    * arguments. It is now equivalent to {@code GROUPING_ID}, so we made
    * {@code GROUPING_ID} a synonym for {@code GROUPING}. */
-  public static final SqlGroupingFunction GROUPING_ID =
+  public static final SqlAggFunction GROUPING_ID =
       new SqlGroupingFunction("GROUPING_ID");
 
   /** {@code EXTEND} operator. */
diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/Ord.java b/linq4j/src/main/java/org/apache/calcite/linq4j/Ord.java
index 4f5192f..27a9e07 100644
--- a/linq4j/src/main/java/org/apache/calcite/linq4j/Ord.java
+++ b/linq4j/src/main/java/org/apache/calcite/linq4j/Ord.java
@@ -150,7 +150,7 @@ public class Ord<E> implements Map.Entry<Integer, E> {
     }
 
     public Ord<E> get(int index) {
-      return of(index, elements.get(index));
+      return Ord.of(index, elements.get(index));
     }
 
     public int size() {


[calcite] 03/06: [CALCITE-3144] Add rule, AggregateCaseToFilterRule, that converts "SUM(CASE WHEN b THEN x END)" to "SUM(x) FILTER (WHERE b)"

Posted by jh...@apache.org.
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 687b7d8cc8c63259138b43745a9e28ef64483839
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Mon Jul 1 12:57:14 2019 -0700

    [CALCITE-3144] Add rule, AggregateCaseToFilterRule, that converts "SUM(CASE WHEN b THEN x END)" to "SUM(x) FILTER (WHERE b)"
    
    Copied from Apache Druid's CaseFilteredAggregatorRule.
    
    Some aggregate functions (e.g. SINGLE_VALUE, GROUPING, GROUP_ID) do
    not allow filter, so we skip them in the rule.
---
 .../java/org/apache/calcite/plan/RelOptRules.java  |   2 +
 .../org/apache/calcite/rel/core/AggregateCall.java |   2 +
 .../rel/rules/AggregateCaseToFilterRule.java       | 268 +++++++++++++++++++++
 .../calcite/sql/fun/SqlSingleValueAggFunction.java |   4 +
 .../org/apache/calcite/test/RelOptRulesTest.java   |  18 +-
 .../org/apache/calcite/test/RelOptTestBase.java    |   8 +-
 .../org/apache/calcite/test/RelOptRulesTest.xml    |  30 +++
 core/src/test/resources/sql/agg.iq                 |  32 +++
 8 files changed, 361 insertions(+), 3 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptRules.java b/core/src/main/java/org/apache/calcite/plan/RelOptRules.java
index dcb2f7d..cff4b6c 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptRules.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptRules.java
@@ -22,6 +22,7 @@ import org.apache.calcite.interpreter.NoneToBindableConverterRule;
 import org.apache.calcite.linq4j.function.Experimental;
 import org.apache.calcite.plan.volcano.AbstractConverter;
 import org.apache.calcite.rel.rules.AbstractMaterializedViewRule;
+import org.apache.calcite.rel.rules.AggregateCaseToFilterRule;
 import org.apache.calcite.rel.rules.AggregateExpandDistinctAggregatesRule;
 import org.apache.calcite.rel.rules.AggregateJoinTransposeRule;
 import org.apache.calcite.rel.rules.AggregateMergeRule;
@@ -121,6 +122,7 @@ public class RelOptRules {
       FilterJoinRule.FILTER_ON_JOIN,
       JoinPushExpressionsRule.INSTANCE,
       AggregateExpandDistinctAggregatesRule.INSTANCE,
+      AggregateCaseToFilterRule.INSTANCE,
       AggregateReduceFunctionsRule.INSTANCE,
       FilterAggregateTransposeRule.INSTANCE,
       ProjectWindowTransposeRule.INSTANCE,
diff --git a/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java b/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java
index e95f46a..bb3c109 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java
@@ -26,6 +26,7 @@ import org.apache.calcite.sql.type.SqlTypeUtil;
 import org.apache.calcite.util.mapping.Mapping;
 import org.apache.calcite.util.mapping.Mappings;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 
 import java.util.List;
@@ -100,6 +101,7 @@ public class AggregateCall {
     this.distinct = distinct;
     this.approximate = approximate;
     this.ignoreNulls = ignoreNulls;
+    Preconditions.checkArgument(filterArg < 0 || aggFunction.allowsFilter());
   }
 
   //~ Methods ----------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateCaseToFilterRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateCaseToFilterRule.java
new file mode 100644
index 0000000..77966ec
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateCaseToFilterRule.java
@@ -0,0 +1,268 @@
+/*
+ * 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.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelCollations;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.AggregateCall;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlPostfixOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+
+import com.google.common.collect.ImmutableList;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+
+/**
+ * Rule that converts CASE-style filtered aggregates into true filtered
+ * aggregates.
+ *
+ * <p>For example,
+ *
+ * <blockquote>
+ *   <code>SELECT SUM(CASE WHEN gender = 'F' THEN salary END)<br>
+ *   FROM Emp</code>
+ * </blockquote>
+ *
+ * <p>becomes
+ *
+ * <blockquote>
+ *   <code>SELECT SUM(salary) FILTER (WHERE gender = 'F')<br>
+ *   FROM Emp</code>
+ * </blockquote>
+ */
+public class AggregateCaseToFilterRule extends RelOptRule {
+  public static final AggregateCaseToFilterRule INSTANCE =
+      new AggregateCaseToFilterRule(RelFactories.LOGICAL_BUILDER, null);
+
+  /** Creates an AggregateCaseToFilterRule. */
+  protected AggregateCaseToFilterRule(RelBuilderFactory relBuilderFactory,
+      String description) {
+    super(operand(Aggregate.class, operand(Project.class, any())),
+        relBuilderFactory, description);
+  }
+
+  @Override public boolean matches(final RelOptRuleCall call) {
+    final Aggregate aggregate = call.rel(0);
+    final Project project = call.rel(1);
+
+    for (AggregateCall aggregateCall : aggregate.getAggCallList()) {
+      final int singleArg = soleArgument(aggregateCall);
+      if (singleArg >= 0
+          && isThreeArgCase(project.getProjects().get(singleArg))) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  @Override public void onMatch(RelOptRuleCall call) {
+    final Aggregate aggregate = call.rel(0);
+    final Project project = call.rel(1);
+    final RexBuilder rexBuilder = aggregate.getCluster().getRexBuilder();
+    final List<AggregateCall> newCalls =
+        new ArrayList<>(aggregate.getAggCallList().size());
+    final List<RexNode> newProjects = new ArrayList<>(project.getProjects());
+    final List<RexNode> newCasts = new ArrayList<>();
+
+    for (int fieldNumber : aggregate.getGroupSet()) {
+      newCasts.add(
+          rexBuilder.makeInputRef(
+              project.getProjects().get(fieldNumber).getType(), fieldNumber));
+    }
+
+    for (AggregateCall aggregateCall : aggregate.getAggCallList()) {
+      AggregateCall newCall =
+          transform(aggregateCall, project, newProjects);
+
+      // Possibly CAST the new aggregator to an appropriate type.
+      final int i = newCasts.size();
+      final RelDataType oldType =
+          aggregate.getRowType().getFieldList().get(i).getType();
+      if (newCall == null) {
+        newCalls.add(aggregateCall);
+        newCasts.add(rexBuilder.makeInputRef(oldType, i));
+      } else {
+        newCalls.add(newCall);
+        newCasts.add(
+            rexBuilder.makeCast(oldType,
+                rexBuilder.makeInputRef(newCall.getType(), i)));
+      }
+    }
+
+    if (newCalls.equals(aggregate.getAggCallList())) {
+      return;
+    }
+
+    final RelBuilder relBuilder = call.builder()
+        .push(project.getInput())
+        .project(newProjects);
+
+    final RelBuilder.GroupKey groupKey =
+        relBuilder.groupKey(aggregate.getGroupSet(),
+            aggregate.getGroupSets());
+
+    relBuilder.aggregate(groupKey, newCalls)
+        .convert(aggregate.getRowType(), false);
+
+    call.transformTo(relBuilder.build());
+    call.getPlanner().setImportance(aggregate, 0.0);
+  }
+
+  private @Nullable AggregateCall transform(AggregateCall aggregateCall,
+      Project project, List<RexNode> newProjects) {
+    final int singleArg = soleArgument(aggregateCall);
+    if (singleArg < 0) {
+      return null;
+    }
+
+    final RexNode rexNode = project.getProjects().get(singleArg);
+    if (!isThreeArgCase(rexNode)) {
+      return null;
+    }
+
+    final RelOptCluster cluster = project.getCluster();
+    final RexBuilder rexBuilder = cluster.getRexBuilder();
+    final RexCall caseCall = (RexCall) rexNode;
+
+    // If one arg is null and the other is not, reverse them and set "flip",
+    // which negates the filter.
+    final boolean flip = RexLiteral.isNullLiteral(caseCall.operands.get(1))
+        && !RexLiteral.isNullLiteral(caseCall.operands.get(2));
+    final RexNode arg1 = caseCall.operands.get(flip ? 2 : 1);
+    final RexNode arg2 = caseCall.operands.get(flip ? 1 : 2);
+
+    // Operand 1: Filter
+    final SqlPostfixOperator op =
+        flip ? SqlStdOperatorTable.IS_FALSE : SqlStdOperatorTable.IS_TRUE;
+    final RexNode filterFromCase =
+        rexBuilder.makeCall(op, caseCall.operands.get(0));
+
+    // Combine the CASE filter with an honest-to-goodness SQL FILTER, if the
+    // latter is present.
+    final RexNode filter;
+    if (aggregateCall.filterArg >= 0) {
+      filter = rexBuilder.makeCall(SqlStdOperatorTable.AND,
+          project.getProjects().get(aggregateCall.filterArg), filterFromCase);
+    } else {
+      filter = filterFromCase;
+    }
+
+    final SqlKind kind = aggregateCall.getAggregation().getKind();
+    if (aggregateCall.isDistinct()) {
+      // Just one style supported:
+      //   COUNT(DISTINCT CASE WHEN x = 'foo' THEN y END)
+      // =>
+      //   COUNT(DISTINCT y) FILTER(WHERE x = 'foo')
+
+      if (kind == SqlKind.COUNT
+          && RexLiteral.isNullLiteral(arg2)) {
+        newProjects.add(arg1);
+        newProjects.add(filter);
+        return AggregateCall.create(SqlStdOperatorTable.COUNT, true, false,
+            false, ImmutableList.of(newProjects.size() - 2),
+            newProjects.size() - 1, RelCollations.EMPTY,
+            aggregateCall.getType(), aggregateCall.getName());
+      }
+      return null;
+    }
+
+    // Four styles supported:
+    //
+    // A1: AGG(CASE WHEN x = 'foo' THEN cnt END)
+    //   => operands (x = 'foo', cnt, null)
+    // A2: SUM(CASE WHEN x = 'foo' THEN cnt ELSE 0 END)
+    //   => operands (x = 'foo', cnt, 0); must be SUM
+    // B: SUM(CASE WHEN x = 'foo' THEN 1 ELSE 0 END)
+    //   => operands (x = 'foo', 1, 0); must be SUM
+    // C: COUNT(CASE WHEN x = 'foo' THEN 'dummy' END)
+    //   => operands (x = 'foo', 'dummy', null)
+
+    if (kind == SqlKind.COUNT // Case C
+        && arg1.isA(SqlKind.LITERAL)
+        && !RexLiteral.isNullLiteral(arg1)
+        && RexLiteral.isNullLiteral(arg2)) {
+      newProjects.add(filter);
+      return AggregateCall.create(SqlStdOperatorTable.COUNT, false, false,
+          false, ImmutableList.of(), newProjects.size() - 1,
+          RelCollations.EMPTY, aggregateCall.getType(),
+          aggregateCall.getName());
+    } else if (kind == SqlKind.SUM // Case B
+        && isIntLiteral(arg1) && RexLiteral.intValue(arg1) == 1
+        && isIntLiteral(arg2) && RexLiteral.intValue(arg2) == 0) {
+
+      newProjects.add(filter);
+      final RelDataTypeFactory typeFactory = cluster.getTypeFactory();
+      final RelDataType dataType =
+          typeFactory.createTypeWithNullability(
+              typeFactory.createSqlType(SqlTypeName.BIGINT), false);
+      return AggregateCall.create(SqlStdOperatorTable.COUNT, false, false,
+          false, ImmutableList.of(), newProjects.size() - 1,
+          RelCollations.EMPTY, dataType, aggregateCall.getName());
+    } else if ((RexLiteral.isNullLiteral(arg2) // Case A1
+            && aggregateCall.getAggregation().allowsFilter())
+        || (kind == SqlKind.SUM // Case A2
+            && isIntLiteral(arg2)
+            && RexLiteral.intValue(arg2) == 0)) {
+      newProjects.add(arg1);
+      newProjects.add(filter);
+      return AggregateCall.create(aggregateCall.getAggregation(), false,
+          false, false, ImmutableList.of(newProjects.size() - 2),
+          newProjects.size() - 1, RelCollations.EMPTY,
+          aggregateCall.getType(), aggregateCall.getName());
+    } else {
+      return null;
+    }
+  }
+
+  /** Returns the argument, if an aggregate call has a single argument,
+   * otherwise -1. */
+  private static int soleArgument(AggregateCall aggregateCall) {
+    return aggregateCall.getArgList().size() == 1
+        ? aggregateCall.getArgList().get(0)
+        : -1;
+  }
+
+  private static boolean isThreeArgCase(final RexNode rexNode) {
+    return rexNode.getKind() == SqlKind.CASE
+        && ((RexCall) rexNode).operands.size() == 3;
+  }
+
+  private static boolean isIntLiteral(final RexNode rexNode) {
+    return rexNode instanceof RexLiteral
+        && SqlTypeName.INT_TYPES.contains(rexNode.getType().getSqlTypeName());
+  }
+}
+
+// End AggregateCaseToFilterRule.java
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlSingleValueAggFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlSingleValueAggFunction.java
index 6f4518e..8eb52b7 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlSingleValueAggFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlSingleValueAggFunction.java
@@ -59,6 +59,10 @@ public class SqlSingleValueAggFunction extends SqlAggFunction {
 
   //~ Methods ----------------------------------------------------------------
 
+  @Override public boolean allowsFilter() {
+    return false;
+  }
+
   @SuppressWarnings("deprecation")
   public List<RelDataType> getParameterTypes(RelDataTypeFactory typeFactory) {
     return ImmutableList.of(type);
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 756d223..8e97702 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -61,6 +61,7 @@ import org.apache.calcite.rel.metadata.CachingRelMetadataProvider;
 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.rel.rules.AggregateCaseToFilterRule;
 import org.apache.calcite.rel.rules.AggregateExpandDistinctAggregatesRule;
 import org.apache.calcite.rel.rules.AggregateExtractProjectRule;
 import org.apache.calcite.rel.rules.AggregateFilterTransposeRule;
@@ -3297,6 +3298,21 @@ public class RelOptRulesTest extends RelOptTestBase {
     sql(sql).withPre(pre).withRule(rule).checkUnchanged();
   }
 
+  @Test public void testAggregateCaseToFilter() {
+    final String sql = "select\n"
+        + " sum(sal) as sum_sal,\n"
+        + " count(distinct case\n"
+        + "       when job = 'CLERK'\n"
+        + "       then deptno else null end) as count_distinct_clerk,\n"
+        + " sum(case when deptno = 10 then sal end) as sum_sal_d10,\n"
+        + " sum(case when deptno = 20 then sal else 0 end) as sum_sal_d20,\n"
+        + " sum(case when deptno = 30 then 1 else 0 end) as count_d30,\n"
+        + " count(case when deptno = 40 then 'x' end) as count_d40,\n"
+        + " count(case when deptno = 20 then 1 end) as count_d20\n"
+        + "from emp";
+    sql(sql).withRule(AggregateCaseToFilterRule.INSTANCE).check();
+  }
+
   @Test public void testPullAggregateThroughUnion() {
     HepProgram program = new HepProgramBuilder()
         .addRuleInstance(AggregateUnionAggregateRule.INSTANCE)
@@ -5256,7 +5272,7 @@ public class RelOptRulesTest extends RelOptTestBase {
 
   /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-3121">[CALCITE-3121]
-   * VolcanoPlanner hangs due to subquery with dynamic star</a>. */
+   * VolcanoPlanner hangs due to sub-query with dynamic star</a>. */
   @Test public void testSubQueryWithDynamicStarHang() {
     String sql = "select n.n_regionkey from (select * from "
         + "(select * from sales.customer) t) n where n.n_nationkey >1";
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 507edd2..bf263c9 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java
@@ -262,8 +262,12 @@ abstract class RelOptTestBase extends SqlToRelTestBase {
           transforms);
     }
 
-    public Sql withRule(RelOptRule rule) {
-      return with(HepProgram.builder().addRuleInstance(rule).build());
+    public Sql withRule(RelOptRule... rules) {
+      final HepProgramBuilder builder = HepProgram.builder();
+      for (RelOptRule rule : rules) {
+        builder.addRuleInstance(rule);
+      }
+      return with(builder.build());
     }
 
     /** Adds a transform that will be applied to {@link #tester}
diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index d2d83e0..288dfce 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -16,6 +16,36 @@ See the License for the specific language governing permissions and
 limitations under the License.
 -->
 <Root>
+    <TestCase name="testAggregateCaseToFilter">
+        <Resource name="sql">
+            <![CDATA[select
+ sum(sal) as sum_sal,
+ count(distinct case
+       when job = 'CLERK'
+       then deptno else null end) as count_distinct_clerk,
+ sum(case when deptno = 10 then sal end) as sum_sal_d10,
+ sum(case when deptno = 20 then sal else 0 end) as sum_sal_d20,
+ sum(case when deptno = 30 then 1 else 0 end) as count_d30,
+ count(case when deptno = 40 then 'x' end) as count_d40,
+ count(case when deptno = 20 then 1 end) as count_d20
+from emp]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalAggregate(group=[{}], SUM_SAL=[SUM($0)], COUNT_DISTINCT_CLERK=[COUNT(DISTINCT $1)], SUM_SAL_D10=[SUM($2)], SUM_SAL_D20=[SUM($3)], COUNT_D30=[SUM($4)], COUNT_D40=[COUNT($5)], COUNT_D20=[COUNT($6)])
+  LogicalProject(SAL=[$5], $f1=[CASE(=($2, 'CLERK'), $7, null:INTEGER)], $f2=[CASE(=($7, 10), $5, null:INTEGER)], $f3=[CASE(=($7, 20), $5, 0)], $f4=[CASE(=($7, 30), 1, 0)], $f5=[CASE(=($7, 40), 'x', null:CHAR(1))], $f6=[CASE(=($7, 20), 1, null:INTEGER)])
+    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalProject(SUM_SAL=[$0], COUNT_DISTINCT_CLERK=[$1], SUM_SAL_D10=[$2], SUM_SAL_D20=[$3], COUNT_D30=[CAST($4):INTEGER], COUNT_D40=[$5], COUNT_D20=[$6])
+  LogicalAggregate(group=[{}], SUM_SAL=[SUM($0)], COUNT_DISTINCT_CLERK=[COUNT(DISTINCT $7) FILTER $8], SUM_SAL_D10=[SUM($9) FILTER $10], SUM_SAL_D20=[SUM($11) FILTER $12], COUNT_D30=[COUNT() FILTER $13], COUNT_D40=[COUNT() FILTER $14], COUNT_D20=[COUNT() FILTER $15])
+    LogicalProject(SAL=[$5], $f1=[CASE(=($2, 'CLERK'), $7, null:INTEGER)], $f2=[CASE(=($7, 10), $5, null:INTEGER)], $f3=[CASE(=($7, 20), $5, 0)], $f4=[CASE(=($7, 30), 1, 0)], $f5=[CASE(=($7, 40), 'x', null:CHAR(1))], $f6=[CASE(=($7, 20), 1, null:INTEGER)], DEPTNO=[$7], $f8=[=($2, 'CLERK')], SAL0=[$5], $f10=[=($7, 10)], SAL1=[$5], $f12=[=($7, 20)], $f13=[=($7, 30)], $f14=[=($7, 40)], $f15=[=($7, 20)])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
     <TestCase name="testAggregateExtractProjectRule">
         <Resource name="sql">
             <![CDATA[select sum(sal)
diff --git a/core/src/test/resources/sql/agg.iq b/core/src/test/resources/sql/agg.iq
index 550e36f..3f939f7 100644
--- a/core/src/test/resources/sql/agg.iq
+++ b/core/src/test/resources/sql/agg.iq
@@ -2548,6 +2548,38 @@ select deptno, bit_and(empno), bit_or(empno) from "scott".emp group by deptno;
 
 !ok
 
+# Based on [DRUID-7593] Exact distinct-COUNT with complex expression (CASE, IN) throws
+# NullPointerException
+WITH wikipedia AS (
+  SELECT empno AS delta,
+    CASE WHEN deptno = 10 THEN 'true' ELSE 'false' END AS isRobot,
+    ename AS "user"
+  FROM "scott".emp)
+SELECT COUNT(DISTINCT
+    CASE WHEN (((CASE WHEN wikipedia.delta IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
+                      THEN REPLACE('Yes', 'Yes', 'Yes')
+                      ELSE REPLACE('No', 'No', 'No') END) = 'No'))
+         AND (wikipedia.isRobot = 'true')
+         THEN (wikipedia."user")
+         ELSE NULL END)
+  - (MAX(CASE WHEN (((CASE WHEN wikipedia.delta IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
+                           THEN REPLACE('Yes', 'Yes', 'Yes')
+                           ELSE REPLACE('No', 'No', 'No') END) = 'No'))
+              AND (wikipedia.isRobot = 'true')
+              THEN NULL
+              ELSE -9223372036854775807 END)
+       + 9223372036854775807 + 1) AS "wikipedia.count_distinct_filters_that_dont_work"
+FROM wikipedia
+LIMIT 500;
++-------------------------------------------------+
+| wikipedia.count_distinct_filters_that_dont_work |
++-------------------------------------------------+
+|                                               2 |
++-------------------------------------------------+
+(1 row)
+
+!ok
+
 # [CALCITE-2266] JSON_OBJECTAGG, JSON_ARRAYAGG
 !use post
 


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

Posted by jh...@apache.org.
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() {


[calcite] 01/06: [CALCITE-3183] During field trimming, Filter is copied with wrong traitSet (Juhwan Kim)

Posted by jh...@apache.org.
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 bb0bae9c047db0f4bebaa6cd5b149876476e6ebe
Author: Juhwan Kim <jk...@dremio.com>
AuthorDate: Fri Jul 12 10:19:10 2019 -0700

    [CALCITE-3183] During field trimming, Filter is copied with wrong traitSet (Juhwan Kim)
    
    New Filter created after trimming uses its old trait set,
    which could cause problem if there has been any update on collation.
    
    Code changes:
    - Add a FilterFactory method that includes correlating variables;
    - Use RelBuilder when creating a new Filter in trimmer;
    - Add test cases for the bug and new RelBuilder method.
    
    Close apache/calcite#1309
---
 .../calcite/adapter/enumerable/EnumerableRel.java  |  9 ++-
 .../org/apache/calcite/adapter/jdbc/JdbcRules.java |  9 ++-
 .../java/org/apache/calcite/plan/RelOptUtil.java   | 11 ++--
 .../org/apache/calcite/rel/core/RelFactories.java  | 30 ++++++++--
 .../rel/rules/SemiJoinFilterTransposeRule.java     |  3 +-
 .../apache/calcite/sql2rel/RelFieldTrimmer.java    |  6 +-
 .../apache/calcite/sql2rel/SqlToRelConverter.java  |  5 +-
 .../java/org/apache/calcite/tools/RelBuilder.java  | 28 +++++++++-
 .../org/apache/calcite/test/RelBuilderTest.java    | 49 +++++++++++++++++
 .../apache/calcite/test/SqlToRelConverterTest.java | 64 ++++++++++++++++++++++
 core/src/test/resources/sql/misc.iq                |  4 +-
 .../adapter/geode/rel/GeodeAllDataTypesTest.java   |  4 +-
 .../calcite/adapter/pig/PigRelFactories.java       | 12 +++-
 site/_docs/algebra.md                              |  4 +-
 14 files changed, 209 insertions(+), 29 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRel.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRel.java
index 18f2594..fd80af7 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRel.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRel.java
@@ -20,6 +20,8 @@ import org.apache.calcite.linq4j.tree.BlockStatement;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.RelFactories;
 
+import com.google.common.base.Preconditions;
+
 /**
  * A relational expression of one of the
  * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention} calling
@@ -27,7 +29,12 @@ import org.apache.calcite.rel.core.RelFactories;
  */
 public interface EnumerableRel
     extends RelNode {
-  RelFactories.FilterFactory FILTER_FACTORY = EnumerableFilter::create;
+  RelFactories.FilterFactory FILTER_FACTORY =
+      (input, condition, variablesSet) -> {
+        Preconditions.checkArgument(variablesSet.isEmpty(),
+            "EnumerableFilter does not allow variables");
+        return EnumerableFilter.create(input, condition);
+      };
 
   RelFactories.ProjectFactory PROJECT_FACTORY = EnumerableProject::create;
 
diff --git a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
index b46835f..7e3e0da 100644
--- a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
+++ b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
@@ -74,6 +74,7 @@ import org.apache.calcite.util.ImmutableBitSet;
 import org.apache.calcite.util.Util;
 import org.apache.calcite.util.trace.CalciteTrace;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 
 import org.slf4j.Logger;
@@ -105,8 +106,12 @@ public class JdbcRules {
       };
 
   static final RelFactories.FilterFactory FILTER_FACTORY =
-      (input, condition) -> new JdbcRules.JdbcFilter(input.getCluster(),
-          input.getTraitSet(), input, condition);
+      (input, condition, variablesSet) -> {
+        Preconditions.checkArgument(variablesSet.isEmpty(),
+            "JdbcFilter does not allow variables");
+        return new JdbcFilter(input.getCluster(),
+            input.getTraitSet(), input, condition);
+      };
 
   static final RelFactories.JoinFactory JOIN_FACTORY =
       (left, right, condition, variablesSet, joinType, semiJoinDone) -> {
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index b795a42..5e8500c 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -104,6 +104,7 @@ import org.apache.calcite.util.mapping.MappingType;
 import org.apache.calcite.util.mapping.Mappings;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Lists;
@@ -453,7 +454,7 @@ public abstract class RelOptUtil {
 
       final RelFactories.FilterFactory factory =
           RelFactories.DEFAULT_FILTER_FACTORY;
-      ret = factory.createFilter(ret, conditionExp);
+      ret = factory.createFilter(ret, conditionExp, ImmutableSet.of());
     }
 
     if (extraExpr != null) {
@@ -602,13 +603,13 @@ public abstract class RelOptUtil {
   public static RelNode createFilter(RelNode child, RexNode condition) {
     final RelFactories.FilterFactory factory =
         RelFactories.DEFAULT_FILTER_FACTORY;
-    return factory.createFilter(child, condition);
+    return factory.createFilter(child, condition, ImmutableSet.of());
   }
 
   @Deprecated // to be removed before 2.0
   public static RelNode createFilter(RelNode child, RexNode condition,
       RelFactories.FilterFactory filterFactory) {
-    return filterFactory.createFilter(child, condition);
+    return filterFactory.createFilter(child, condition, ImmutableSet.of());
   }
 
   /** Creates a filter, using the default filter factory,
@@ -631,7 +632,7 @@ public abstract class RelOptUtil {
     if (condition == null) {
       return child;
     } else {
-      return filterFactory.createFilter(child, condition);
+      return filterFactory.createFilter(child, condition, ImmutableSet.of());
     }
   }
 
@@ -681,7 +682,7 @@ public abstract class RelOptUtil {
 
     final RelFactories.FilterFactory factory =
         RelFactories.DEFAULT_FILTER_FACTORY;
-    return factory.createFilter(rel, condition);
+    return factory.createFilter(rel, condition, ImmutableSet.of());
   }
 
   /**
diff --git a/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java b/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
index 728bc20..ff8af92 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
@@ -297,12 +297,30 @@ public class RelFactories {
   }
 
   /**
-   * Can create a {@link LogicalFilter} of the appropriate type
+   * Can create a {@link Filter} of the appropriate type
    * for this rule's calling convention.
    */
   public interface FilterFactory {
-    /** Creates a filter. */
-    RelNode createFilter(RelNode input, RexNode condition);
+    /** Creates a filter.
+     *
+     * <p>Some implementations of {@code Filter} do not support correlation
+     * variables, and for these, this method will throw if {@code variablesSet}
+     * is not empty.
+     *
+     * @param input Input relational expression
+     * @param condition Filter condition; only rows for which this condition
+     *   evaluates to TRUE will be emitted
+     * @param variablesSet Correlating variables that are set when reading
+     *   a row from the input, and which may be referenced from inside the
+     *   condition
+     */
+    RelNode createFilter(RelNode input, RexNode condition,
+        Set<CorrelationId> variablesSet);
+
+    @Deprecated // to be removed before 2.0
+    default RelNode createFilter(RelNode input, RexNode condition) {
+      return createFilter(input, condition, ImmutableSet.of());
+    }
   }
 
   /**
@@ -310,8 +328,10 @@ public class RelFactories {
    * returns a vanilla {@link LogicalFilter}.
    */
   private static class FilterFactoryImpl implements FilterFactory {
-    public RelNode createFilter(RelNode input, RexNode condition) {
-      return LogicalFilter.create(input, condition);
+    public RelNode createFilter(RelNode input, RexNode condition,
+        Set<CorrelationId> variablesSet) {
+      return LogicalFilter.create(input, condition,
+          ImmutableSet.copyOf(variablesSet));
     }
   }
 
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/SemiJoinFilterTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/SemiJoinFilterTransposeRule.java
index 16dc13c..2f2f3ca 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/SemiJoinFilterTransposeRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/SemiJoinFilterTransposeRule.java
@@ -72,7 +72,8 @@ public class SemiJoinFilterTransposeRule extends RelOptRule {
     final RelFactories.FilterFactory factory =
         RelFactories.DEFAULT_FILTER_FACTORY;
     RelNode newFilter =
-        factory.createFilter(newSemiJoin, filter.getCondition());
+        factory.createFilter(newSemiJoin, filter.getCondition(),
+            ImmutableSet.of());
 
     call.transformTo(newFilter);
   }
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
index b92d5a1..2d01a96 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
@@ -482,9 +482,9 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
     RexNode newConditionExpr =
         conditionExpr.accept(shuttle);
 
-    // Use copy rather than relBuilder so that correlating variables get set.
-    relBuilder.push(
-        filter.copy(filter.getTraitSet(), newInput, newConditionExpr));
+    // Build new filter with trimmed input and condition.
+    relBuilder.push(newInput)
+        .filter(filter.getVariablesSet(), newConditionExpr);
 
     // The result has the same mapping as the input gave us. Sometimes we
     // return fields that the consumer didn't ask for, because the filter
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
index 7668ee8..fe490a0 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -989,7 +989,8 @@ public class SqlToRelConverter {
 
     final RelFactories.FilterFactory filterFactory =
         RelFactories.DEFAULT_FILTER_FACTORY;
-    final RelNode filter = filterFactory.createFilter(bb.root, convertedWhere2);
+    final RelNode filter =
+        filterFactory.createFilter(bb.root, convertedWhere2, ImmutableSet.of());
     final RelNode r;
     final CorrelationUse p = getCorrelationUse(bb, filter);
     if (p != null) {
@@ -2490,7 +2491,7 @@ public class SqlToRelConverter {
         // Replace outer RexInputRef with RexFieldAccess,
         // and push lateral join predicate into inner child
         final RexNode newCond = joinCond.accept(shuttle);
-        innerRel = factory.createFilter(p.r, newCond);
+        innerRel = factory.createFilter(p.r, newCond, ImmutableSet.of());
         requiredCols = ImmutableBitSet
             .fromBitSet(shuttle.varCols)
             .union(p.requiredColumns);
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 c8545ed..7058e50 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -1146,7 +1146,7 @@ public class RelBuilder {
    * and optimized in a similar way to the {@link #and} method.
    * If the result is TRUE no filter is created. */
   public RelBuilder filter(RexNode... predicates) {
-    return filter(ImmutableList.copyOf(predicates));
+    return filter(ImmutableSet.of(), ImmutableList.copyOf(predicates));
   }
 
   /** Creates a {@link Filter} of a list of
@@ -1156,6 +1156,29 @@ public class RelBuilder {
    * and optimized in a similar way to the {@link #and} method.
    * If the result is TRUE no filter is created. */
   public RelBuilder filter(Iterable<? extends RexNode> predicates) {
+    return filter(ImmutableSet.of(), predicates);
+  }
+
+  /** Creates a {@link Filter} of a list of correlation variables
+   * and an array of predicates.
+   *
+   * <p>The predicates are combined using AND,
+   * and optimized in a similar way to the {@link #and} method.
+   * If the result is TRUE no filter is created. */
+  public RelBuilder filter(Iterable<CorrelationId> variablesSet,
+      RexNode... predicates) {
+    return filter(variablesSet, ImmutableList.copyOf(predicates));
+  }
+
+  /**
+   * Creates a {@link Filter} of a list of correlation variables
+   * and a list of predicates.
+   *
+   * <p>The predicates are combined using AND,
+   * and optimized in a similar way to the {@link #and} method.
+   * If the result is TRUE no filter is created. */
+  public RelBuilder filter(Iterable<CorrelationId> variablesSet,
+      Iterable<? extends RexNode> predicates) {
     final RexNode simplifiedPredicates =
         simplifier.simplifyFilterPredicates(predicates);
     if (simplifiedPredicates == null) {
@@ -1164,7 +1187,8 @@ public class RelBuilder {
 
     if (!simplifiedPredicates.isAlwaysTrue()) {
       final Frame frame = stack.pop();
-      final RelNode filter = filterFactory.createFilter(frame.rel, simplifiedPredicates);
+      final RelNode filter = filterFactory.createFilter(frame.rel,
+          simplifiedPredicates, ImmutableSet.copyOf(variablesSet));
       stack.push(new Frame(filter, frame.fields));
     }
     return this;
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 a71ce05..29e42c2 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -25,6 +25,7 @@ import org.apache.calcite.rel.RelDistributions;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.AggregateCall;
 import org.apache.calcite.rel.core.Correlate;
+import org.apache.calcite.rel.core.CorrelationId;
 import org.apache.calcite.rel.core.Exchange;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.core.RelFactories;
@@ -2587,6 +2588,54 @@ public class RelBuilderTest {
     assertThat(root, hasTree(expected));
   }
 
+  /** Tests filter builder with correlation variables */
+  @Test public void testFilterWithCorrelationVariables() {
+    final RelBuilder builder = RelBuilder.create(config().build());
+    final Holder<RexCorrelVariable> v = Holder.of(null);
+    RelNode root = builder.scan("EMP")
+        .variable(v)
+        .scan("DEPT")
+        .filter(Collections.singletonList(v.get().id),
+            builder.call(SqlStdOperatorTable.OR,
+                builder.call(SqlStdOperatorTable.AND,
+                    builder.call(SqlStdOperatorTable.LESS_THAN,
+                        builder.field(v.get(), "DEPTNO"),
+                        builder.literal(30)),
+                    builder.call(SqlStdOperatorTable.GREATER_THAN,
+                        builder.field(v.get(), "DEPTNO"),
+                        builder.literal(20))),
+                builder.isNull(builder.field(2))))
+        .join(JoinRelType.LEFT,
+            builder.equals(builder.field(2, 0, "SAL"),
+                builder.literal(1000)),
+            ImmutableSet.of(v.get().id))
+        .build();
+
+    final String expected = ""
+        + "LogicalCorrelate(correlation=[$cor0], joinType=[left], requiredColumns=[{7}])\n"
+        + "  LogicalTableScan(table=[[scott, EMP]])\n"
+        + "  LogicalFilter(condition=[=($cor0.SAL, 1000)])\n"
+        + "    LogicalFilter(condition=[OR(AND(<($cor0.DEPTNO, 30), >($cor0.DEPTNO, 20)), "
+        + "IS NULL($2))], variablesSet=[[$cor0]])\n"
+        + "      LogicalTableScan(table=[[scott, DEPT]])\n";
+
+    assertThat(root, hasTree(expected));
+  }
+
+  @Test public void testFilterEmpty() {
+    final RelBuilder builder = RelBuilder.create(config().build());
+    final RelNode root =
+        builder.scan("EMP")
+            // We intend to call
+            //   filter(Iterable<CorrelationId>, RexNode...)
+            // with zero varargs, not
+            //   filter(Iterable<RexNode>)
+            // Let's hope they're distinct after type erasure.
+            .filter(ImmutableSet.<CorrelationId>of())
+            .build();
+    assertThat(root, hasTree("LogicalTableScan(table=[[scott, EMP]])\n"));
+  }
+
   @Test public void testRelBuilderToString() {
     final RelBuilder builder = RelBuilder.create(config().build());
     builder.scan("EMP");
diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index 3499657..a7d2ae5 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -20,12 +20,20 @@ import org.apache.calcite.config.CalciteConnectionConfigImpl;
 import org.apache.calcite.config.CalciteConnectionProperty;
 import org.apache.calcite.config.NullCollation;
 import org.apache.calcite.plan.Contexts;
+import org.apache.calcite.plan.RelOptPlanner;
 import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.plan.RelTrait;
+import org.apache.calcite.plan.RelTraitDef;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollationTraitDef;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.RelRoot;
+import org.apache.calcite.rel.RelShuttleImpl;
 import org.apache.calcite.rel.RelVisitor;
 import org.apache.calcite.rel.core.CorrelationId;
 import org.apache.calcite.rel.externalize.RelXmlWriter;
+import org.apache.calcite.rel.logical.LogicalFilter;
+import org.apache.calcite.rel.logical.LogicalSort;
 import org.apache.calcite.sql.SqlExplainLevel;
 import org.apache.calcite.sql.validate.SqlConformance;
 import org.apache.calcite.sql.validate.SqlConformanceEnum;
@@ -37,6 +45,7 @@ import org.apache.calcite.util.Litmus;
 import org.apache.calcite.util.TestUtil;
 import org.apache.calcite.util.Util;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 
 import org.junit.Ignore;
@@ -45,12 +54,16 @@ import org.junit.Test;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.ArrayDeque;
+import java.util.ArrayList;
 import java.util.Deque;
+import java.util.List;
 import java.util.Properties;
 import java.util.Set;
 
 import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Unit test for {@link org.apache.calcite.sql2rel.SqlToRelConverter}.
@@ -1926,6 +1939,57 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
     sql(sql).trim(true).ok();
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-3183">[CALCITE-3183]
+   * Trimming method for Filter rel uses wrong traitSet </a> */
+  @Test public void testFilterAndSortWithTrim() {
+    // Create a customized test with RelCollation trait in the test cluster.
+    Tester tester = new TesterImpl(getDiffRepos(),
+        false, true,
+        true, false,
+        null, null) {
+      @Override public RelOptPlanner createPlanner() {
+        return new MockRelOptPlanner(Contexts.empty()) {
+          @Override public List<RelTraitDef> getRelTraitDefs() {
+            return ImmutableList.<RelTraitDef>of(RelCollationTraitDef.INSTANCE);
+          }
+          @Override public RelTraitSet emptyTraitSet() {
+            return RelTraitSet.createEmpty().plus(
+                RelCollationTraitDef.INSTANCE.getDefault());
+          }
+        };
+      }
+    };
+
+    // Run query and save plan after trimming
+    final String sql = "select count(a.EMPNO)\n"
+        + "from (select * from emp order by sal limit 3) a\n"
+        + "where a.EMPNO > 10 group by 2";
+    RelNode afterTrim = tester.convertSqlToRel(sql).rel;
+
+    // Get Sort and Filter operators
+    final List<RelNode> rels = new ArrayList<>();
+    final RelShuttleImpl visitor = new RelShuttleImpl() {
+      @Override public RelNode visit(LogicalSort sort) {
+        rels.add(sort);
+        return super.visit(sort);
+      }
+      @Override public RelNode visit(LogicalFilter filter) {
+        rels.add(filter);
+        return super.visit(filter);
+      }
+    };
+    visitor.visit(afterTrim);
+
+    // Ensure sort and filter operators have consistent traitSet after trimming
+    assertThat(rels.size(), is(2));
+    RelTrait filterCollation = rels.get(0).getTraitSet()
+        .getTrait(RelCollationTraitDef.INSTANCE);
+    RelTrait sortCollation = rels.get(1).getTraitSet()
+        .getTrait(RelCollationTraitDef.INSTANCE);
+    assertTrue(filterCollation.satisfies(sortCollation));
+  }
+
   @Test public void testOffset0() {
     final String sql = "select * from emp offset 0";
     sql(sql).ok();
diff --git a/core/src/test/resources/sql/misc.iq b/core/src/test/resources/sql/misc.iq
index 78a6c1c..a7127ac 100644
--- a/core/src/test/resources/sql/misc.iq
+++ b/core/src/test/resources/sql/misc.iq
@@ -355,7 +355,7 @@ where not exists (select 1 from "hr"."emps");
 (0 rows)
 
 !ok
-EnumerableCalc(expr#0..1=[{inputs}], expr#2=[IS NOT NULL($t1)], expr#3=[NOT($t2)], deptno=[$t0], $condition=[$t3])
+EnumerableCalc(expr#0..1=[{inputs}], expr#2=[IS NULL($t1)], deptno=[$t0], $condition=[$t2])
   EnumerableHashJoin(condition=[true], joinType=[left])
     EnumerableCalc(expr#0..3=[{inputs}], deptno=[$t0])
       EnumerableTableScan(table=[[hr, depts]])
@@ -397,7 +397,7 @@ where not exists (select 1 from "hr"."emps" where "empid" < 0);
 (3 rows)
 
 !ok
-EnumerableCalc(expr#0..1=[{inputs}], expr#2=[IS NOT NULL($t1)], expr#3=[NOT($t2)], deptno=[$t0], $condition=[$t3])
+EnumerableCalc(expr#0..1=[{inputs}], expr#2=[IS NULL($t1)], deptno=[$t0], $condition=[$t2])
   EnumerableHashJoin(condition=[true], joinType=[left])
     EnumerableCalc(expr#0..3=[{inputs}], deptno=[$t0])
       EnumerableTableScan(table=[[hr, depts]])
diff --git a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java
index 7b7433e..67c2e6e 100644
--- a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java
+++ b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java
@@ -323,11 +323,11 @@ public class GeodeAllDataTypesTest extends AbstractGeodeTest {
         .queryContains(
             GeodeAssertions.query("SELECT stringValue AS stringValue "
                 + "FROM /allDataTypesRegion WHERE "
-                + "stringValue IN SET('abc', 'def') OR floatValue IN SET(1.5678, null) OR dateValue "
+                + "stringValue IN SET('abc', 'def') OR floatValue = 1.5678 OR dateValue "
                 + "IN SET(DATE '2018-02-05', DATE '2018-02-06') OR timeValue "
                 + "IN SET(TIME '03:22:23', TIME '07:22:23') OR timestampValue "
                 + "IN SET(TIMESTAMP '2018-02-05 04:22:33', TIMESTAMP '2017-02-05 04:22:33') "
-                + "OR booleanValue IN SET(true, false, null)"));
+                + "OR booleanValue = true OR booleanValue = false"));
   }
 }
 
diff --git a/pig/src/main/java/org/apache/calcite/adapter/pig/PigRelFactories.java b/pig/src/main/java/org/apache/calcite/adapter/pig/PigRelFactories.java
index eaeed00..c64d1fb 100644
--- a/pig/src/main/java/org/apache/calcite/adapter/pig/PigRelFactories.java
+++ b/pig/src/main/java/org/apache/calcite/adapter/pig/PigRelFactories.java
@@ -20,6 +20,7 @@ import org.apache.calcite.plan.Context;
 import org.apache.calcite.plan.Contexts;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.plan.RelTraitSet;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.AggregateCall;
 import org.apache.calcite.rel.core.CorrelationId;
@@ -28,6 +29,7 @@ import org.apache.calcite.rel.core.RelFactories;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.util.ImmutableBitSet;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 
 import java.util.List;
@@ -70,9 +72,13 @@ public class PigRelFactories {
 
     public static final PigFilterFactory INSTANCE = new PigFilterFactory();
 
-    @Override public RelNode createFilter(RelNode input, RexNode condition) {
-      return new PigFilter(input.getCluster(), input.getTraitSet().replace(PigRel.CONVENTION),
-          input, condition);
+    @Override public RelNode createFilter(RelNode input, RexNode condition,
+        Set<CorrelationId> variablesSet) {
+      Preconditions.checkArgument(variablesSet.isEmpty(),
+          "PigFilter does not allow variables");
+      final RelTraitSet traitSet =
+          input.getTraitSet().replace(PigRel.CONVENTION);
+      return new PigFilter(input.getCluster(), traitSet, input, condition);
     }
   }
 
diff --git a/site/_docs/algebra.md b/site/_docs/algebra.md
index ba07531..f8294d7 100644
--- a/site/_docs/algebra.md
+++ b/site/_docs/algebra.md
@@ -309,7 +309,7 @@ return the `RelBuilder`.
 | `functionScan(operator, n, expr...)`<br/>`functionScan(operator, n, exprList)` | Creates a [TableFunctionScan]({{ site.apiRoot }}/org/apache/calcite/rel/core/TableFunctionScan.html) of the `n` most recent relational expressions.
 | `transientScan(tableName [, rowType])` | Creates a [TableScan]({{ site.apiRoot }}/org/apache/calcite/rel/core/TableScan.html) on a [TransientTable]]({{ site.apiRoot }}/org/apache/calcite/schema/TransientTable.html) with the given type (if not specified, the most recent relational expression's type will be used).
 | `values(fieldNames, value...)`<br/>`values(rowType, tupleList)` | Creates a [Values]({{ site.apiRoot }}/org/apache/calcite/rel/core/Values.html).
-| `filter(expr...)`<br/>`filter(exprList)` | Creates a [Filter]({{ site.apiRoot }}/org/apache/calcite/rel/core/Filter.html) over the AND of the given predicates.
+| `filter([variablesSet, ] exprList)`<br/>`filter([variablesSet, ] expr...)` | Creates a [Filter]({{ site.apiRoot }}/org/apache/calcite/rel/core/Filter.html) over the AND of the given predicates; if `variablesSet` is specified, the predicates may reference those variables.
 | `project(expr...)`<br/>`project(exprList [, fieldNames])` | Creates a [Project]({{ site.apiRoot }}/org/apache/calcite/rel/core/Project.html). To override the default name, wrap expressions using `alias`, or specify the `fieldNames` argument.
 | `projectPlus(expr...)`<br/>`projectPlus(exprList)` | Variant of `project` that keeps original fields and appends the given expressions.
 | `permute(mapping)` | Creates a [Project]({{ site.apiRoot }}/org/apache/calcite/rel/core/Project.html) that permutes the fields using `mapping`.
@@ -350,6 +350,8 @@ Argument types:
 * `tupleList` Iterable of List of [RexLiteral]({{ site.apiRoot }}/org/apache/calcite/rex/RexLiteral.html)
 * `all`, `distinct`, `strictStart`, `strictEnd`, `allRows` boolean
 * `alias` String
+* `variablesSet` Iterable of
+  [CorrelationId]({{ site.apiRoot }}/org/apache/calcite/rel/core/CorrelationId.html)
 * `varHolder` [Holder]({{ site.apiRoot }}/org/apache/calcite/util/Holder.html) of [RexCorrelVariable]({{ site.apiRoot }}/org/apache/calcite/rex/RexCorrelVariable.html)
 * `patterns` Map whose key is String, value is [RexNode]({{ site.apiRoot }}/org/apache/calcite/rex/RexNode.html)
 * `subsets` Map whose key is String, value is a sorted set of String


[calcite] 06/06: [CALCITE-3145] RelBuilder.aggregate throws IndexOutOfBoundsException if groupKey is non-empty and there are duplicate aggregate functions

Posted by jh...@apache.org.
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 0cce229903a845a7b8ed36cf86d6078fd82d73d3
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Mon Jun 24 13:01:37 2019 -0700

    [CALCITE-3145] RelBuilder.aggregate throws IndexOutOfBoundsException if groupKey is non-empty and there are duplicate aggregate functions
    
    The cause is that [CALCITE-3123] did not handle the case of non-empty groupKey.
    
    Enable RelBuilder.Config.dedupAggregateCalls by default.
---
 .../java/org/apache/calcite/tools/RelBuilder.java  | 53 +++++++++++-----------
 .../org/apache/calcite/test/RelBuilderTest.java    | 47 +++++++++++++++----
 2 files changed, 64 insertions(+), 36 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 079c3de..f19a510 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -1634,32 +1634,33 @@ public class RelBuilder {
     if (!config.dedupAggregateCalls || Util.isDistinct(aggregateCalls)) {
       return aggregate_(groupSet, groupSets, r, aggregateCalls,
           registrar.extraNodes, frame.fields);
-    } else {
-      // There are duplicate aggregate calls.
-      final Set<AggregateCall> callSet = new HashSet<>();
-      final List<Integer> projects =
-          new ArrayList<>(Util.range(groupSet.cardinality()));
-      final List<AggregateCall> distinctAggregateCalls = new ArrayList<>();
-      for (AggregateCall aggregateCall : aggregateCalls) {
-        final int i;
-        if (callSet.add(aggregateCall)) {
-          i = distinctAggregateCalls.size();
-          distinctAggregateCalls.add(aggregateCall);
-        } else {
-          i = distinctAggregateCalls.indexOf(aggregateCall);
-          assert i >= 0;
-        }
-        projects.add(i);
-      }
-      aggregate_(groupSet, groupSets, r, distinctAggregateCalls,
-          registrar.extraNodes, frame.fields);
-      final List<RexNode> fields =
-          new ArrayList<>(fields(Util.range(groupSet.cardinality())));
-      for (Ord<Integer> project : Ord.zip(projects)) {
-        fields.add(alias(field(project.e), aggregateCalls.get(project.i).name));
+    }
+
+    // There are duplicate aggregate calls. Rebuild the list to eliminate
+    // duplicates, then add a Project.
+    final Set<AggregateCall> callSet = new HashSet<>();
+    final List<Pair<Integer, String>> projects = new ArrayList<>();
+    Util.range(groupSet.cardinality())
+        .forEach(i -> projects.add(Pair.of(i, null)));
+    final List<AggregateCall> distinctAggregateCalls = new ArrayList<>();
+    for (AggregateCall aggregateCall : aggregateCalls) {
+      final int i;
+      if (callSet.add(aggregateCall)) {
+        i = distinctAggregateCalls.size();
+        distinctAggregateCalls.add(aggregateCall);
+      } else {
+        i = distinctAggregateCalls.indexOf(aggregateCall);
+        assert i >= 0;
       }
-      return project(fields);
+      projects.add(Pair.of(groupSet.cardinality() + i, aggregateCall.name));
     }
+    aggregate_(groupSet, groupSets, r, distinctAggregateCalls,
+        registrar.extraNodes, frame.fields);
+    final List<RexNode> fields = projects.stream()
+        .map(p -> p.right == null ? field(p.left)
+            : alias(field(p.left), p.right))
+        .collect(Collectors.toList());
+    return project(fields);
   }
 
   /** Finishes the implementation of {@link #aggregate} by creating an
@@ -2787,10 +2788,10 @@ public class RelBuilder {
   public static class Config {
     /** Default configuration. */
     public static final Config DEFAULT =
-        new Config(false, true);
+        new Config(true, true);
 
     /** Whether {@link RelBuilder#aggregate} should eliminate duplicate
-     * aggregate calls; default true but currently disabled. */
+     * aggregate calls; default true. */
     public final boolean dedupAggregateCalls;
 
     /** Whether to simplify expressions; default true. */
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 1178a30..1f8f844 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -967,14 +967,6 @@ public class RelBuilderTest {
   /** Tests that {@link RelBuilder#aggregate} eliminates duplicate aggregate
    * calls and creates a {@code Project} to compensate. */
   @Test public void testAggregateEliminatesDuplicateCalls() {
-    final Function<RelBuilder, RelNode> fn = builder ->
-        builder.scan("EMP")
-            .aggregate(builder.groupKey(),
-                builder.sum(builder.field(1)).as("S1"),
-                builder.count().as("C"),
-                builder.sum(builder.field(2)).as("S2"),
-                builder.sum(builder.field(1)).as("S1b"))
-            .build();
     final RelBuilder builder =
         createBuilder(configBuilder ->
             configBuilder.withDedupAggregateCalls(true));
@@ -982,7 +974,7 @@ public class RelBuilderTest {
         + "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(fn.apply(builder), hasTree(expected));
+    assertThat(buildRelWithDuplicateAggregates(builder), hasTree(expected));
 
     // Now, disable the rewrite
     final RelBuilder builder2 =
@@ -991,7 +983,42 @@ public class RelBuilderTest {
     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));
+    assertThat(buildRelWithDuplicateAggregates(builder2), 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);
+    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"
+        + "    LogicalTableScan(table=[[scott, EMP]])\n";
+    assertThat(root, hasTree(expected));
+  }
+
+  /** 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);
+    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"
+        + "    LogicalTableScan(table=[[scott, EMP]])\n";
+    assertThat(root, hasTree(expected));
+  }
+
+  private RelNode buildRelWithDuplicateAggregates(RelBuilder builder,
+      int... groupFieldOrdinals) {
+    return builder.scan("EMP")
+        .aggregate(builder.groupKey(groupFieldOrdinals),
+            builder.sum(builder.field(1)).as("S1"),
+            builder.count().as("C"),
+            builder.sum(builder.field(2)).as("S2"),
+            builder.sum(builder.field(1)).as("S1b"))
+        .build();
   }
 
   @Test public void testAggregateFilter() {


[calcite] 02/06: [CALCITE-3196] In Frameworks, add interface BasePrepareAction (a functional interface) and deprecate abstract class PrepareAction

Posted by jh...@apache.org.
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 1c5de1cd27a0251ac28a503e877ef9c21e61b620
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Wed Jul 10 17:26:26 2019 -0700

    [CALCITE-3196] In Frameworks, add interface BasePrepareAction (a functional interface) and deprecate abstract class PrepareAction
---
 .../apache/calcite/prepare/CalcitePrepareImpl.java | 17 +++++--
 .../java/org/apache/calcite/tools/Frameworks.java  | 59 +++++++++++++++-------
 .../java/org/apache/calcite/tools/RelBuilder.java  | 17 ++-----
 .../org/apache/calcite/rex/RexExecutorTest.java    | 58 +++++++++------------
 .../calcite/test/RexImplicationCheckerTest.java    | 26 ++--------
 .../org/apache/calcite/tools/FrameworksTest.java   | 32 +++++-------
 6 files changed, 96 insertions(+), 113 deletions(-)

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 7dc1109..7faead7 100644
--- a/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
+++ b/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
@@ -103,6 +103,7 @@ import org.apache.calcite.sql.validate.SqlValidator;
 import org.apache.calcite.sql2rel.SqlRexConvertletTable;
 import org.apache.calcite.sql2rel.SqlToRelConverter;
 import org.apache.calcite.sql2rel.StandardConvertletTable;
+import org.apache.calcite.tools.FrameworkConfig;
 import org.apache.calcite.tools.Frameworks;
 import org.apache.calcite.util.ImmutableIntList;
 import org.apache.calcite.util.Pair;
@@ -884,15 +885,21 @@ public class CalcitePrepareImpl implements CalcitePrepare {
     return typeFactory.builder().add("$0", type).build();
   }
 
-  /** Executes a prepare action. */
+  @Deprecated // to be removed before 2.0
   public <R> R perform(CalciteServerStatement statement,
       Frameworks.PrepareAction<R> action) {
+    return perform(statement, action.getConfig(), action);
+  }
+
+  /** Executes a prepare action. */
+  public <R> R perform(CalciteServerStatement statement,
+      FrameworkConfig config, Frameworks.BasePrepareAction<R> action) {
     final CalcitePrepare.Context prepareContext =
         statement.createPrepareContext();
     final JavaTypeFactory typeFactory = prepareContext.getTypeFactory();
     final CalciteSchema schema =
-        action.getConfig().getDefaultSchema() != null
-            ? CalciteSchema.from(action.getConfig().getDefaultSchema())
+        config.getDefaultSchema() != null
+            ? CalciteSchema.from(config.getDefaultSchema())
             : prepareContext.getRootSchema();
     CalciteCatalogReader catalogReader =
         new CalciteCatalogReader(schema.root(),
@@ -902,8 +909,8 @@ public class CalcitePrepareImpl implements CalcitePrepare {
     final RexBuilder rexBuilder = new RexBuilder(typeFactory);
     final RelOptPlanner planner =
         createPlanner(prepareContext,
-            action.getConfig().getContext(),
-            action.getConfig().getCostFactory());
+            config.getContext(),
+            config.getCostFactory());
     final RelOptCluster cluster = createCluster(planner, rexBuilder);
     return action.apply(cluster, catalogReader,
         prepareContext.getRootSchema().plus(), statement);
diff --git a/core/src/main/java/org/apache/calcite/tools/Frameworks.java b/core/src/main/java/org/apache/calcite/tools/Frameworks.java
index c1f0a31..f4c8875 100644
--- a/core/src/main/java/org/apache/calcite/tools/Frameworks.java
+++ b/core/src/main/java/org/apache/calcite/tools/Frameworks.java
@@ -72,6 +72,7 @@ public class Frameworks {
    * other useful objects.
    *
    * @param <R> result type */
+  @FunctionalInterface
   public interface PlannerAction<R> {
     R apply(RelOptCluster cluster, RelOptSchema relOptSchema,
         SchemaPlus rootSchema);
@@ -84,10 +85,22 @@ public class Frameworks {
    * statement.
    *
    * @param <R> result type */
-  public abstract static class PrepareAction<R> {
+  @FunctionalInterface
+  public interface BasePrepareAction<R> {
+    R apply(RelOptCluster cluster, RelOptSchema relOptSchema,
+        SchemaPlus rootSchema, CalciteServerStatement statement);
+  }
+
+  /** As {@link BasePrepareAction} but with a {@link FrameworkConfig} included.
+   * Deprecated because a functional interface is more convenient.
+   *
+   * @param <R> result type */
+  @Deprecated // to be removed before 2.0
+  public abstract static class PrepareAction<R>
+      implements BasePrepareAction<R> {
     private final FrameworkConfig config;
     public PrepareAction() {
-      this.config = newConfigBuilder() //
+      this.config = newConfigBuilder()
           .defaultSchema(Frameworks.createRootSchema(true)).build();
     }
 
@@ -98,9 +111,6 @@ public class Frameworks {
     public FrameworkConfig getConfig() {
       return config;
     }
-
-    public abstract R apply(RelOptCluster cluster, RelOptSchema relOptSchema,
-        SchemaPlus rootSchema, CalciteServerStatement statement);
   }
 
   /**
@@ -110,17 +120,14 @@ public class Frameworks {
    * @param config FrameworkConfig to use for planner action.
    * @return Return value from action
    */
-  public static <R> R withPlanner(final PlannerAction<R> action, //
+  public static <R> R withPlanner(final PlannerAction<R> action,
       final FrameworkConfig config) {
-    return withPrepare(
-        new Frameworks.PrepareAction<R>(config) {
-          public R apply(RelOptCluster cluster, RelOptSchema relOptSchema,
-              SchemaPlus rootSchema, CalciteServerStatement statement) {
-            final CalciteSchema schema =
-                CalciteSchema.from(
-                    Util.first(config.getDefaultSchema(), rootSchema));
-            return action.apply(cluster, relOptSchema, schema.root().plus());
-          }
+    return withPrepare(config,
+        (cluster, relOptSchema, rootSchema, statement) -> {
+          final CalciteSchema schema =
+              CalciteSchema.from(
+                  Util.first(config.getDefaultSchema(), rootSchema));
+          return action.apply(cluster, relOptSchema, schema.root().plus());
         });
   }
 
@@ -136,6 +143,19 @@ public class Frameworks {
     return withPlanner(action, config);
   }
 
+  @Deprecated // to be removed before 2.0
+  public static <R> R withPrepare(PrepareAction<R> action) {
+    return withPrepare(action.getConfig(), action);
+  }
+
+  /** As {@link #withPrepare(FrameworkConfig, BasePrepareAction)} but using a
+   * default configuration. */
+  public static <R> R withPrepare(BasePrepareAction<R> action) {
+    final FrameworkConfig config = newConfigBuilder()
+        .defaultSchema(Frameworks.createRootSchema(true)).build();
+    return withPrepare(config, action);
+  }
+
   /**
    * Initializes a container then calls user-specified code with a planner
    * and statement.
@@ -143,19 +163,20 @@ public class Frameworks {
    * @param action Callback containing user-specified code
    * @return Return value from action
    */
-  public static <R> R withPrepare(PrepareAction<R> action) {
+  public static <R> R withPrepare(FrameworkConfig config,
+      BasePrepareAction<R> action) {
     try {
       final Properties info = new Properties();
-      if (action.config.getTypeSystem() != RelDataTypeSystem.DEFAULT) {
+      if (config.getTypeSystem() != RelDataTypeSystem.DEFAULT) {
         info.setProperty(CalciteConnectionProperty.TYPE_SYSTEM.camelName(),
-            action.config.getTypeSystem().getClass().getName());
+            config.getTypeSystem().getClass().getName());
       }
       Connection connection =
           DriverManager.getConnection("jdbc:calcite:", info);
       final CalciteServerStatement statement =
           connection.createStatement()
               .unwrap(CalciteServerStatement.class);
-      return new CalcitePrepareImpl().perform(statement, action);
+      return new CalcitePrepareImpl().perform(statement, config, action);
     } catch (Exception e) {
       throw new RuntimeException(e);
     }
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 7058e50..983fa35 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -70,10 +70,8 @@ import org.apache.calcite.rex.RexShuttle;
 import org.apache.calcite.rex.RexSimplify;
 import org.apache.calcite.rex.RexUtil;
 import org.apache.calcite.runtime.Hook;
-import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.schema.TransientTable;
 import org.apache.calcite.schema.impl.ListTransientTable;
-import org.apache.calcite.server.CalciteServerStatement;
 import org.apache.calcite.sql.SqlAggFunction;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlOperator;
@@ -225,18 +223,9 @@ public class RelBuilder {
 
   /** Creates a RelBuilder. */
   public static RelBuilder create(FrameworkConfig config) {
-    final RelOptCluster[] clusters = {null};
-    final RelOptSchema[] relOptSchemas = {null};
-    Frameworks.withPrepare(
-        new Frameworks.PrepareAction<Void>(config) {
-          public Void apply(RelOptCluster cluster, RelOptSchema relOptSchema,
-              SchemaPlus rootSchema, CalciteServerStatement statement) {
-            clusters[0] = cluster;
-            relOptSchemas[0] = relOptSchema;
-            return null;
-          }
-        });
-    return new RelBuilder(config.getContext(), clusters[0], relOptSchemas[0]);
+    return Frameworks.withPrepare(config,
+        (cluster, relOptSchema, rootSchema, statement) ->
+            new RelBuilder(config.getContext(), cluster, relOptSchema));
   }
 
   /** Converts this RelBuilder to a string.
diff --git a/core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java b/core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
index 2b35452..cb68fb9 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
@@ -20,13 +20,10 @@ import org.apache.calcite.DataContext;
 import org.apache.calcite.adapter.java.JavaTypeFactory;
 import org.apache.calcite.avatica.util.ByteString;
 import org.apache.calcite.linq4j.QueryProvider;
-import org.apache.calcite.plan.RelOptCluster;
-import org.apache.calcite.plan.RelOptSchema;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.schema.Schemas;
-import org.apache.calcite.server.CalciteServerStatement;
 import org.apache.calcite.sql.SqlBinaryOperator;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlOperator;
@@ -71,18 +68,14 @@ public class RexExecutorTest {
   }
 
   protected void check(final Action action) throws Exception {
-    Frameworks.withPrepare(
-        new Frameworks.PrepareAction<Void>() {
-          public Void apply(RelOptCluster cluster, RelOptSchema relOptSchema,
-              SchemaPlus rootSchema, CalciteServerStatement statement) {
-            final RexBuilder rexBuilder = cluster.getRexBuilder();
-            DataContext dataContext =
-                Schemas.createDataContext(statement.getConnection(), rootSchema);
-            final RexExecutorImpl executor = new RexExecutorImpl(dataContext);
-            action.check(rexBuilder, executor);
-            return null;
-          }
-        });
+    Frameworks.withPrepare((cluster, relOptSchema, rootSchema, statement) -> {
+      final RexBuilder rexBuilder = cluster.getRexBuilder();
+      DataContext dataContext =
+          Schemas.createDataContext(statement.getConnection(), rootSchema);
+      final RexExecutorImpl executor = new RexExecutorImpl(dataContext);
+      action.check(rexBuilder, executor);
+      return null;
+    });
   }
 
   /** Tests an executor that uses variables stored in a {@link DataContext}.
@@ -217,27 +210,22 @@ public class RexExecutorTest {
       final SqlOperator operator,
       final DataContext.Variable variable,
       final Object value) {
-    Frameworks.withPrepare(new Frameworks.PrepareAction<Void>() {
-      public Void apply(final RelOptCluster cluster,
-          final RelOptSchema relOptSchema,
-          final SchemaPlus rootSchema,
-          final CalciteServerStatement statement) {
-        final RexBuilder rexBuilder = cluster.getRexBuilder();
-        final RexExecutorImpl executor =
-            new RexExecutorImpl(
-                new SingleValueDataContext(variable.camelName, value));
-        try {
-          checkConstant(value, builder -> {
-            final List<RexNode> output = new ArrayList<>();
-            executor.reduce(rexBuilder,
-                ImmutableList.of(rexBuilder.makeCall(operator)), output);
-            return output.get(0);
-          });
-        } catch (Exception e) {
-          throw TestUtil.rethrow(e);
-        }
-        return null;
+    Frameworks.withPrepare((cluster, relOptSchema, rootSchema, statement) -> {
+      final RexBuilder rexBuilder = cluster.getRexBuilder();
+      final RexExecutorImpl executor =
+          new RexExecutorImpl(
+              new SingleValueDataContext(variable.camelName, value));
+      try {
+        checkConstant(value, builder -> {
+          final List<RexNode> output = new ArrayList<>();
+          executor.reduce(rexBuilder,
+              ImmutableList.of(rexBuilder.makeCall(operator)), output);
+          return output.get(0);
+        });
+      } catch (Exception e) {
+        throw TestUtil.rethrow(e);
       }
+      return null;
     });
   }
 
diff --git a/core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java b/core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java
index 2b9b57f..7d1e900 100644
--- a/core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java
@@ -16,12 +16,9 @@
  */
 package org.apache.calcite.test;
 
-import org.apache.calcite.DataContext;
 import org.apache.calcite.avatica.util.TimeUnitRange;
 import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
-import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptPredicateList;
-import org.apache.calcite.plan.RelOptSchema;
 import org.apache.calcite.plan.RexImplicationChecker;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
@@ -34,15 +31,12 @@ import org.apache.calcite.rex.RexLiteral;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.rex.RexSimplify;
 import org.apache.calcite.rex.RexUnknownAs;
-import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.schema.Schemas;
-import org.apache.calcite.server.CalciteServerStatement;
 import org.apache.calcite.sql.SqlCollation;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.tools.Frameworks;
 import org.apache.calcite.util.DateString;
-import org.apache.calcite.util.Holder;
 import org.apache.calcite.util.NlsString;
 import org.apache.calcite.util.TimeString;
 import org.apache.calcite.util.TimestampString;
@@ -536,21 +530,11 @@ public class RexImplicationCheckerTest {
           .add("string", stringDataType)
           .build();
 
-      final Holder<RexExecutorImpl> holder = Holder.of(null);
-      Frameworks.withPrepare(
-          new Frameworks.PrepareAction<Void>() {
-            public Void apply(RelOptCluster cluster,
-                RelOptSchema relOptSchema,
-                SchemaPlus rootSchema,
-                CalciteServerStatement statement) {
-              DataContext dataContext =
-                  Schemas.createDataContext(statement.getConnection(), rootSchema);
-              holder.set(new RexExecutorImpl(dataContext));
-              return null;
-            }
-          });
-
-      executor = holder.get();
+      executor = Frameworks.withPrepare(
+          (cluster, relOptSchema, rootSchema, statement) ->
+              new RexExecutorImpl(
+                  Schemas.createDataContext(statement.getConnection(),
+                      rootSchema)));
       simplify =
           new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, executor)
               .withParanoid(true);
diff --git a/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java b/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java
index 29c0ec3..4bec720 100644
--- a/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java
+++ b/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java
@@ -28,7 +28,6 @@ import org.apache.calcite.plan.ConventionTraitDef;
 import org.apache.calcite.plan.RelOptAbstractTable;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptPlanner;
-import org.apache.calcite.plan.RelOptSchema;
 import org.apache.calcite.plan.RelOptTable;
 import org.apache.calcite.plan.RelOptUtil;
 import org.apache.calcite.plan.RelTraitDef;
@@ -57,7 +56,6 @@ 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.server.CalciteServerStatement;
 import org.apache.calcite.sql.SqlExplainFormat;
 import org.apache.calcite.sql.SqlExplainLevel;
 import org.apache.calcite.sql.SqlNode;
@@ -173,23 +171,19 @@ public class FrameworksTest {
   }
 
   private void checkTypeSystem(final int expected, FrameworkConfig config) {
-    Frameworks.withPrepare(
-        new Frameworks.PrepareAction<Void>(config) {
-          @Override public Void apply(RelOptCluster cluster,
-              RelOptSchema relOptSchema, SchemaPlus rootSchema,
-              CalciteServerStatement statement) {
-            final RelDataType type =
-                cluster.getTypeFactory()
-                    .createSqlType(SqlTypeName.DECIMAL, 30, 2);
-            final RexLiteral literal =
-                cluster.getRexBuilder().makeExactLiteral(BigDecimal.ONE, type);
-            final RexNode call =
-                cluster.getRexBuilder().makeCall(SqlStdOperatorTable.PLUS,
-                    literal,
-                    literal);
-            assertEquals(expected, call.getType().getPrecision());
-            return null;
-          }
+    Frameworks.withPrepare(config,
+        (cluster, relOptSchema, rootSchema, statement) -> {
+          final RelDataType type =
+              cluster.getTypeFactory()
+                  .createSqlType(SqlTypeName.DECIMAL, 30, 2);
+          final RexLiteral literal =
+              cluster.getRexBuilder().makeExactLiteral(BigDecimal.ONE, type);
+          final RexNode call =
+              cluster.getRexBuilder().makeCall(SqlStdOperatorTable.PLUS,
+                  literal,
+                  literal);
+          assertEquals(expected, call.getType().getPrecision());
+          return null;
         });
   }