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


##########
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:
   I didn't notice that the new test in `sub-query.iq` was passing from here, 
great! Since there are tests then I guess it is OK to keep the changes as part 
of this PR.
   
   > If there is a test case which is affected by this change, then it was 
already wrong.
   It may be wrong but not always, right? This is a rough check as we discussed 
not precise.
   
   Moreover if there is a sub-query in the top project then field trimmer will 
stop without doing anything. I think we can do better than that. Is it valid to 
trim its input? I mean something like the following:
   
   ```java
   if (containsSubQuery) {
     return trimFields((RelNode) project, fieldsUsed, extraFields);
   }
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to