You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2020/06/11 19:56:46 UTC

[calcite] branch master updated (f577b7e -> eb22c01867)

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 f577b7e  [CALCITE-3724] Presto dialect implementation
     new 59d6eb8  [CALCITE-4038] Refactor RexVisitor, RexBiVisitor, RelOptUtil.InputFinder
     new eb22c01867 [CALCITE-3975] Add options to ProjectFilterTransposeRule to push down project and filter expressions whole, not just field references

The 2 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:
 .../java/org/apache/calcite/plan/RelOptUtil.java   |  42 +++--
 .../java/org/apache/calcite/rel/core/Match.java    |  22 +--
 .../apache/calcite/rel/logical/LogicalCalc.java    |   5 +-
 .../apache/calcite/rel/logical/LogicalWindow.java  |   7 +-
 .../calcite/rel/metadata/RelMdAllPredicates.java   |   4 +-
 .../calcite/rel/metadata/RelMdColumnOrigins.java   |   2 +-
 .../rel/metadata/RelMdExpressionLineage.java       |   2 +-
 .../apache/calcite/rel/rules/DateRangeRules.java   |   4 +-
 .../calcite/rel/rules/JoinAssociateRule.java       |   6 +-
 .../apache/calcite/rel/rules/LoptMultiJoin.java    |   2 +-
 .../rel/rules/ProjectFilterTransposeRule.java      | 171 ++++++++++++++++++---
 .../calcite/rel/rules/ProjectTableScanRule.java    |  19 ++-
 .../calcite/rel/rules/ProjectToWindowRule.java     |  19 +--
 .../rel/rules/ProjectWindowTransposeRule.java      |  14 +-
 .../rel/rules/UnionPullUpConstantsRule.java        |   4 +-
 .../materialize/MaterializedViewAggregateRule.java |   6 +-
 .../java/org/apache/calcite/rex/LogicVisitor.java  |  46 +-----
 .../java/org/apache/calcite/rex/RexAnalyzer.java   |   2 +-
 .../java/org/apache/calcite/rex/RexBiVisitor.java  |  40 +++++
 .../org/apache/calcite/rex/RexBiVisitorImpl.java   | 119 ++++++++++++++
 .../org/apache/calcite/rex/RexInterpreter.java     |   6 +-
 .../org/apache/calcite/rex/RexProgramBuilder.java  |   4 +-
 .../java/org/apache/calcite/rex/RexShuttle.java    |  15 +-
 .../java/org/apache/calcite/rex/RexSimplify.java   |   7 +-
 .../org/apache/calcite/rex/RexUnaryBiVisitor.java  |  88 +++++++++++
 .../main/java/org/apache/calcite/rex/RexUtil.java  |  29 ++--
 .../java/org/apache/calcite/rex/RexVisitor.java    |  27 ++++
 .../org/apache/calcite/rex/RexVisitorImpl.java     |   4 +-
 .../org/apache/calcite/rex/RexWindowBound.java     |  13 ++
 .../apache/calcite/sql2rel/RelFieldTrimmer.java    |  12 +-
 .../sql2rel/RelStructuredTypeFlattener.java        |   7 +-
 .../apache/calcite/sql2rel/SqlToRelConverter.java  |   8 +-
 .../java/org/apache/calcite/tools/RelBuilder.java  |   2 +-
 .../main/java/org/apache/calcite/util/Util.java    |  17 +-
 .../org/apache/calcite/test/RelOptRulesTest.java   |  36 +++++
 .../java/org/apache/calcite/util/UtilTest.java     |  13 ++
 .../org/apache/calcite/test/RelOptRulesTest.xml    | 119 +++++++++++---
 .../apache/calcite/adapter/druid/DruidRules.java   |  31 ++--
 .../adapter/elasticsearch/ElasticsearchRules.java  |   9 --
 .../adapter/elasticsearch/PredicateAnalyzer.java   |  15 +-
 .../calcite/adapter/geode/rel/GeodeRules.java      |  11 +-
 .../apache/calcite/adapter/mongodb/MongoRules.java |   9 --
 .../calcite/piglet/PigToSqlAggregateRule.java      |   4 +-
 43 files changed, 725 insertions(+), 297 deletions(-)
 create mode 100644 core/src/main/java/org/apache/calcite/rex/RexBiVisitorImpl.java
 create mode 100644 core/src/main/java/org/apache/calcite/rex/RexUnaryBiVisitor.java


[calcite] 02/02: [CALCITE-3975] Add options to ProjectFilterTransposeRule to push down project and filter expressions whole, not just field references

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 eb22c01867a54cbe521a69725608351491f971ca
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Fri May 29 10:18:41 2020 -0700

    [CALCITE-3975] Add options to ProjectFilterTransposeRule to push down project and filter expressions whole, not just field references
    
    Close apache/calcite#1999
---
 .../rel/rules/ProjectFilterTransposeRule.java      | 171 ++++++++++++++++++---
 .../org/apache/calcite/test/RelOptRulesTest.java   |  36 +++++
 .../org/apache/calcite/test/RelOptRulesTest.xml    | 119 +++++++++++---
 .../apache/calcite/adapter/druid/DruidRules.java   |   3 +-
 4 files changed, 282 insertions(+), 47 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ProjectFilterTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ProjectFilterTransposeRule.java
index c8202ec..593a7c8 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ProjectFilterTransposeRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ProjectFilterTransposeRule.java
@@ -19,6 +19,7 @@ package org.apache.calcite.rel.rules;
 import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.plan.RelOptRuleCall;
 import org.apache.calcite.plan.RelOptRuleOperand;
+import org.apache.calcite.plan.RelOptUtil;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.Filter;
 import org.apache.calcite.rel.core.Project;
@@ -26,18 +27,43 @@ import org.apache.calcite.rel.core.RelFactories;
 import org.apache.calcite.rel.logical.LogicalFilter;
 import org.apache.calcite.rel.logical.LogicalProject;
 import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexInputRef;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.rex.RexOver;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.tools.RelBuilder;
 import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
 
 /**
  * Planner rule that pushes a {@link org.apache.calcite.rel.core.Project}
  * past a {@link org.apache.calcite.rel.core.Filter}.
  */
