zabetak commented on code in PR #2813:
URL: https://github.com/apache/calcite/pull/2813#discussion_r977414870


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProjectRule.java:
##########
@@ -42,6 +43,14 @@ protected EnumerableProjectRule(Config config) {
     super(config);
   }
 
+  @Override public boolean matches(RelOptRuleCall call) {
+    LogicalProject project = call.rel(0);
+    if (!project.getVariablesSet().isEmpty()) {
+      return false;
+    }
+    return true;

Review Comment:
   Replace with `return project.getVariablesSet().isEmpty();` Idem to the other 
rules.



##########
core/src/test/java/org/apache/calcite/plan/RelWriterTest.java:
##########
@@ -998,6 +999,21 @@ void testCorrelateQuery(SqlExplainFormat format) {
         .assertThatPlan(isLinux(expected));
   }
 
+  @Test void testProjectionWithCorrelationVaribles() {
+    final Function<RelBuilder, RelNode> relFn = b -> b.scan("EMP")
+        .project(
+            ImmutableList.of(b.field("ENAME")),
+            ImmutableList.of("ename"),
+            true,
+            ImmutableSet.of(b.getCluster().createCorrel()))
+        .build();
+
+    final String expected = "LogicalProject(variablesSet=[[$cor0]], 
ename=[$1])\n"
+        + "  LogicalTableScan(table=[[scott, EMP]])\n";

Review Comment:
   Agreed!



##########
core/src/main/java/org/apache/calcite/tools/RelBuilder.java:
##########
@@ -1817,7 +1817,24 @@ public RelBuilder project(Iterable<? extends RexNode> 
nodes,
    */
   public RelBuilder project(Iterable<? extends RexNode> nodes,
       Iterable<? extends @Nullable String> fieldNames, boolean force) {
-    return project_(nodes, fieldNames, ImmutableList.of(), force);
+    return project(nodes, fieldNames, force, ImmutableSet.of());
+  }
+
+  /**
+   * The same with {@link #project(Iterable, Iterable, boolean)}, with 
additional
+   * variablesSet param.
+   *
+   * @param nodes Expressions
+   * @param fieldNames Suggested field names
+   * @param force create project even if it is identity
+   * @param variablesSet Correlating variables that are set when reading a row
+   *                     from the input, and which may be referenced from the
+   *                     projection expressions
+   */
+  public RelBuilder project(Iterable<? extends RexNode> nodes,
+      Iterable<? extends @Nullable String> fieldNames, boolean force,
+      Iterable<CorrelationId> variablesSet) {
+    return project_(nodes, fieldNames, ImmutableList.of(), force, 
variablesSet);

Review Comment:
   There are certain disadvantages with the approach I suggested. We can take 
advantage of `variablesSet` in various places such as `RelFieldTrimmer` so 
overall it makes sense to move forward with this change. Marking this as 
resovled.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to