julianhyde commented on code in PR #4328:
URL: https://github.com/apache/calcite/pull/4328#discussion_r2060712755


##########
core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java:
##########
@@ -561,6 +682,24 @@ public TrimResult trimFields(
     return result(newProject, mapping, project);
   }
 
+  private RexNode changeCorrelateReferences(
+      RexSubQuery node,
+      CorrelationId corrId,
+      RelDataType rowType,
+      Mapping inputMapping
+  ) {

Review Comment:
   remove newline before ')'



##########
core/src/test/resources/sql/sub-query.iq:
##########
@@ -63,6 +63,20 @@ select * from dept where deptno not in (select deptno from 
emp);
 (0 rows)
 
 !ok
+
+# [CALCITE-5638] Assertion Failure during planning correlated query

Review Comment:
   split query over multiple lines, one clause per line, for readability.



##########
core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java:
##########
@@ -473,6 +479,107 @@ public TrimResult trimFields(
     return result(newCalc, mapping, calc);
   }
 
+  /**
+   * Shuttle that finds all {@link TableScan}`s inside a given {@link RelNode}.
+   */
+  private static class TableScanCollector extends RelHomogeneousShuttle {
+    private ImmutableSet.Builder<List<String>> builder = 
ImmutableSet.builder();
+
+    /** Qualified names. */
+    Set<List<String>> tables() {
+      return builder.build();
+    }
+
+    @Override public RelNode visit(TableScan scan) {
+      builder.add(scan.getTable().getQualifiedName());
+      return super.visit(scan);
+    }
+  }
+
+  /**
+   * Shuttle that finds all {@link TableScan}`s inside a given {@link RexNode}.
+   */
+  private static class InputTablesVisitor extends RexVisitorImpl<Void> {
+    private ImmutableSet.Builder<List<String>> builder = 
ImmutableSet.builder();
+
+    protected InputTablesVisitor() {
+      super(true);
+    }
+
+    /** Qualified names. */
+    Set<List<String>> tables() {
+      return builder.build();
+    }
+
+    @Override public Void visitSubQuery(RexSubQuery subQuery) {
+      if (subQuery.getKind() == SqlKind.SCALAR_QUERY) {
+        subQuery.rel.accept(new RelHomogeneousShuttle() {
+          @Override public RelNode visit(TableScan scan) {
+            builder.add(scan.getTable().getQualifiedName());
+            return super.visit(scan);
+          }
+        });
+      }
+      return null;
+    }
+  }
+
+  /** Extension of {@link RelOptUtil.InputFinder} with subquery lookup. */
+  private static class SubQueryAwareInputFinder extends RelOptUtil.InputFinder 
{

Review Comment:
   I think you should extend InputFinder. Most people who use InputFinder will 
want to visit subqueries.
   
   Which means that this subclass (or boolean constructor parameter) is in 
RelOptUtil, which means that you don't need to drill holes (adding `protected` 
etc.)



##########
core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java:
##########
@@ -473,6 +479,107 @@ public TrimResult trimFields(
     return result(newCalc, mapping, calc);
   }
 
+  /**
+   * Shuttle that finds all {@link TableScan}`s inside a given {@link RelNode}.
+   */
+  private static class TableScanCollector extends RelHomogeneousShuttle {
+    private ImmutableSet.Builder<List<String>> builder = 
ImmutableSet.builder();
+
+    /** Qualified names. */
+    Set<List<String>> tables() {
+      return builder.build();
+    }
+
+    @Override public RelNode visit(TableScan scan) {
+      builder.add(scan.getTable().getQualifiedName());
+      return super.visit(scan);
+    }
+  }
+
+  /**
+   * Shuttle that finds all {@link TableScan}`s inside a given {@link RexNode}.
+   */
+  private static class InputTablesVisitor extends RexVisitorImpl<Void> {

Review Comment:
   Should there a boolean flag to `RexVisitorImpl`'s constructor that says 
whether to visit subqueries?



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