mihaibudiu commented on code in PR #4328:
URL: https://github.com/apache/calcite/pull/4328#discussion_r2058873870
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -4556,10 +4556,10 @@ private void acceptFields(final List<RelDataTypeField>
fields) {
* Visitor which builds a bitmap of the inputs used by an expression.
*/
public static class InputFinder extends RexVisitorImpl<Void> {
- private final ImmutableBitSet.Builder bitBuilder;
- private final @Nullable Set<RelDataTypeField> extraFields;
+ protected final ImmutableBitSet.Builder bitBuilder;
+ protected final @Nullable Set<RelDataTypeField> extraFields;
- private InputFinder(@Nullable Set<RelDataTypeField> extraFields,
+ public InputFinder(@Nullable Set<RelDataTypeField> extraFields,
Review Comment:
I think this can be protected too
##########
core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java:
##########
@@ -473,6 +478,117 @@ public TrimResult trimFields(
return result(newCalc, mapping, calc);
}
+ /**
+ * Shuttle that finds all references to a {@link TableScan} within a tree
+ * of {@link RelNode}s.
+ */
+ private static class InputTables extends RelHomogeneousShuttle {
+ private List<TableScan> tables = new ArrayList<>();
Review Comment:
Could this be a Set? (I am not sure there is a benefit)
##########
core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java:
##########
@@ -910,6 +911,13 @@ private static void matchJoin(SubQueryRemoveRule rule,
RelOptRuleCall call) {
int nFieldsLeft = join.getLeft().getRowType().getFieldCount();
int nFieldsRight = join.getRight().getRowType().getFieldCount();
+ // Correlation columns are also should be considered.
+ final Set<CorrelationId> variablesSet = RelOptUtil.getVariablesUsed(e.rel);
+ if (!variablesSet.isEmpty()) {
+ final CorrelationId id = Iterables.getOnlyElement(variablesSet);
Review Comment:
why is there always an only element?
Can't people write nested correlated queries?
##########
core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java:
##########
@@ -473,6 +478,117 @@ public TrimResult trimFields(
return result(newCalc, mapping, calc);
}
+ /**
+ * Shuttle that finds all references to a {@link TableScan} within a tree
+ * of {@link RelNode}s.
+ */
+ private static class InputTables extends RelHomogeneousShuttle {
+ private List<TableScan> tables = new ArrayList<>();
+
+ List<TableScan> tables() {
+ return tables;
+ }
+
+ @Override public RelNode visit(TableScan scan) {
+ tables.add(scan);
+ return super.visit(scan);
+ }
+ }
+
+ /**
+ * Shuttle that finds all references to a {@link TableScan} within current
{@link RexNode}.
+ */
+ private static class InputTablesVisitor extends RexVisitorImpl<Void> {
+ private List<TableScan> tables = new ArrayList<>();
+
+ protected InputTablesVisitor() {
+ super(true);
+ }
+
+ List<TableScan> tables() {
+ return tables;
+ }
+
+ @Override public Void visitSubQuery(RexSubQuery subQuery) {
+ if (subQuery.getKind() == SqlKind.SCALAR_QUERY) {
+ subQuery.rel.accept(new RelHomogeneousShuttle() {
+ @Override public RelNode visit(TableScan scan) {
+ tables.add(scan);
+ return super.visit(scan);
+ }
+ });
+ }
+ return null;
+ }
+ }
+
+ /** Extension of {@link RelOptUtil.InputFinder} with subquery lookup. */
+ private static class SubQueryAwareInputFinder extends RelOptUtil.InputFinder
{
+ boolean visitSubQuery;
+
+ SubQueryAwareInputFinder(@Nullable Set<RelDataTypeField> extraFields,
boolean visitSubQuery) {
+ super(extraFields, ImmutableBitSet.builder());
+ this.visitSubQuery = visitSubQuery;
+ }
+
+ @Override public Void visitSubQuery(RexSubQuery subQuery) {
+ if (visitSubQuery && subQuery.getKind() == SqlKind.SCALAR_QUERY) {
+ subQuery.rel.accept(new RelHomogeneousShuttle() {
+ @Override public RelNode visit(LogicalProject project) {
+ project.getProjects().forEach(r ->
r.accept(SubQueryAwareInputFinder.this));
+ return super.visit(project);
+ }
+
+ @Override public RelNode visit(LogicalFilter filter) {
+ filter.getCondition().accept(SubQueryAwareInputFinder.this);
+ return super.visit(filter);
+ }
+ });
+
+ return null;
+ } else {
+ return super.visitSubQuery(subQuery);
+ }
+ }
+ }
+
+ private boolean inputContainSubQueryTables(Project project, RelNode input) {
+ InputTablesVisitor inputSubQueryTablesCollector = new InputTablesVisitor();
+
+ for (RexNode node : project.getProjects()) {
+ node.accept(inputSubQueryTablesCollector);
+ }
+
+ List<TableScan> subQueryTables = inputSubQueryTablesCollector.tables();
+
+ List<String> qName = null;
+ for (TableScan t : subQueryTables) {
Review Comment:
This would be simpler by using a `Set<String>` for the qualified names.
But I don't understand why there is only one table.
##########
core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java:
##########
@@ -473,6 +478,117 @@ public TrimResult trimFields(
return result(newCalc, mapping, calc);
}
+ /**
+ * Shuttle that finds all references to a {@link TableScan} within a tree
Review Comment:
the java doc suggests that this finds references to a particular TableScan,
but this finds all references to table scans. I would also call this FindTables
##########
core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java:
##########
@@ -473,6 +478,117 @@ public TrimResult trimFields(
return result(newCalc, mapping, calc);
}
+ /**
+ * Shuttle that finds all references to a {@link TableScan} within a tree
+ * of {@link RelNode}s.
+ */
+ private static class InputTables extends RelHomogeneousShuttle {
+ private List<TableScan> tables = new ArrayList<>();
+
+ List<TableScan> tables() {
+ return tables;
+ }
+
+ @Override public RelNode visit(TableScan scan) {
+ tables.add(scan);
+ return super.visit(scan);
+ }
+ }
+
+ /**
+ * Shuttle that finds all references to a {@link TableScan} within current
{@link RexNode}.
Review Comment:
same comment as above
##########
core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java:
##########
@@ -561,6 +691,19 @@ public TrimResult trimFields(
return result(newProject, mapping, project);
}
+ private RexNode changeSubQuery(RexSubQuery node, CorrelationId corrId,
RelDataType rowType,
Review Comment:
can this function have a more descriptive name?
this could mean anything
--
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]