-public class ProjectFilterTransposeRule extends RelOptRule implements TransformationRule {
+public class ProjectFilterTransposeRule extends RelOptRule
+    implements TransformationRule {
   public static final ProjectFilterTransposeRule INSTANCE =
       new ProjectFilterTransposeRule(LogicalProject.class, LogicalFilter.class,
-          RelFactories.LOGICAL_BUILDER, expr -> false);
+          RelFactories.LOGICAL_BUILDER, expr -> false, false, false);
+
+  /** Instance that pushes down project and filter expressions whole, not field
+   * references. */
+  public static final ProjectFilterTransposeRule EXPRESSION_INSTANCE =
+      new ProjectFilterTransposeRule(LogicalProject.class, LogicalFilter.class,
+          RelFactories.LOGICAL_BUILDER, expr -> false, true, true);
+
+  /** Instance that pushes down project expressions whole, but pushes down
+   * field references for filters. */
+  public static final ProjectFilterTransposeRule PROJECT_EXPRESSION_INSTANCE =
+      new ProjectFilterTransposeRule(LogicalProject.class, LogicalFilter.class,
+          RelFactories.LOGICAL_BUILDER, expr -> false, true, false);
 
   //~ Instance fields --------------------------------------------------------
 
@@ -45,6 +71,8 @@ public class ProjectFilterTransposeRule extends RelOptRule implements Transforma
    * Expressions that should be preserved in the projection
    */
   private final PushProjector.ExprCondition preserveExprCondition;
+  private final boolean wholeProject;
+  private final boolean wholeFilter;
 
   //~ Constructors -----------------------------------------------------------
 
@@ -53,44 +81,61 @@ public class ProjectFilterTransposeRule extends RelOptRule implements Transforma
    *
    * @param preserveExprCondition Condition for expressions that should be
    *                              preserved in the projection
+   * @param wholeProject Whether to push whole expressions from the project;
+   *                   if false, only pushes references
+   * @param wholeFilter Whether to push whole expressions;
+   *                   if false, only pushes references
    */
   public ProjectFilterTransposeRule(
       Class<? extends Project> projectClass,
       Class<? extends Filter> filterClass,
       RelBuilderFactory relBuilderFactory,
-      PushProjector.ExprCondition preserveExprCondition) {
+      PushProjector.ExprCondition preserveExprCondition,
+      boolean wholeProject, boolean wholeFilter) {
     this(
         operand(
             projectClass,
             operand(filterClass, any())),
-        preserveExprCondition, relBuilderFactory);
+        preserveExprCondition, wholeProject, wholeFilter, relBuilderFactory);
+  }
+
+  @Deprecated
+  public ProjectFilterTransposeRule(
+      Class<? extends Project> projectClass,
+      Class<? extends Filter> filterClass,
+      RelBuilderFactory relBuilderFactory,
+      PushProjector.ExprCondition preserveExprCondition) {
+    this(projectClass, filterClass, relBuilderFactory, preserveExprCondition,
+        false, false);
   }
 
   protected ProjectFilterTransposeRule(RelOptRuleOperand operand,
-      PushProjector.ExprCondition preserveExprCondition,
-      RelBuilderFactory relBuilderFactory) {
+      PushProjector.ExprCondition preserveExprCondition, boolean wholeProject,
+      boolean wholeFilter, RelBuilderFactory relBuilderFactory) {
     super(operand, relBuilderFactory, null);
     this.preserveExprCondition = preserveExprCondition;
+    this.wholeProject = wholeProject;
+    this.wholeFilter = wholeFilter;
   }
 
   //~ Methods ----------------------------------------------------------------
 
   // implement RelOptRule
   public void onMatch(RelOptRuleCall call) {
-    Project origProj;
-    Filter filter;
+    final Project origProject;
+    final Filter filter;
     if (call.rels.length >= 2) {
-      origProj = call.rel(0);
+      origProject = call.rel(0);
       filter = call.rel(1);
     } else {
-      origProj = null;
+      origProject = null;
       filter = call.rel(0);
     }
-    RelNode rel = filter.getInput();
-    RexNode origFilter = filter.getCondition();
+    final RelNode input = filter.getInput();
+    final RexNode origFilter = filter.getCondition();
 
-    if ((origProj != null)
-        && RexOver.containsOver(origProj.getProjects(), null)) {
+    if ((origProject != null)
+        && RexOver.containsOver(origProject.getProjects(), null)) {
       // Cannot push project through filter if project contains a windowed
       // aggregate -- it will affect row counts. Abort this rule
       // invocation; pushdown will be considered after the windowed
@@ -99,9 +144,9 @@ public class ProjectFilterTransposeRule extends RelOptRule implements Transforma
       return;
     }
 
-    if ((origProj != null)
-        && origProj.getRowType().isStruct()
-        && origProj.getRowType().getFieldList().stream()
+    if ((origProject != null)
+        && origProject.getRowType().isStruct()
+        && origProject.getRowType().getFieldList().stream()
           .anyMatch(RelDataTypeField::isDynamicStar)) {
       // The PushProjector would change the plan:
       //
@@ -122,13 +167,97 @@ public class ProjectFilterTransposeRule extends RelOptRule implements Transforma
       return;
     }
 
-    PushProjector pushProjector =
-        new PushProjector(
-            origProj, origFilter, rel, preserveExprCondition, call.builder());
-    RelNode topProject = pushProjector.convertProject(null);
+    final RelBuilder builder = call.builder();
+    final RelNode topProject;
+    if (origProject != null && (wholeProject || wholeFilter)) {
+      builder.push(input);
+
+      final Set<RexNode> set = new LinkedHashSet<>();
+      final RelOptUtil.InputFinder refCollector = new RelOptUtil.InputFinder();
+
+      if (wholeFilter) {
+        set.add(filter.getCondition());
+      } else {
+        filter.getCondition().accept(refCollector);
+      }
+      if (wholeProject) {
+        set.addAll(origProject.getProjects());
+      } else {
+        refCollector.visitEach(origProject.getProjects());
+      }
+
+      // Build a list with inputRefs, in order, first, then other expressions.
+      final List<RexNode> list = new ArrayList<>();
+      final ImmutableBitSet refs = refCollector.build();
+      for (RexNode field : builder.fields()) {
+        if (refs.get(((RexInputRef) field).getIndex()) || set.contains(field)) {
+          list.add(field);
+        }
+      }
+      set.removeAll(list);
+      list.addAll(set);
+      builder.project(list);
+      final Replacer replacer = new Replacer(list, builder);
+      builder.filter(replacer.visit(filter.getCondition()));
+      builder.project(replacer.visitList(origProject.getProjects()),
+          origProject.getRowType().getFieldNames());
+      topProject = builder.build();
+    } else {
+      // The traditional mode of operation of this rule: push down field
+      // references. The effect is similar to RelFieldTrimmer.
+      final PushProjector pushProjector =
+          new PushProjector(origProject, origFilter, input,
+              preserveExprCondition, builder);
+      topProject = pushProjector.convertProject(null);
+    }
 
     if (topProject != null) {
       call.transformTo(topProject);
     }
   }
+
+  /** Replaces whole expressions, or parts of an expression, with references to
+   * expressions computed by an underlying Project. */
+  private static class Replacer extends RexShuttle {
+    final ImmutableMap<RexNode, Integer> map;
+    final RelBuilder relBuilder;
+
+    Replacer(Iterable<? extends RexNode> exprs, RelBuilder relBuilder) {
+      this.relBuilder = relBuilder;
+      final ImmutableMap.Builder<RexNode, Integer> b = ImmutableMap.builder();
+      int i = 0;
+      for (RexNode expr : exprs) {
+        b.put(expr, i++);
+      }
+      map = b.build();
+    }
+
+    RexNode visit(RexNode e) {
+      final Integer i = map.get(e);
+      if (i != null) {
+        return relBuilder.field(i);
+      }
+      return e.accept(this);
+    }
+
+    @Override public void visitList(Iterable<? extends RexNode> exprs,
+        List<RexNode> out) {
+      for (RexNode expr : exprs) {
+        out.add(visit(expr));
+      }
+    }
+
+    @Override protected List<RexNode> visitList(List<? extends RexNode> exprs,
+        boolean[] update) {
+      ImmutableList.Builder<RexNode> clonedOperands = ImmutableList.builder();
+      for (RexNode operand : exprs) {
+        RexNode clonedOperand = visit(operand);
+        if ((clonedOperand != operand) && (update != null)) {
+          update[0] = true;
+        }
+        clonedOperands.add(clonedOperand);
+      }
+      return clonedOperands.build();
+    }
+  }
 }
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 2a867eb..1d88adc 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -1665,6 +1665,38 @@ class RelOptRulesTest extends RelOptTestBase {
     sql(sql).withRule(ProjectFilterTransposeRule.INSTANCE).check();
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-3975">[CALCITE-3975]
+   * ProjectFilterTransposeRule should succeed for project that happens to
+   * reference all input columns</a>. */
+  @Test void testPushProjectPastFilter3() {
+    checkPushProjectPastFilter3(ProjectFilterTransposeRule.INSTANCE)
+        .checkUnchanged();
+  }
+
+  /** As {@link #testPushProjectPastFilter3()} but pushes down project and
+   * filter expressions whole. */
+  @Test void testPushProjectPastFilter3b() {
+    checkPushProjectPastFilter3(ProjectFilterTransposeRule.EXPRESSION_INSTANCE)
+        .check();
+  }
+
+  /** As {@link #testPushProjectPastFilter3()} but pushes down project
+   * expressions whole. */
+  @Test void testPushProjectPastFilter3c() {
+    checkPushProjectPastFilter3(ProjectFilterTransposeRule.PROJECT_EXPRESSION_INSTANCE)
+        .check();
+  }
+
+  private Sql checkPushProjectPastFilter3(ProjectFilterTransposeRule rule) {
+    final String sql = "select empno + deptno as x, ename, job, mgr,\n"
+        + "  hiredate, sal, comm, slacker\n"
+        + "from emp\n"
+        + "where sal = 10 * comm\n"
+        + "and upper(ename) = 'FOO'";
+    return sql(sql).withRule(rule);
+  }
+
   @Test void testPushProjectPastJoin() {
     final String sql = "select e.sal + b.comm from emp e inner join bonus b\n"
         + "on e.ename = b.ename and e.deptno = 10";
@@ -6715,6 +6747,10 @@ class RelOptRulesTest extends RelOptTestBase {
     sql(sql, false).check();
   }
 
+  // TODO: obsolete this method;
+  // move the code into a new method Sql.withTopDownPlanner() so that you can
+  // write sql.withTopDownPlanner();
+  // withTopDownPlanner should call Sql.withTester and should be documented.
   Sql sql(String sql, boolean topDown) {
     VolcanoPlanner planner = new VolcanoPlanner();
     planner.setTopDownOpt(topDown);
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 a1cfd67..57fd8c4 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -3823,6 +3823,30 @@ LogicalIntersect(all=[false])
 ]]>
         </Resource>
     </TestCase>
+    <TestCase name="testMergeJoinCollation">
+        <Resource name="sql">
+            <![CDATA[select r.ename, s.sal from
+sales.emp r join sales.bonus s
+on r.ename=s.ename where r.sal+1=s.sal]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalProject(ENAME=[$1], SAL=[$11])
+  LogicalFilter(condition=[=(+($5, 1), $11)])
+    LogicalJoin(condition=[=($1, $9)], joinType=[inner])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+      LogicalTableScan(table=[[CATALOG, SALES, BONUS]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+EnumerableProject(ENAME=[$5], SAL=[$2])
+  EnumerableHashJoin(condition=[AND(=($0, $5), =(+($9, 1), $2))], joinType=[inner])
+    EnumerableTableScan(table=[[CATALOG, SALES, BONUS]])
+    EnumerableTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
     <TestCase name="testMergeMinus">
         <Resource name="sql">
             <![CDATA[select * from emp where deptno = 10
@@ -8151,6 +8175,77 @@ LogicalProject(EXPR$0=[CASE(=($1, 0), null:INTEGER, $0)])
 ]]>
         </Resource>
     </TestCase>
+    <TestCase name="testPushProjectPastFilter3">
+        <Resource name="sql">
+            <![CDATA[select empno + deptno as x, ename, job, mgr,
+  hiredate, sal, comm, slacker
+from emp
+where sal = 10 * comm
+and upper(ename) = 'FOO']]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalProject(X=[+($0, $7)], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], SLACKER=[$8])
+  LogicalFilter(condition=[AND(=($5, *(10, $6)), =(UPPER($1), 'FOO'))])
+    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalProject(EXPR$0=[+($0, $7)], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], SLACKER=[$8])
+  LogicalFilter(condition=[AND(=($5, *(10, $6)), =(UPPER($1), 'FOO'))])
+    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
+    <TestCase name="testPushProjectPastFilter3b">
+        <Resource name="sql">
+            <![CDATA[select empno + deptno as x, ename, job, mgr,
+  hiredate, sal, comm, slacker
+from emp
+where sal = 10 * comm
+and upper(ename) = 'FOO']]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalProject(X=[+($0, $7)], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], SLACKER=[$8])
+  LogicalFilter(condition=[AND(=($5, *(10, $6)), =(UPPER($1), 'FOO'))])
+    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalProject(X=[$8], ENAME=[$0], JOB=[$1], MGR=[$2], HIREDATE=[$3], SAL=[$4], COMM=[$5], SLACKER=[$6])
+  LogicalFilter(condition=[$7])
+    LogicalProject(ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], SLACKER=[$8], $f7=[AND(=($5, *(10, $6)), =(UPPER($1), 'FOO'))], $f8=[+($0, $7)])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
+    <TestCase name="testPushProjectPastFilter3c">
+        <Resource name="sql">
+            <![CDATA[select empno + deptno as x, ename, job, mgr,
+  hiredate, sal, comm, slacker
+from emp
+where sal = 10 * comm
+and upper(ename) = 'FOO']]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalProject(X=[+($0, $7)], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], SLACKER=[$8])
+  LogicalFilter(condition=[AND(=($5, *(10, $6)), =(UPPER($1), 'FOO'))])
+    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalProject(X=[$7], ENAME=[$0], JOB=[$1], MGR=[$2], HIREDATE=[$3], SAL=[$4], COMM=[$5], SLACKER=[$6])
+  LogicalFilter(condition=[AND(=($4, *(10, $5)), =(UPPER($0), 'FOO'))])
+    LogicalProject(ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], SLACKER=[$8], $f7=[+($0, $7)])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
     <TestCase name="testReduceCaseNullabilityChange">
         <Resource name="sql">
             <![CDATA[select case when empno = 1 then 1
@@ -12590,28 +12685,4 @@ LogicalProject(NAME=[$10], ENAME=[$1])
 ]]>
         </Resource>
     </TestCase>
