[CALCITE-7646] CorrelateProjectExtractor does not handle nested field accesses cor0.field0.field1

core/src/main/java/org/apache/calcite/sql2rel/CorrelateProjectExtractor.java

@@ -88,15 +88,33 @@ public CorrelateProjectExtractor(RelBuilderFactory factory) {
     this.builderFactory = factory;
   }
 
+  /** Returns whether {@code node} is a direct field access on the correlation
+   * variable with the specified id, such as {@code $cor0.DEPTNO}. A nested
+   * access such as {@code $cor0.REC.DEPTNO} is not direct. */
+  private static boolean isDirectFieldAccess(RexNode node, CorrelationId id) {
+    if (node instanceof RexFieldAccess) {
+      RexFieldAccess access = (RexFieldAccess) node;
+      if (access.getReferenceExpr() instanceof RexCorrelVariable) {
+        RexCorrelVariable var = (RexCorrelVariable) access.getReferenceExpr();
+        return var.id.equals(id);
+      }
+    }
+    return false;
+  }
+
   @Override public RelNode visit(LogicalCorrelate correlate) {
     RelNode left = correlate.getLeft().accept(this);
     RelNode right = correlate.getRight().accept(this);
     int oldLeft = left.getRowType().getFieldCount();
     // Find the correlated expressions from the right side that can be moved to the left
     Set<RexNode> callsWithCorrelationInRight =
         findCorrelationDependentCalls(correlate.getCorrelationId(), right);
+        // Only direct field accesses on the correlation variable, such as
+        // $cor0.DEPTNO, are left in place. A nested field access, such as
+        // $cor0.REC.DEPTNO, is extracted
     boolean isTrivialCorrelation =
-        callsWithCorrelationInRight.stream().allMatch(exp -> exp instanceof RexFieldAccess);
+        callsWithCorrelationInRight.stream()
+            .allMatch(exp -> isDirectFieldAccess(exp, correlate.getCorrelationId()));
     // Early exit condition
     if (isTrivialCorrelation) {
       if (correlate.getLeft().equals(left) && correlate.getRight().equals(right)) {
@@ -125,11 +143,15 @@ public CorrelateProjectExtractor(RelBuilderFactory factory) {
     // Construct the mapping to transform the expressions in the right side based on the new
     // projection in the left side.
     Map<RexNode, RexNode> transformMapping = new HashMap<>();
+    int newFieldIndex = oldLeft;
     for (RexNode callInRight : callsWithCorrelationInRight) {
-      RexBuilder xb = builder.getRexBuilder();
-      RexNode v = xb.makeCorrel(builder.peek().getRowType(), correlate.getCorrelationId());
-      RexNode flatCorrelationInRight = xb.makeFieldAccess(v, oldLeft + transformMapping.size());
-      transformMapping.put(callInRight, flatCorrelationInRight);
+      if (!isDirectFieldAccess(callInRight, correlate.getCorrelationId())) {
+        RexBuilder xb = builder.getRexBuilder();
+        RexNode v = xb.makeCorrel(builder.peek().getRowType(), correlate.getCorrelationId());
+        RexNode flatCorrelationInRight = xb.makeFieldAccess(v, newFieldIndex);
+        transformMapping.put(callInRight, flatCorrelationInRight);
+      }
+      newFieldIndex++;
     }
 
     // Select the required fields/columns from the left side of the correlation. Based on the code
@@ -264,8 +286,13 @@ private static boolean isSimpleCorrelatedExpression(RexNode node, CorrelationId
    * +(10, $cor0.DEPTNO) -> TRUE
    * /(100,+(10, $cor0.DEPTNO)) -> TRUE
    * CAST(+(10, $cor0.DEPTNO)):INTEGER NOT NULL -> TRUE
+   * CASE(IS NOT NULL($cor0.PATH), $cor0.PATH, ARRAY(NULL)) -> TRUE
    * +($0, $cor0.DEPTNO) -> FALSE
    * }</pre>
+   *
+   * <p>A subexpression built only from literals and dynamic parameters, such
+   * as {@code ARRAY(NULL)} above, is neutral: it neither qualifies nor
+   * disqualifies the enclosing call.
    */
   private static class SimpleCorrelationDetector
       extends RexVisitorImpl<@Nullable Boolean> {
@@ -284,15 +311,17 @@ private SimpleCorrelationDetector(CorrelationId corrId) {
       return Boolean.FALSE;
     }
 
-    @Override public Boolean visitCall(RexCall call) {
+    @Override public @Nullable Boolean visitCall(RexCall call) {
+      // Constant operands must not disqualify the call
       Boolean hasSimpleCorrelation = null;
       for (RexNode op : call.operands) {
         Boolean b = op.accept(this);
         if (b != null) {
           hasSimpleCorrelation = hasSimpleCorrelation == null ? b : hasSimpleCorrelation && b;
         }
       }
-      return hasSimpleCorrelation == null ? Boolean.FALSE : hasSimpleCorrelation;
+      // If unsure return null; caller will decide
+      return hasSimpleCorrelation;
     }
 
     @Override public @Nullable Boolean visitFieldAccess(RexFieldAccess fieldAccess) {
@@ -332,8 +361,10 @@ private static RexNode replaceCorrelationsWithInputRef(RexNode exp, RelBuilder b
   }
 
   /**
-   * A visitor traversing row expressions and replacing calls with other expressions according
-   * to the specified mapping.
+   * A visitor traversing row expressions and replacing calls and field
+   * accesses with other expressions according to the specified mapping.
+   * The mapping is consulted before recursing so that the outermost
+   * matching expression wins.
    */
   private static final class CallReplacer extends RexShuttle {
     private final Map<RexNode, RexNode> mapping;
@@ -350,5 +381,14 @@ private static final class CallReplacer extends RexShuttle {
         return super.visitCall(oldCall);
       }
     }
+
+    @Override public RexNode visitFieldAccess(RexFieldAccess fieldAccess) {
+      RexNode replacement = mapping.get(fieldAccess);
+      if (replacement != null) {
+        return replacement;
+      } else {
+        return super.visitFieldAccess(fieldAccess);
+      }
+    }
   }
 }

core/src/test/java/org/apache/calcite/sql2rel/CorrelateProjectExtractorTest.java

@@ -84,6 +84,78 @@ public static Frameworks.ConfigBuilder config() {
     assertThat(after, hasTree(planAfter));
   }
 
+  /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-7646">[CALCITE-7646]
+   * CorrelateProjectExtractor does not handle nested field accesses cor0.field0.field1</a>. */
+  @Test void testNestedCorrelationFieldAccessInFilter() {
+    final RelBuilder builder = RelBuilder.create(config().build());
+    final Holder<@Nullable RexCorrelVariable> v = Holder.empty();
+    RelNode before = builder.scan("EMP")
+        .project(
+            builder.alias(
+                builder.call(SqlStdOperatorTable.ROW,
+                    builder.field("EMPNO"), builder.field("DEPTNO")), "R"))
+        .variable(v::set)
+        .scan("DEPT")
+        .filter(
+            builder.equals(builder.field(0),
+                builder.getRexBuilder().makeFieldAccess(builder.field(v.get(), "R"), 1)))
+        .correlate(JoinRelType.LEFT, v.get().id, builder.field(2, 0, "R"))
+        .build();
+
+    final String planBefore = ""
+        + "LogicalCorrelate(correlation=[$cor0], joinType=[left], requiredColumns=[{0}])\n"
+        + "  LogicalProject(R=[ROW($0, $7)])\n"
+        + "    LogicalTableScan(table=[[scott, EMP]])\n"
+        + "  LogicalFilter(condition=[=($0, $cor0.R.EXPR$1)])\n"
+        + "    LogicalTableScan(table=[[scott, DEPT]])\n";
+    assertThat(before, hasTree(planBefore));
+
+    RelNode after = before.accept(new CorrelateProjectExtractor(RelFactories.LOGICAL_BUILDER));
+    final String planAfter = ""
+        + "LogicalProject(R=[$0], DEPTNO=[$2], DNAME=[$3], LOC=[$4])\n"
+        + "  LogicalCorrelate(correlation=[$cor0], joinType=[left], requiredColumns=[{1}])\n"
+        + "    LogicalProject(R=[ROW($0, $7)], $f1=[ROW($0, $7).EXPR$1])\n"
+        + "      LogicalTableScan(table=[[scott, EMP]])\n"
+        + "    LogicalFilter(condition=[=($0, $cor0.$f1)])\n"
+        + "      LogicalTableScan(table=[[scott, DEPT]])\n";
+    assertThat(after, hasTree(planAfter));
+  }
+
+  /** Tests that a constant call operand, such as {@code POWER(2, 3)}, does
+   * not prevent extracting the enclosing correlated call. */
+  @Test void testCorrelationCallWithConstantCallOperandInFilter() {
+    final RelBuilder builder = RelBuilder.create(config().build());
+    final Holder<@Nullable RexCorrelVariable> v = Holder.empty();
+    RelNode before = builder.scan("EMP")
+        .variable(v::set)
+        .scan("DEPT")
+        .filter(
+            builder.equals(builder.field(0),
+                builder.call(SqlStdOperatorTable.PLUS,
+                    builder.call(SqlStdOperatorTable.POWER,
+                        builder.literal(2), builder.literal(3)),
+                    builder.field(v.get(), "DEPTNO"))))
+        .correlate(JoinRelType.LEFT, v.get().id, builder.field(2, 0, "DEPTNO"))
+        .build();
+
+    final String planBefore = ""
+        + "LogicalCorrelate(correlation=[$cor0], joinType=[left], requiredColumns=[{7}])\n"
+        + "  LogicalTableScan(table=[[scott, EMP]])\n"
+        + "  LogicalFilter(condition=[=($0, +(POWER(2, 3), $cor0.DEPTNO))])\n"
+        + "    LogicalTableScan(table=[[scott, DEPT]])\n";
+    assertThat(before, hasTree(planBefore));
+
+    RelNode after = before.accept(new CorrelateProjectExtractor(RelFactories.LOGICAL_BUILDER));
+    final String planAfter = ""
+        + "LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], DEPTNO0=[$9], DNAME=[$10], LOC=[$11])\n"
+        + "  LogicalCorrelate(correlation=[$cor0], joinType=[left], requiredColumns=[{8}])\n"
+        + "    LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], $f8=[+(POWER(2, 3), $7)])\n"
+        + "      LogicalTableScan(table=[[scott, EMP]])\n"
+        + "    LogicalFilter(condition=[=($0, $cor0.$f8)])\n"
+        + "      LogicalTableScan(table=[[scott, DEPT]])\n";
+    assertThat(after, hasTree(planAfter));
+  }
+
   @Test void testDoubleCorrelationCallOverVariableInFilters() {
     final RelBuilder builder = RelBuilder.create(config().build());
     final Holder<@Nullable RexCorrelVariable> v = Holder.empty();