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