-    <TestCase name="testMergeJoinCollation">
-        <Resource name="sql">
-            <![CDATA[select * from
-        sales.emp r join sales.bonus s
-        on r.ename=s.ename where r.sal+1=s.sal]]>
-        </Resource>
-        <Resource name="planBefore">
-            <![CDATA[
-LogicalProject(ENAME=[$1], SAL=[$11])
-  LogicalFilter(condition=[=(+($5, 1), $11)])
-    LogicalJoin(condition=[=($1, $9)], joinType=[inner])
-      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
-      LogicalTableScan(table=[[CATALOG, SALES, BONUS]])
-]]>
-        </Resource>
-        <Resource name="planAfter">
-            <![CDATA[
-EnumerableProject(ENAME=[$5], SAL=[$2])
-  EnumerableHashJoin(condition=[AND(=($0, $5), =(+($9, 1), $2))], joinType=[inner])
-    EnumerableTableScan(table=[[CATALOG, SALES, BONUS]])
-    EnumerableTableScan(table=[[CATALOG, SALES, EMP]])
-]]>
-        </Resource>
-    </TestCase>
 </Root>
diff --git a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
index ce2a66f..20781dc 100644
--- a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
+++ b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
@@ -799,8 +799,7 @@ public class DruidRules {
           operand(Project.class,
               operand(Filter.class,
                   operand(DruidQuery.class, none()))),
-          expr -> false,
-          relBuilderFactory);
+          expr -> false, false, false, relBuilderFactory);
     }
   }
 


[calcite] 01/02: [CALCITE-4038] Refactor RexVisitor, RexBiVisitor, RelOptUtil.InputFinder

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 59d6eb852676ab5d77eccd63f488c9f121804d5d
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Fri May 29 10:18:58 2020 -0700

    [CALCITE-4038] Refactor RexVisitor, RexBiVisitor, RelOptUtil.InputFinder
    
    Make mutable field RelOptUtil.InputFinder.inputBitSet private.
    The field still exists, public and deprecated, but it shadows
    a new private field. Rather than calling InputFinder.inputBitSet.build()
    you should now call InputFinder.build().
    
    In RexVisitor and RexBiVisitor add methods visitList and
    visitEach (the former returns a list, and the latter returns
    void).
    
    Deprecate RexShuttle.apply in favor of visitList.
    
    Add RexWindowBound.accept(RexBiVisitor).
    
    For RexBiVisitor<X, Integer> add a method visitEachIndexed,
    the index passed as the 'payload' argument.
    
    Add abstract implementations of RexBiVisitor:
    RexUnaryBiVisitor and RexBiVisitorImpl.
