libenchao commented on code in PR #2911:
URL: https://github.com/apache/calcite/pull/2911#discussion_r974896291


##########
core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java:
##########
@@ -481,6 +481,18 @@ public TrimResult trimFields(
     final int fieldCount = rowType.getFieldCount();
     final RelNode input = project.getInput();
 
+    boolean containsSubQuery = false;
+    for (RexNode node : project.getProjects()) {
+      if (RexUtil.containsSubQuery(node)) {
+        containsSubQuery = true;
+        break;
+      }
+    }
+    // Do not trim Project's fields before SubQueryRemoveRule applies.
+    if (containsSubQuery) {
+      return result(project, Mappings.createIdentity(fieldCount));
+    }
+

Review Comment:
   The test added in `sub-query.iq` will fail without this change. It surprised 
me too that this does not affect any existing plan tests. I guess that's 
because we don't have enough plan tests for sub query. If there is a test case 
which is affected by this change, then it was already wrong.
   
   Due to the integrity of this issue I think this is ok to keep it in the same 
PR, WDYT?



-- 
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