---
 .../java/org/apache/calcite/plan/RelOptUtil.java   |  42 ++++++--
 .../java/org/apache/calcite/rel/core/Match.java    |  22 ++--
 .../apache/calcite/rel/logical/LogicalCalc.java    |   5 +-
 .../apache/calcite/rel/logical/LogicalWindow.java  |   7 +-
 .../calcite/rel/metadata/RelMdAllPredicates.java   |   4 +-
 .../calcite/rel/metadata/RelMdColumnOrigins.java   |   2 +-
 .../rel/metadata/RelMdExpressionLineage.java       |   2 +-
 .../apache/calcite/rel/rules/DateRangeRules.java   |   4 +-
 .../calcite/rel/rules/JoinAssociateRule.java       |   6 +-
 .../apache/calcite/rel/rules/LoptMultiJoin.java    |   2 +-
 .../calcite/rel/rules/ProjectTableScanRule.java    |  19 ++--
 .../calcite/rel/rules/ProjectToWindowRule.java     |  19 ++--
 .../rel/rules/ProjectWindowTransposeRule.java      |  14 +--
 .../rel/rules/UnionPullUpConstantsRule.java        |   4 +-
 .../materialize/MaterializedViewAggregateRule.java |   6 +-
 .../java/org/apache/calcite/rex/LogicVisitor.java  |  46 ++------
 .../java/org/apache/calcite/rex/RexAnalyzer.java   |   2 +-
 .../java/org/apache/calcite/rex/RexBiVisitor.java  |  40 +++++++
 .../org/apache/calcite/rex/RexBiVisitorImpl.java   | 119 +++++++++++++++++++++
 .../org/apache/calcite/rex/RexInterpreter.java     |   6 +-
 .../org/apache/calcite/rex/RexProgramBuilder.java  |   4 +-
 .../java/org/apache/calcite/rex/RexShuttle.java    |  15 +--
 .../java/org/apache/calcite/rex/RexSimplify.java   |   7 +-
 .../org/apache/calcite/rex/RexUnaryBiVisitor.java  |  88 +++++++++++++++
 .../main/java/org/apache/calcite/rex/RexUtil.java  |  29 ++---
 .../java/org/apache/calcite/rex/RexVisitor.java    |  27 +++++
 .../org/apache/calcite/rex/RexVisitorImpl.java     |   4 +-
 .../org/apache/calcite/rex/RexWindowBound.java     |  13 +++
 .../apache/calcite/sql2rel/RelFieldTrimmer.java    |  12 +--
 .../sql2rel/RelStructuredTypeFlattener.java        |   7 +-
 .../apache/calcite/sql2rel/SqlToRelConverter.java  |   8 +-
 .../java/org/apache/calcite/tools/RelBuilder.java  |   2 +-
 .../main/java/org/apache/calcite/util/Util.java    |  17 ++-
 .../java/org/apache/calcite/util/UtilTest.java     |  13 +++
 .../apache/calcite/adapter/druid/DruidRules.java   |  28 ++---
 .../adapter/elasticsearch/ElasticsearchRules.java  |   9 --
 .../adapter/elasticsearch/PredicateAnalyzer.java   |  15 +--
 .../calcite/adapter/geode/rel/GeodeRules.java      |  11 +-
 .../apache/calcite/adapter/mongodb/MongoRules.java |   9 --
 .../calcite/piglet/PigToSqlAggregateRule.java      |   4 +-
 40 files changed, 443 insertions(+), 250 deletions(-)

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 2bf32cd..8ace22d 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -2751,7 +2751,7 @@ public abstract class RelOptUtil {
     final List<RexNode> filtersToRemove = new ArrayList<>();
     for (RexNode filter : filters) {
       final InputFinder inputFinder = InputFinder.analyze(filter);
-      final ImmutableBitSet inputBits = inputFinder.inputBitSet.build();
+      final ImmutableBitSet inputBits = inputFinder.build();
 
       // REVIEW - are there any expressions that need special handling
       // and therefore cannot be pushed?
@@ -2974,9 +2974,7 @@ public abstract class RelOptUtil {
    */
   public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
       Project project) {
-    final List<RexNode> list = new ArrayList<>();
-    pushShuttle(project).visitList(nodes, list);
-    return list;
+    return pushShuttle(project).visitList(nodes);
   }
 
   /** As {@link #pushPastProject}, but returns null if the resulting expressions
@@ -4244,17 +4242,33 @@ public abstract class RelOptUtil {
    * Visitor which builds a bitmap of the inputs used by an expression.
    */
   public static class InputFinder extends RexVisitorImpl<Void> {
+    /** @deprecated Being replaced by private field {@link #bitBuilder}.
+     * Use {@link #build}. */
+    @Deprecated // to be removed before 1.25
     public final ImmutableBitSet.Builder inputBitSet;
+
+    private final ImmutableBitSet.Builder bitBuilder;
     private final Set<RelDataTypeField> extraFields;
 
+    private InputFinder(Set<RelDataTypeField> extraFields,
+        ImmutableBitSet.Builder bitBuilder) {
+      super(true);
+      this.bitBuilder = bitBuilder;
+      this.inputBitSet = bitBuilder; // deprecated field mirrors private field
+      this.extraFields = extraFields;
+    }
+
     public InputFinder() {
       this(null);
     }
 
     public InputFinder(Set<RelDataTypeField> extraFields) {
-      super(true);
-      this.inputBitSet = ImmutableBitSet.builder();
-      this.extraFields = extraFields;
+      this(extraFields, ImmutableBitSet.builder());
+    }
+
+    public InputFinder(Set<RelDataTypeField> extraFields,
+        ImmutableBitSet initialBits) {
+      this(extraFields, initialBits.rebuild());
     }
 
     /** Returns an input finder that has analyzed a given expression. */
@@ -4268,7 +4282,7 @@ public abstract class RelOptUtil {
      * Returns a bit set describing the inputs used by an expression.
      */
     public static ImmutableBitSet bits(RexNode node) {
-      return analyze(node).inputBitSet.build();
+      return analyze(node).build();
     }
 
     /**
@@ -4278,11 +4292,19 @@ public abstract class RelOptUtil {
     public static ImmutableBitSet bits(List<RexNode> exprs, RexNode expr) {
       final InputFinder inputFinder = new InputFinder();
       RexUtil.apply(inputFinder, exprs, expr);
-      return inputFinder.inputBitSet.build();
+      return inputFinder.build();
+    }
+
+    /** Returns the bit set.
+     *
+     * <p>After calling this method, you cannot do any more visits or call this
+     * method again. */
+    public ImmutableBitSet build() {
+      return bitBuilder.build();
     }
 
     public Void visitInputRef(RexInputRef inputRef) {
-      inputBitSet.set(inputRef.getIndex());
+      bitBuilder.set(inputRef.getIndex());
       return null;
     }
 
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Match.java b/core/src/main/java/org/apache/calcite/rel/core/Match.java
index a985e5b..12fe3b9 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Match.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Match.java
@@ -209,7 +209,7 @@ public abstract class Match extends SingleRel {
   /**
    * Find aggregate functions in operands.
    */
-  private static class AggregateFinder extends RexVisitorImpl {
+  private static class AggregateFinder extends RexVisitorImpl<Void> {
     final SortedSet<RexMRAggCall> aggregateCalls = new TreeSet<>();
     final Map<String, SortedSet<RexMRAggCall>> aggregateCallsPerVar =
         new TreeMap<>();
@@ -218,7 +218,7 @@ public abstract class Match extends SingleRel {
       super(true);
     }
 
-    @Override public Object visitCall(RexCall call) {
+    @Override public Void visitCall(RexCall call) {
       SqlAggFunction aggFunction = null;
       switch (call.getKind()) {
       case SUM:
@@ -243,9 +243,7 @@ public abstract class Match extends SingleRel {
         aggFunction = new SqlBitOpAggFunction(call.getKind());
         break;
       default:
-        for (RexNode rex : call.getOperands()) {
-          rex.accept(this);
-        }
+        visitEach(call.operands);
       }
       if (aggFunction != null) {
         RexMRAggCall aggCall = new RexMRAggCall(aggFunction,
@@ -287,22 +285,20 @@ public abstract class Match extends SingleRel {
    * Visits the operands of an aggregate call to retrieve relevant pattern
    * variables.
    */
-  private static class PatternVarFinder extends RexVisitorImpl {
+  private static class PatternVarFinder extends RexVisitorImpl<Void> {
     final Set<String> patternVars = new HashSet<>();
 
     PatternVarFinder() {
       super(true);
     }
 
-    @Override public Object visitPatternFieldRef(RexPatternFieldRef fieldRef) {
+    @Override public Void visitPatternFieldRef(RexPatternFieldRef fieldRef) {
       patternVars.add(fieldRef.getAlpha());
       return null;
     }
 
-    @Override public Object visitCall(RexCall call) {
-      for (RexNode node : call.getOperands()) {
-        node.accept(this);
-      }
+    @Override public Void visitCall(RexCall call) {
+      visitEach(call.operands);
       return null;
     }
 
@@ -312,9 +308,7 @@ public abstract class Match extends SingleRel {
     }
 
     public Set<String> go(List<RexNode> rexNodeList) {
-      for (RexNode rex : rexNodeList) {
-        rex.accept(this);
-      }
+      visitEach(rexNodeList);
       return patternVars;
     }
   }
diff --git a/core/src/main/java/org/apache/calcite/rel/logical/LogicalCalc.java b/core/src/main/java/org/apache/calcite/rel/logical/LogicalCalc.java
index 05a0b4e..8e5d2ac 100644
--- a/core/src/main/java/org/apache/calcite/rel/logical/LogicalCalc.java
+++ b/core/src/main/java/org/apache/calcite/rel/logical/LogicalCalc.java
@@ -33,7 +33,6 @@ import org.apache.calcite.rel.metadata.RelMdDistribution;
 import org.apache.calcite.rel.metadata.RelMetadataQuery;
 import org.apache.calcite.rel.rules.FilterToCalcRule;
 import org.apache.calcite.rel.rules.ProjectToCalcRule;
-import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.rex.RexProgram;
 import org.apache.calcite.util.Util;
 
@@ -133,9 +132,7 @@ public final class LogicalCalc extends Calc {
   @Override public void collectVariablesUsed(Set<CorrelationId> variableSet) {
     final RelOptUtil.VariableUsedVisitor vuv =
         new RelOptUtil.VariableUsedVisitor(null);
-    for (RexNode expr : program.getExprList()) {
-      expr.accept(vuv);
-    }
+    vuv.visitEach(program.getExprList());
     variableSet.addAll(vuv.variables);
   }
 
diff --git a/core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java b/core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java
index cc7d5ad..7272a57 100644
--- a/core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java
+++ b/core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java
@@ -263,11 +263,8 @@ public final class LogicalWindow extends Window {
     // partitions may not match the order in which they occurred in the
     // original expression.
     // Add a project to permute them.
-    final List<RexNode> rexNodesWindow = new ArrayList<>();
-    for (RexNode rexNode : program.getExprList()) {
-      rexNodesWindow.add(rexNode.accept(shuttle));
-    }
-    final List<RexNode> refToWindow = toInputRefs(rexNodesWindow);
+    final List<RexNode> refToWindow =
+        toInputRefs(shuttle.visitList(program.getExprList()));
 
     final List<RexNode> projectList = new ArrayList<>();
     for (RexLocalRef inputRef : program.getProjectList()) {
diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java
index f034057..7945780 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java
@@ -134,7 +134,7 @@ public class RelMdAllPredicates
     final Set<RelDataTypeField> inputExtraFields = new LinkedHashSet<>();
     final RelOptUtil.InputFinder inputFinder = new RelOptUtil.InputFinder(inputExtraFields);
     pred.accept(inputFinder);
-    final ImmutableBitSet inputFieldsUsed = inputFinder.inputBitSet.build();
+    final ImmutableBitSet inputFieldsUsed = inputFinder.build();
 
     // Infer column origin expressions for given references
     final Map<RexInputRef, Set<RexNode>> mapping = new LinkedHashMap<>();
@@ -212,7 +212,7 @@ public class RelMdAllPredicates
     final Set<RelDataTypeField> inputExtraFields = new LinkedHashSet<>();
     final RelOptUtil.InputFinder inputFinder = new RelOptUtil.InputFinder(inputExtraFields);
     pred.accept(inputFinder);
-    final ImmutableBitSet inputFieldsUsed = inputFinder.inputBitSet.build();
+    final ImmutableBitSet inputFieldsUsed = inputFinder.build();
 
     // Infer column origin expressions for given references
     final Map<RexInputRef, Set<RexNode>> mapping = new LinkedHashMap<>();
diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnOrigins.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnOrigins.java
index d2b2d18..2958c12 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnOrigins.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnOrigins.java
@@ -270,7 +270,7 @@ public class RelMdColumnOrigins
   private Set<RelColumnOrigin> getMultipleColumns(RexNode rexNode, RelNode input,
       final RelMetadataQuery mq) {
     final Set<RelColumnOrigin> set = new HashSet<>();
-    RexVisitor visitor =
+    final RexVisitor<Void> visitor =
         new RexVisitorImpl<Void>(true) {
           public Void visitInputRef(RexInputRef inputRef) {
             Set<RelColumnOrigin> inputSet =
diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java
index fea2fa7..7f495b1 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java
@@ -493,6 +493,6 @@ public class RelMdExpressionLineage
     final Set<RelDataTypeField> inputExtraFields = new LinkedHashSet<>();
     final RelOptUtil.InputFinder inputFinder = new RelOptUtil.InputFinder(inputExtraFields);
     expr.accept(inputFinder);
-    return inputFinder.inputBitSet.build();
+    return inputFinder.build();
   }
 }
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java b/core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java
index 9029ab2..9e1dac4 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java
@@ -195,7 +195,7 @@ public abstract class DateRangeRules {
 
   /** Visitor that searches for calls to {@code EXTRACT}, {@code FLOOR} or
    * {@code CEIL}, building a list of distinct time units. */
-  private static class ExtractFinder extends RexVisitorImpl
+  private static class ExtractFinder extends RexVisitorImpl<Void>
       implements AutoCloseable {
     private final Set<TimeUnitRange> timeUnits =
         EnumSet.noneOf(TimeUnitRange.class);
@@ -208,7 +208,7 @@ public abstract class DateRangeRules {
       super(true);
     }
 
-    @Override public Object visitCall(RexCall call) {
+    @Override public Void visitCall(RexCall call) {
       switch (call.getKind()) {
       case EXTRACT:
         final RexLiteral operand = (RexLiteral) call.getOperands().get(0);
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/JoinAssociateRule.java b/core/src/main/java/org/apache/calcite/rel/rules/JoinAssociateRule.java
index e90746e..3be093f 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/JoinAssociateRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/JoinAssociateRule.java
@@ -133,9 +133,9 @@ public class JoinAssociateRule extends RelOptRule implements TransformationRule
             aCount + bCount + cCount,
             0, aCount, bCount,
             bCount, aCount + bCount, cCount);
-    final List<RexNode> newBottomList = new ArrayList<>();
-    new RexPermuteInputsShuttle(bottomMapping, relB, relC)
-        .visitList(bottom, newBottomList);
+    final List<RexNode> newBottomList =
+        new RexPermuteInputsShuttle(bottomMapping, relB, relC)
+            .visitList(bottom);
     RexNode newBottomCondition =
         RexUtil.composeConjunction(rexBuilder, newBottomList);
 
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/LoptMultiJoin.java b/core/src/main/java/org/apache/calcite/rel/rules/LoptMultiJoin.java
index 855bfdc..f49644f 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/LoptMultiJoin.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/LoptMultiJoin.java
@@ -453,7 +453,7 @@ public class LoptMultiJoin {
   private ImmutableBitSet fieldBitmap(RexNode joinFilter) {
     final RelOptUtil.InputFinder inputFinder = new RelOptUtil.InputFinder();
     joinFilter.accept(inputFinder);
-    return inputFinder.inputBitSet.build();
+    return inputFinder.build();
   }
 
   /**
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ProjectTableScanRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ProjectTableScanRule.java
index 23788a8..ce288ae 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ProjectTableScanRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ProjectTableScanRule.java
@@ -109,16 +109,15 @@ public abstract class ProjectTableScanRule extends RelOptRule {
     assert table.unwrap(ProjectableFilterableTable.class) != null;
 
     final List<Integer> selectedColumns = new ArrayList<>();
-    project.getProjects().forEach(proj -> {
-      proj.accept(new RexVisitorImpl<Void>(true) {
-        public Void visitInputRef(RexInputRef inputRef) {
-          if (!selectedColumns.contains(inputRef.getIndex())) {
-            selectedColumns.add(inputRef.getIndex());
-          }
-          return null;
+    final RexVisitorImpl<Void> visitor = new RexVisitorImpl<Void>(true) {
+      public Void visitInputRef(RexInputRef inputRef) {
+        if (!selectedColumns.contains(inputRef.getIndex())) {
+          selectedColumns.add(inputRef.getIndex());
         }
-      });
-    });
+        return null;
+      }
+    };
+    visitor.visitEach(project.getProjects());
 
     final List<RexNode> filtersPushDown;
     final List<Integer> projectsPushDown;
@@ -138,7 +137,7 @@ public abstract class ProjectTableScanRule extends RelOptRule {
     Mapping mapping =
         Mappings.target(selectedColumns, scan.getRowType().getFieldCount());
     final List<RexNode> newProjectRexNodes =
-        ImmutableList.copyOf(RexUtil.apply(mapping, project.getProjects()));
+        RexUtil.apply(mapping, project.getProjects());
 
     if (RexUtil.isIdentity(newProjectRexNodes, newScan.getRowType())) {
       call.transformTo(newScan);
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ProjectToWindowRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ProjectToWindowRule.java
index 53e3235..d61fb95 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ProjectToWindowRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ProjectToWindowRule.java
@@ -16,7 +16,6 @@
  */
 package org.apache.calcite.rel.rules;
 
-import org.apache.calcite.linq4j.Ord;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.plan.RelOptRuleCall;
@@ -28,6 +27,7 @@ import org.apache.calcite.rel.core.Project;
 import org.apache.calcite.rel.core.RelFactories;
 import org.apache.calcite.rel.logical.LogicalCalc;
 import org.apache.calcite.rel.logical.LogicalWindow;
+import org.apache.calcite.rex.RexBiVisitorImpl;
 import org.apache.calcite.rex.RexCall;
 import org.apache.calcite.rex.RexDynamicParam;
 import org.apache.calcite.rex.RexFieldAccess;
@@ -36,7 +36,6 @@ import org.apache.calcite.rex.RexLocalRef;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.rex.RexOver;
 import org.apache.calcite.rex.RexProgram;
-import org.apache.calcite.rex.RexVisitorImpl;
 import org.apache.calcite.rex.RexWindow;
 import org.apache.calcite.tools.RelBuilder;
 import org.apache.calcite.tools.RelBuilderFactory;
@@ -362,15 +361,13 @@ public abstract class ProjectToWindowRule extends RelOptRule implements Transfor
         graph.addVertex(i);
       }
 
-      for (final Ord<RexNode> expr : Ord.zip(exprs)) {
-        expr.e.accept(
-            new RexVisitorImpl<Void>(true) {
-              public Void visitLocalRef(RexLocalRef localRef) {
-                graph.addEdge(localRef.getIndex(), expr.i);
-                return null;
-              }
-            });
-      }
+      new RexBiVisitorImpl<Void, Integer>(true) {
+        public Void visitLocalRef(RexLocalRef localRef, Integer i) {
+          graph.addEdge(localRef.getIndex(), i);
+          return null;
+        }
+      }.visitEachIndexed(exprs);
+
       assert graph.vertexSet().size() == exprs.size();
       return graph;
     }
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ProjectWindowTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ProjectWindowTransposeRule.java
index edd688f..74b9a96 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ProjectWindowTransposeRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ProjectWindowTransposeRule.java
@@ -173,10 +173,8 @@ public class ProjectWindowTransposeRule extends RelOptRule implements Transforma
         window.constants, outputBuilder.build(), groups);
 
     // Modify the top LogicalProject
-    final List<RexNode> topProjExps = new ArrayList<>();
-    for (RexNode rexNode : project.getChildExps()) {
-      topProjExps.add(rexNode.accept(indexAdjustment));
-    }
+    final List<RexNode> topProjExps =
+        indexAdjustment.visitList(project.getChildExps());
 
     final LogicalProject newTopProj = project.copy(
         newLogicalWindow.getTraitSet(),
@@ -207,9 +205,7 @@ public class ProjectWindowTransposeRule extends RelOptRule implements Transforma
     };
 
     // Reference in LogicalProject
-    for (RexNode rexNode : project.getChildExps()) {
-      rexNode.accept(referenceFinder);
-    }
+    referenceFinder.visitEach(project.getChildExps());
 
     // Reference in LogicalWindow
     for (Window.Group group : window.groups) {
@@ -228,9 +224,7 @@ public class ProjectWindowTransposeRule extends RelOptRule implements Transforma
       }
 
       // Reference in Window Functions
-      for (Window.RexWinAggCall rexWinAggCall : group.aggCalls) {
-        rexWinAggCall.accept(referenceFinder);
-      }
+      referenceFinder.visitEach(group.aggCalls);
     }
     return beReferred.build();
   }
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/UnionPullUpConstantsRule.java b/core/src/main/java/org/apache/calcite/rel/rules/UnionPullUpConstantsRule.java
index 2180612..ca6ffb1 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/UnionPullUpConstantsRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/UnionPullUpConstantsRule.java
@@ -35,8 +35,6 @@ import org.apache.calcite.util.ImmutableBitSet;
 import org.apache.calcite.util.Pair;
 import org.apache.calcite.util.mapping.Mappings;
 
-import com.google.common.collect.ImmutableList;
-
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -108,7 +106,7 @@ public class UnionPullUpConstantsRule extends RelOptRule implements Transformati
     // Update top Project positions
     final Mappings.TargetMapping mapping =
         RelOptUtil.permutation(refs, union.getInput(0).getRowType()).inverse();
-    topChildExprs = ImmutableList.copyOf(RexUtil.apply(mapping, topChildExprs));
+    topChildExprs = RexUtil.apply(mapping, topChildExprs);
 
     // Create new Project-Union-Project sequences
     final RelBuilder relBuilder = call.builder();
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java b/core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java
index f13e75d..f4c318d 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java
@@ -414,10 +414,8 @@ public abstract class MaterializedViewAggregateRule extends MaterializedViewRule
       // We have a Project on top, gather only what is needed
       final RelOptUtil.InputFinder inputFinder =
           new RelOptUtil.InputFinder(new LinkedHashSet<>());
-      for (RexNode e : topProject.getChildExps()) {
-        e.accept(inputFinder);
-      }
-      references = inputFinder.inputBitSet.build();
+      inputFinder.visitEach(topProject.getChildExps());
+      references = inputFinder.build();
       for (int i = 0; i < queryAggregate.getGroupCount(); i++) {
         indexes.set(queryAggregate.getGroupSet().nth(i));
       }
diff --git a/core/src/main/java/org/apache/calcite/rex/LogicVisitor.java b/core/src/main/java/org/apache/calcite/rex/LogicVisitor.java
index 59adf52..9bcd686 100644
--- a/core/src/main/java/org/apache/calcite/rex/LogicVisitor.java
+++ b/core/src/main/java/org/apache/calcite/rex/LogicVisitor.java
@@ -29,12 +29,13 @@ import java.util.Set;
 /**
  * Visitor pattern for traversing a tree of {@link RexNode} objects.
  */
-public class LogicVisitor implements RexBiVisitor<Logic, Logic> {
+public class LogicVisitor extends RexUnaryBiVisitor<Logic> {
   private final RexNode seek;
   private final Collection<Logic> logicCollection;
 
   /** Creates a LogicVisitor. */
   private LogicVisitor(RexNode seek, Collection<Logic> logicCollection) {
+    super(true);
     this.seek = seek;
     this.logicCollection = logicCollection;
   }
@@ -77,7 +78,7 @@ public class LogicVisitor implements RexBiVisitor<Logic, Logic> {
     Collections.replaceAll(logicList, Logic.FALSE, Logic.UNKNOWN_AS_TRUE);
   }
 
-  public Logic visitCall(RexCall call, Logic logic) {
+  @Override public Logic visitCall(RexCall call, Logic logic) {
     final Logic arg0 = logic;
     switch (call.getKind()) {
     case IS_NOT_NULL:
@@ -114,47 +115,22 @@ public class LogicVisitor implements RexBiVisitor<Logic, Logic> {
     return end(call, arg0);
   }
 
-  private Logic end(RexNode node, Logic arg) {
+  @Override protected Logic end(RexNode node, Logic arg) {
     if (node.equals(seek)) {
       logicCollection.add(arg);
     }
     return arg;
   }
 
-  public Logic visitInputRef(RexInputRef inputRef, Logic arg) {
-    return end(inputRef, arg);
-  }
-
-  public Logic visitLocalRef(RexLocalRef localRef, Logic arg) {
-    return end(localRef, arg);
-  }
-
-  public Logic visitLiteral(RexLiteral literal, Logic arg) {
-    return end(literal, arg);
-  }
-
-  public Logic visitOver(RexOver over, Logic arg) {
+  @Override public Logic visitOver(RexOver over, Logic arg) {
     return end(over, arg);
   }
 
-  public Logic visitCorrelVariable(RexCorrelVariable correlVariable,
-      Logic arg) {
-    return end(correlVariable, arg);
-  }
-
-  public Logic visitDynamicParam(RexDynamicParam dynamicParam, Logic arg) {
-    return end(dynamicParam, arg);
-  }
-
-  public Logic visitRangeRef(RexRangeRef rangeRef, Logic arg) {
-    return end(rangeRef, arg);
-  }
-
-  public Logic visitFieldAccess(RexFieldAccess fieldAccess, Logic arg) {
+  @Override public Logic visitFieldAccess(RexFieldAccess fieldAccess, Logic arg) {
     return end(fieldAccess, arg);
   }
 
-  public Logic visitSubQuery(RexSubQuery subQuery, Logic arg) {
+  @Override public Logic visitSubQuery(RexSubQuery subQuery, Logic arg) {
     if (!subQuery.getType().isNullable()) {
       if (arg == Logic.TRUE_FALSE_UNKNOWN) {
         arg = Logic.TRUE_FALSE;
@@ -162,12 +138,4 @@ public class LogicVisitor implements RexBiVisitor<Logic, Logic> {
     }
     return end(subQuery, arg);
   }
-
-  @Override public Logic visitTableInputRef(RexTableInputRef ref, Logic arg) {
-    return end(ref, arg);
-  }
-
-  @Override public Logic visitPatternFieldRef(RexPatternFieldRef ref, Logic arg) {
-    return end(ref, arg);
-  }
 }
diff --git a/core/src/main/java/org/apache/calcite/rex/RexAnalyzer.java b/core/src/main/java/org/apache/calcite/rex/RexAnalyzer.java
index 3b07ca7..0e059a2 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexAnalyzer.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexAnalyzer.java
@@ -46,7 +46,7 @@ public class RexAnalyzer {
     this.e = e;
     final VariableCollector variableCollector = new VariableCollector();
     e.accept(variableCollector);
-    predicates.pulledUpPredicates.forEach(p -> p.accept(variableCollector));
+    variableCollector.visitEach(predicates.pulledUpPredicates);
     variables = ImmutableList.copyOf(variableCollector.builder);
     unsupportedCount = variableCollector.unsupportedCount;
   }
diff --git a/core/src/main/java/org/apache/calcite/rex/RexBiVisitor.java b/core/src/main/java/org/apache/calcite/rex/RexBiVisitor.java
index be2280d..65bef92 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexBiVisitor.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexBiVisitor.java
@@ -16,6 +16,11 @@
  */
 package org.apache.calcite.rex;
 
+import com.google.common.collect.ImmutableList;
+
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * Visitor pattern for traversing a tree of {@link RexNode} objects
  * and passing a payload to each.
@@ -51,4 +56,39 @@ public interface RexBiVisitor<R, P> {
   R visitTableInputRef(RexTableInputRef ref, P arg);
 
   R visitPatternFieldRef(RexPatternFieldRef ref, P arg);
+
+  /** Visits a list and writes the results to another list. */
+  default void visitList(Iterable<? extends RexNode> exprs, P arg,
+      List<R> out) {
+    for (RexNode expr : exprs) {
+      out.add(expr.accept(this, arg));
+    }
+  }
+
+  /** Visits a list and returns a list of the results.
+   * The resulting list is immutable and does not contain nulls. */
+  default List<R> visitList(Iterable<? extends RexNode> exprs, P arg) {
+    final List<R> out = new ArrayList<>();
+    visitList(exprs, arg, out);
+    return ImmutableList.copyOf(out);
+  }
+
+  /** Visits a list of expressions. */
+  default void visitEach(Iterable<? extends RexNode> exprs, P arg) {
+    for (RexNode expr : exprs) {
+      expr.accept(this, arg);
+    }
+  }
+
+  /** Visits a list of expressions, passing the 0-based index of the expression
+   * in the list.
+   *
+   * <p>Assumes that the payload type {@code P} is {@code Integer}. */
+  default void visitEachIndexed(Iterable<? extends RexNode> exprs) {
+    int i = 0;
+    for (RexNode expr : exprs) {
+      //noinspection unchecked
+      expr.accept(this, (P) (Integer) i++);
+    }
+  }
 }
diff --git a/core/src/main/java/org/apache/calcite/rex/RexBiVisitorImpl.java b/core/src/main/java/org/apache/calcite/rex/RexBiVisitorImpl.java
new file mode 100644
index 0000000..ed5ba26
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/rex/RexBiVisitorImpl.java
@@ -0,0 +1,119 @@
+/*
+ * 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.rex;
+
+/**
+ * Default implementation of {@link RexBiVisitor}, which visits each node but
+ * does nothing while it's there.
+ *
+ * @param <R> Return type from each {@code visitXxx} method
+ * @param <P> Payload type
+ */
+public class RexBiVisitorImpl<R, P> implements RexBiVisitor<R, P> {
+  //~ Instance fields --------------------------------------------------------
+
+  protected final boolean deep;
+
+  //~ Constructors -----------------------------------------------------------
+
+  protected RexBiVisitorImpl(boolean deep) {
+    this.deep = deep;
+  }
+
+  //~ Methods ----------------------------------------------------------------
+
+  public R visitInputRef(RexInputRef inputRef, P arg) {
+    return null;
+  }
+
+  public R visitLocalRef(RexLocalRef localRef, P arg) {
+    return null;
+  }
+
+  public R visitLiteral(RexLiteral literal, P arg) {
+    return null;
+  }
+
+  public R visitOver(RexOver over, P arg) {
+    R r = visitCall(over, arg);
+    if (!deep) {
+      return null;
+    }
+    final RexWindow window = over.getWindow();
+    for (RexFieldCollation orderKey : window.orderKeys) {
+      orderKey.left.accept(this, arg);
+    }
+    for (RexNode partitionKey : window.partitionKeys) {
+      partitionKey.accept(this, arg);
+    }
+    window.getLowerBound().accept(this, arg);
+    window.getUpperBound().accept(this, arg);
+    return r;
+  }
+
+  public R visitCorrelVariable(RexCorrelVariable correlVariable, P arg) {
+    return null;
+  }
+
+  public R visitCall(RexCall call, P arg) {
+    if (!deep) {
+      return null;
+    }
+
+    R r = null;
+    for (RexNode operand : call.operands) {
+      r = operand.accept(this, arg);
+    }
+    return r;
+  }
+
+  public R visitDynamicParam(RexDynamicParam dynamicParam, P arg) {
+    return null;
+  }
+
+  public R visitRangeRef(RexRangeRef rangeRef, P arg) {
+    return null;
+  }
+
+  public R visitFieldAccess(RexFieldAccess fieldAccess, P arg) {
+    if (!deep) {
+      return null;
+    }
+    final RexNode expr = fieldAccess.getReferenceExpr();
+    return expr.accept(this, arg);
+  }
+
+  public R visitSubQuery(RexSubQuery subQuery, P arg) {
+    if (!deep) {
+      return null;
+    }
+
+    R r = null;
+    for (RexNode operand : subQuery.operands) {
+      r = operand.accept(this, arg);
+    }
+    return r;
+  }
+
+  @Override public R visitTableInputRef(RexTableInputRef ref, P arg) {
+    return null;
+  }
+
+  @Override public R visitPatternFieldRef(RexPatternFieldRef fieldRef, P arg) {
+    return null;
+  }
+}
diff --git a/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java b/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java
index e3c3240..11d91ae 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java
@@ -28,7 +28,6 @@ import com.google.common.collect.ImmutableMap;
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
-import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.EnumSet;
 import java.util.List;
@@ -136,10 +135,7 @@ public class RexInterpreter implements RexVisitor<Comparable> {
   }
 
   public Comparable visitCall(RexCall call) {
-    final List<Comparable> values = new ArrayList<>(call.operands.size());
-    for (RexNode operand : call.operands) {
-      values.add(operand.accept(this));
-    }
+    final List<Comparable> values = visitList(call.operands);
     switch (call.getKind()) {
     case IS_NOT_DISTINCT_FROM:
       if (containsNull(values)) {
diff --git a/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java b/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java
index 932aa3e..71ac25c 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java
@@ -111,9 +111,7 @@ public class RexProgramBuilder {
     // are normalizing, expressions will be registered if and when they are
     // first used.
     if (!normalize) {
-      for (RexNode expr : exprList) {
-        expr.accept(shuttle);
-      }
+      shuttle.visitEach(exprList);
     }
 
     final RexShuttle expander = new RexProgram.ExpansionShuttle(exprList);
diff --git a/core/src/main/java/org/apache/calcite/rex/RexShuttle.java b/core/src/main/java/org/apache/calcite/rex/RexShuttle.java
index c898221..2aaabad 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexShuttle.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexShuttle.java
@@ -166,16 +166,6 @@ public class RexShuttle implements RexVisitor<RexNode> {
   }
 
   /**
-   * Visits a list and writes the results to another list.
-   */
-  public void visitList(
-      List<? extends RexNode> exprs, List<RexNode> outExprs) {
-    for (RexNode expr : exprs) {
-      outExprs.add(expr.accept(this));
-    }
-  }
-
-  /**
    * Visits each of a list of field collations and returns a list of the
    * results.
    *
@@ -271,9 +261,8 @@ public class RexShuttle implements RexVisitor<RexNode> {
     }
   }
 
-  /**
-   * Applies this shuttle to each expression in an iterable.
-   */
+  /** @deprecated Use {@link RexVisitor#visitList(Iterable)} if possible. */
+  @Deprecated // to be removed before 1.25
   public final Iterable<RexNode> apply(Iterable<? extends RexNode> iterable) {
     return Iterables.transform(iterable,
         t -> t == null ? null : t.accept(RexShuttle.this));
diff --git a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
index f43da20..e569a7b 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
@@ -1134,12 +1134,7 @@ public class RexSimplify {
       if (!safeOps.contains(call.getKind())) {
         return false;
       }
-      for (RexNode o : call.getOperands()) {
-        if (!o.accept(this)) {
-          return false;
-        }
-      }
-      return true;
+      return RexVisitorImpl.visitArrayAnd(this, call.operands);
     }
 
     @Override public Boolean visitOver(RexOver over) {
diff --git a/core/src/main/java/org/apache/calcite/rex/RexUnaryBiVisitor.java b/core/src/main/java/org/apache/calcite/rex/RexUnaryBiVisitor.java
new file mode 100644
index 0000000..57007e4
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/rex/RexUnaryBiVisitor.java
@@ -0,0 +1,88 @@
+/*
+ * 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.rex;
+
+/**
+ * Default implementation of a {@link RexBiVisitor} whose payload and return
+ * type are the same.
+ *
+ * @param <R> Return type from each {@code visitXxx} method
+ */
+public class RexUnaryBiVisitor<R> extends RexBiVisitorImpl<R, R> {
+  /** Creates a RexUnaryBiVisitor. */
+  protected RexUnaryBiVisitor(boolean deep) {
+    super(deep);
+  }
+
+  /** Called as the last action of, and providing the result for,
+   *  each {@code visitXxx} method; derived classes may override. */
+  protected R end(RexNode e, R arg) {
+    return arg;
+  }
+
+  @Override public R visitInputRef(RexInputRef inputRef, R arg) {
+    return end(inputRef, arg);
+  }
+
+  @Override public R visitLocalRef(RexLocalRef localRef, R arg) {
+    return end(localRef, arg);
+  }
+
+  @Override public R visitTableInputRef(RexTableInputRef ref, R arg) {
+    return end(ref, arg);
+  }
+
+  @Override public R visitPatternFieldRef(RexPatternFieldRef fieldRef, R arg) {
+    return end(fieldRef, arg);
+  }
+
+  @Override public R visitLiteral(RexLiteral literal, R arg) {
+    return end(literal, arg);
+  }
+
+  @Override public R visitDynamicParam(RexDynamicParam dynamicParam, R arg) {
+    return end(dynamicParam, arg);
+  }
+
+  @Override public R visitRangeRef(RexRangeRef rangeRef, R arg) {
+    return end(rangeRef, arg);
+  }
+
+  @Override public R visitCorrelVariable(RexCorrelVariable correlVariable, R arg) {
+    return end(correlVariable, arg);
+  }
+
+  @Override public R visitOver(RexOver over, R arg) {
+    super.visitOver(over, arg);
+    return end(over, arg);
+  }
+
+  @Override public R visitCall(RexCall call, R arg) {
+    super.visitCall(call, arg);
+    return end(call, arg);
+  }
+
+  @Override public R visitFieldAccess(RexFieldAccess fieldAccess, R arg) {
+    super.visitFieldAccess(fieldAccess, arg);
+    return end(fieldAccess, arg);
+  }
+
+  @Override public R visitSubQuery(RexSubQuery subQuery, R arg) {
+    super.visitSubQuery(subQuery, arg);
+    return end(subQuery, arg);
+  }
+}
diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
index 98ef1e2..67d60a5 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
@@ -1322,10 +1322,9 @@ public class RexUtil {
   /**
    * Applies a mapping to an iterable over expressions.
    */
-  public static Iterable<RexNode> apply(Mappings.TargetMapping mapping,
+  public static List<RexNode> apply(Mappings.TargetMapping mapping,
       Iterable<? extends RexNode> nodes) {
-    final RexPermuteInputsShuttle shuttle = RexPermuteInputsShuttle.of(mapping);
-    return Iterables.transform(nodes, e -> e.accept(shuttle));
+    return RexPermuteInputsShuttle.of(mapping).visitList(nodes);
   }
 
   /**
@@ -1646,8 +1645,8 @@ public class RexUtil {
   /**
    * Shifts every {@link RexInputRef} in an expression by {@code offset}.
    */
-  public static Iterable<RexNode> shift(Iterable<RexNode> nodes, int offset) {
-    return new RexShiftShuttle(offset).apply(nodes);
+  public static List<RexNode> shift(Iterable<RexNode> nodes, int offset) {
+    return new RexShiftShuttle(offset).visitList(nodes);
   }
 
   /**
@@ -2145,16 +2144,12 @@ public class RexUtil {
    */
   public static Set<RelTableRef> gatherTableReferences(final List<RexNode> nodes) {
     final Set<RelTableRef> occurrences = new HashSet<>();
-    RexVisitor<Void> visitor =
-        new RexVisitorImpl<Void>(true) {
-          @Override public Void visitTableInputRef(RexTableInputRef ref) {
-            occurrences.add(ref.getTableRef());
-            return super.visitTableInputRef(ref);
-          }
-        };
-    for (RexNode e : nodes) {
-      e.accept(visitor);
-    }
+    new RexVisitorImpl<Void>(true) {
+      @Override public Void visitTableInputRef(RexTableInputRef ref) {
+        occurrences.add(ref.getTableRef());
+        return super.visitTableInputRef(ref);
+      }
+    }.visitEach(nodes);
     return occurrences;
   }
 
@@ -2305,9 +2300,7 @@ public class RexUtil {
     }
 
     public Void visitCall(RexCall call) {
-      for (RexNode operand : call.operands) {
-        operand.accept(this);
-      }
+      visitEach(call.operands);
       return null;
     }
 
diff --git a/core/src/main/java/org/apache/calcite/rex/RexVisitor.java b/core/src/main/java/org/apache/calcite/rex/RexVisitor.java
index b958aa1..b202ded 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexVisitor.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexVisitor.java
@@ -16,6 +16,11 @@
  */
 package org.apache.calcite.rex;
 
+import com.google.common.collect.ImmutableList;
+
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * Visitor pattern for traversing a tree of {@link RexNode} objects.
  *
@@ -51,4 +56,26 @@ public interface RexVisitor<R> {
   R visitTableInputRef(RexTableInputRef fieldRef);
 
   R visitPatternFieldRef(RexPatternFieldRef fieldRef);
+
+  /** Visits a list and writes the results to another list. */
+  default void visitList(Iterable<? extends RexNode> exprs, List<R> out) {
+    for (RexNode expr : exprs) {
+      out.add(expr.accept(this));
+    }
+  }
+
+  /** Visits a list and returns a list of the results.
+   * The resulting list is immutable and does not contain nulls. */
+  default List<R> visitList(Iterable<? extends RexNode> exprs) {
+    final List<R> out = new ArrayList<>();
+    visitList(exprs, out);
+    return ImmutableList.copyOf(out);
+  }
+
+  /** Visits a list of expressions. */
+  default void visitEach(Iterable<? extends RexNode> exprs) {
+    for (RexNode expr : exprs) {
+      expr.accept(this);
+    }
+  }
 }
diff --git a/core/src/main/java/org/apache/calcite/rex/RexVisitorImpl.java b/core/src/main/java/org/apache/calcite/rex/RexVisitorImpl.java
index 707d583..c480878 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexVisitorImpl.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexVisitorImpl.java
@@ -58,9 +58,7 @@ public class RexVisitorImpl<R> implements RexVisitor<R> {
     for (RexFieldCollation orderKey : window.orderKeys) {
       orderKey.left.accept(this);
     }
-    for (RexNode partitionKey : window.partitionKeys) {
-      partitionKey.accept(this);
-    }
+    visitEach(window.partitionKeys);
     window.getLowerBound().accept(this);
     window.getUpperBound().accept(this);
     return r;
diff --git a/core/src/main/java/org/apache/calcite/rex/RexWindowBound.java b/core/src/main/java/org/apache/calcite/rex/RexWindowBound.java
index 3734b88..7f94b11 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexWindowBound.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexWindowBound.java
@@ -85,6 +85,7 @@ public abstract class RexWindowBound {
 
   /**
    * Transforms the bound via {@link org.apache.calcite.rex.RexVisitor}.
+   *
    * @param visitor visitor to accept
    * @param <R> return type of the visitor
    * @return transformed bound
@@ -94,6 +95,18 @@ public abstract class RexWindowBound {
   }
 
   /**
+   * Transforms the bound via {@link org.apache.calcite.rex.RexBiVisitor}.
+   *
+   * @param visitor visitor to accept
+   * @param arg Payload
+   * @param <R> return type of the visitor
+   * @return transformed bound
+   */
+  public <R, P> RexWindowBound accept(RexBiVisitor<R, P> visitor, P arg) {
+    return this;
+  }
+
+  /**
    * Returns the number of nodes in this bound.
    *
    * @see RexNode#nodeCount()
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 be4df0f..2da00c0 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
@@ -372,7 +372,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
         ord.e.accept(inputFinder);
       }
     }
-    ImmutableBitSet inputFieldsUsed = inputFinder.inputBitSet.build();
+    ImmutableBitSet inputFieldsUsed = inputFinder.build();
 
     // Create input with trimmed columns.
     TrimResult trimResult =
@@ -461,10 +461,9 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
     final Set<RelDataTypeField> inputExtraFields =
         new LinkedHashSet<>(extraFields);
     RelOptUtil.InputFinder inputFinder =
-        new RelOptUtil.InputFinder(inputExtraFields);
-    inputFinder.inputBitSet.addAll(fieldsUsed);
+        new RelOptUtil.InputFinder(inputExtraFields, fieldsUsed);
     conditionExpr.accept(inputFinder);
-    final ImmutableBitSet inputFieldsUsed = inputFinder.inputBitSet.build();
+    final ImmutableBitSet inputFieldsUsed = inputFinder.build();
 
     // Create input with trimmed columns.
     TrimResult trimResult =
@@ -650,10 +649,9 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
     final Set<RelDataTypeField> combinedInputExtraFields =
         new LinkedHashSet<>(extraFields);
     RelOptUtil.InputFinder inputFinder =
-        new RelOptUtil.InputFinder(combinedInputExtraFields);
-    inputFinder.inputBitSet.addAll(fieldsUsed);
+        new RelOptUtil.InputFinder(combinedInputExtraFields, fieldsUsed);
     conditionExpr.accept(inputFinder);
-    final ImmutableBitSet fieldsUsedPlus = inputFinder.inputBitSet.build();
+    final ImmutableBitSet fieldsUsedPlus = inputFinder.build();
 
     // If no system fields are used, we can remove them.
     int systemFieldUsedCount = 0;
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java b/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
index 897eb63..1e9ba3d 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
@@ -91,7 +91,6 @@ import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.SortedMap;
 import java.util.SortedSet;
-import java.util.stream.Collectors;
 
 // TODO jvs 10-Feb-2005:  factor out generic rewrite helper, with the
 // ability to map between old and new rels and field ordinals.  Also,
@@ -688,10 +687,8 @@ public class RelStructuredTypeFlattener implements ReflectiveVisitor {
             }
           }
         } else {
-          List<RexNode> newOperands = operands.stream()
-              .map(op -> op.accept(shuttle))
-              .collect(Collectors.toList());
-          newExp = rexBuilder.makeCall(exp.getType(), operator, newOperands);
+          newExp = rexBuilder.makeCall(exp.getType(), operator,
+              shuttle.visitList(operands));
           // flatten call result type
           flattenResultTypeOfRexCall(newExp, fieldName, flattenedExps);
         }
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 28ffa01..fdb26df 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -201,7 +201,6 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.TreeSet;
-import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
@@ -5926,15 +5925,10 @@ public class SqlToRelConverter {
     /** Whether to push down join conditions; default true. */
     public ConfigBuilder withPushJoinCondition(boolean pushJoinCondition) {
       return withRelBuilderConfigTransform(
-          compose(relBuilderConfigTransform,
+          Util.andThen(relBuilderConfigTransform,
               c -> c.withPushJoinCondition(pushJoinCondition)));
     }
 
-    private static <X> UnaryOperator<X> compose(Function<X, X> f1,
-        Function<X, X> f2) {
-      return x -> f2.apply(f1.apply(x));
-    }
-
     @Deprecated // to be removed before 2.0
     public ConfigBuilder withInSubqueryThreshold(int inSubQueryThreshold) {
       return withInSubQueryThreshold(inSubQueryThreshold);
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 9e19e74..77e10c0 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -1483,7 +1483,7 @@ public class RelBuilder {
    * <p>The default implementation returns {@code true};
    * sub-classes may disable merge by overriding to return {@code false}. */
   @Experimental
-  @Deprecated
+  @Deprecated // to be removed before 1.25
   protected boolean shouldMergeProject() {
     return true;
   }
diff --git a/core/src/main/java/org/apache/calcite/util/Util.java b/core/src/main/java/org/apache/calcite/util/Util.java
index 8b59a88..d5b9198 100644
--- a/core/src/main/java/org/apache/calcite/util/Util.java
+++ b/core/src/main/java/org/apache/calcite/util/Util.java
@@ -97,6 +97,7 @@ import java.util.StringTokenizer;
 import java.util.TimeZone;
 import java.util.function.Function;
 import java.util.function.Predicate;
+import java.util.function.UnaryOperator;
 import java.util.jar.JarFile;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -2374,8 +2375,12 @@ public class Util {
    * Returns a {@code Collector} that accumulates the input elements into a
    * Guava {@link ImmutableList} via a {@link ImmutableList.Builder}.
    *
-   * <p>It will be obsolete when we move to {@link Bug#upgrade Guava 21.0},
-   * which has {@code ImmutableList.toImmutableList()}.
+   * <p>It will be obsolete when we move to {@link Bug#upgrade Guava 28.0-jre}.
+   * Guava 21.0 introduced {@code ImmutableList.toImmutableList()}, but it had
+   * a {@link com.google.common.annotations.Beta} tag until 28.0-jre.
+   *
+   * <p>In {@link Bug#upgrade Guava 21.0}, change this method to call
+   * {@code ImmutableList.toImmutableList()}, ignoring the {@code @Beta} tag.
    *
    * @param <T> Type of the input elements
    *
@@ -2392,6 +2397,14 @@ public class Util {
         ImmutableList.Builder::build);
   }
 
+  /** Returns an operator that applies {@code op1} and then {@code op2}.
+   *
+   * <p>As {@link Function#andThen(Function)} but for {@link UnaryOperator}. */
+  public static <X> UnaryOperator<X> andThen(UnaryOperator<X> op1,
+      UnaryOperator<X> op2) {
+    return op1.andThen(op2)::apply;
+  }
+
   /** Transforms a list, applying a function to each element. */
   public static <F, T> List<T> transform(List<F> list,
       java.util.function.Function<F, T> function) {
diff --git a/core/src/test/java/org/apache/calcite/util/UtilTest.java b/core/src/test/java/org/apache/calcite/util/UtilTest.java
index d952fa7..fd6c1ea 100644
--- a/core/src/test/java/org/apache/calcite/util/UtilTest.java
+++ b/core/src/test/java/org/apache/calcite/util/UtilTest.java
@@ -90,6 +90,7 @@ import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.BiConsumer;
 import java.util.function.Function;
 import java.util.function.ObjIntConsumer;
+import java.util.function.UnaryOperator;
 
 import static org.apache.calcite.test.Matchers.isLinux;
 
@@ -2427,6 +2428,18 @@ class UtilTest {
     assertThat(isLinux("x\r\ny").matches("x\ny"), is(false));
   }
 
+  /** Tests {@link Util#andThen(UnaryOperator, UnaryOperator)}. */
+  @Test void testAndThen() {
+    final UnaryOperator<Integer> inc = x -> x + 1;
+    final UnaryOperator<Integer> triple = x -> x * 3;
+    final UnaryOperator<Integer> tripleInc = Util.andThen(triple, inc);
+    final UnaryOperator<Integer> incTriple = Util.andThen(inc, triple);
+    final Function<Integer, Integer> incTripleFn = inc.andThen(triple);
+    assertThat(tripleInc.apply(2), is(7));
+    assertThat(incTriple.apply(2), is(9));
+    assertThat(incTripleFn.apply(2), is(9));
+  }
+
   /** Tests {@link Util#transform(List, java.util.function.Function)}. */
   @Test void testTransform() {
     final List<String> beatles =
diff --git a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
index 3e51f58..ce2a66f 100644
--- a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
+++ b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
@@ -332,12 +332,11 @@ public class DruidRules {
       call.transformTo(newProject2);
     }
 
-    private static Pair<List<RexNode>, List<RexNode>> splitProjects(final RexBuilder rexBuilder,
-            final RelNode input, List<RexNode> nodes) {
-      final RelOptUtil.InputReferencedVisitor visitor = new RelOptUtil.InputReferencedVisitor();
-      for (RexNode node : nodes) {
-        node.accept(visitor);
-      }
+    private static Pair<List<RexNode>, List<RexNode>> splitProjects(
+        final RexBuilder rexBuilder, final RelNode input, List<RexNode> nodes) {
+      final RelOptUtil.InputReferencedVisitor visitor =
+          new RelOptUtil.InputReferencedVisitor();
+      visitor.visitEach(nodes);
       if (visitor.inputPosReferenced.size() == input.getRowType().getFieldCount()) {
         // All inputs are referenced
         return null;
@@ -350,17 +349,12 @@ public class DruidRules {
         belowNodes.add(node);
         belowTypes.add(node.getType());
       }
-      final List<RexNode> aboveNodes = new ArrayList<>();
-      for (RexNode node : nodes) {
-        aboveNodes.add(
-            node.accept(
-              new RexShuttle() {
-                @Override public RexNode visitInputRef(RexInputRef ref) {
-                  final int index = positions.indexOf(ref.getIndex());
-                  return rexBuilder.makeInputRef(belowTypes.get(index), index);
-                }
-              }));
-      }
+      final List<RexNode> aboveNodes = new RexShuttle() {
+        @Override public RexNode visitInputRef(RexInputRef ref) {
+          final int index = positions.indexOf(ref.getIndex());
+          return rexBuilder.makeInputRef(belowTypes.get(index), index);
+        }
+      }.visitList(nodes);
       return Pair.of(aboveNodes, belowNodes);
     }
   }
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
index c70d935..7f2487c 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
@@ -43,7 +43,6 @@ import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.sql.validate.SqlValidatorUtil;
 
 import java.util.AbstractList;
-import java.util.ArrayList;
 import java.util.List;
 
 /**
@@ -182,14 +181,6 @@ class ElasticsearchRules {
       throw new IllegalArgumentException("Translation of " + call
           + " is not supported by ElasticsearchProject");
     }
-
-    List<String> visitList(List<RexNode> list) {
-      final List<String> strings = new ArrayList<>();
-      for (RexNode node: list) {
-        strings.add(node.accept(this));
-      }
-      return strings;
-    }
   }
 
   /**
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java
index 1679f03..335f894 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java
@@ -36,14 +36,12 @@ import org.apache.calcite.sql.type.SqlTypeName;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
 
-import java.util.ArrayList;
 import java.util.GregorianCalendar;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
-import java.util.stream.Collectors;
 
 import static org.apache.calcite.adapter.elasticsearch.QueryBuilders.boolQuery;
 import static org.apache.calcite.adapter.elasticsearch.QueryBuilders.existsQuery;
@@ -133,11 +131,8 @@ class PredicateAnalyzer {
       if (call.getOperator().getKind() == SqlKind.NOT) {
         RexNode child = call.getOperands().get(0);
         if (child.getKind() == SqlKind.LIKE) {
-          List<RexNode> operands = ((RexCall) child).getOperands()
-              .stream()
-              .map(rexNode -> rexNode.accept(NotLikeConverter.this))
-              .collect(Collectors.toList());
-          return rexBuilder.makeCall(SqlStdOperatorTable.NOT_LIKE, operands);
+          return rexBuilder.makeCall(SqlStdOperatorTable.NOT_LIKE,
+              visitList(((RexCall) child).getOperands()));
         }
       }
       return super.visitCall(call);
@@ -246,11 +241,7 @@ class PredicateAnalyzer {
         }
       case FUNCTION:
         if (call.getOperator().getName().equalsIgnoreCase("CONTAINS")) {
-          List<Expression> operands = new ArrayList<>();
-          for (RexNode node : call.getOperands()) {
-            final Expression nodeExpr = node.accept(this);
-            operands.add(nodeExpr);
-          }
+          List<Expression> operands = visitList(call.getOperands());
           String query = convertQueryString(operands.subList(0, operands.size() - 1),
               operands.get(operands.size() - 1));
           return QueryExpression.create(new NamedFieldExpression()).queryString(query);
diff --git a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java
index 80e9220..ab7f402 100644
--- a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java
+++ b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java
@@ -99,7 +99,8 @@ public class GeodeRules {
     }
 
     @Override public String visitCall(RexCall call) {
-      final List<String> strings = visitList(call.operands);
+      final List<String> strings = new ArrayList<>();
+      visitList(call.operands, strings);
       if (call.getOperator() == SqlStdOperatorTable.ITEM) {
         final RexNode op1 = call.getOperands().get(1);
         if (op1 instanceof RexLiteral) {
@@ -117,14 +118,6 @@ public class GeodeRules {
     private String stripQuotes(String s) {
       return s.startsWith("'") && s.endsWith("'") ? s.substring(1, s.length() - 1) : s;
     }
-
-    List<String> visitList(List<RexNode> list) {
-      final List<String> strings = new ArrayList<>();
-      for (RexNode node : list) {
-        strings.add(node.accept(this));
-      }
-      return strings;
-    }
   }
 
   /**
diff --git a/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoRules.java b/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoRules.java
index befa5ca..f065bfc 100644
--- a/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoRules.java
+++ b/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoRules.java
@@ -49,7 +49,6 @@ import org.apache.calcite.util.trace.CalciteTrace;
 import org.slf4j.Logger;
 
 import java.util.AbstractList;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -234,14 +233,6 @@ public class MongoRules {
           ? s.substring(1, s.length() - 1)
           : s;
     }
-
-    public List<String> visitList(List<RexNode> list) {
-      final List<String> strings = new ArrayList<>();
-      for (RexNode node : list) {
-        strings.add(node.accept(this));
-      }
-      return strings;
-    }
   }
 
   /** Base class for planner rules that convert a relational expression to
diff --git a/piglet/src/main/java/org/apache/calcite/piglet/PigToSqlAggregateRule.java b/piglet/src/main/java/org/apache/calcite/piglet/PigToSqlAggregateRule.java
index e584771..bd94628 100644
--- a/piglet/src/main/java/org/apache/calcite/piglet/PigToSqlAggregateRule.java
+++ b/piglet/src/main/java/org/apache/calcite/piglet/PigToSqlAggregateRule.java
@@ -101,9 +101,7 @@ public class PigToSqlAggregateRule extends RelOptRule {
       } else if (isMultisetProjection(call) && !ignoreMultisetProj) {
         pigAggCalls.add(call);
       }
-      for (RexNode operand : call.operands) {
-        operand.accept(this);
-      }
+      visitEach(call.operands);
       return null;
     }