clesaec commented on code in PR #3050:
URL: https://github.com/apache/calcite/pull/3050#discussion_r1090316349
##########
core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java:
##########
@@ -737,6 +745,7 @@ public RelNode visit(
planner.provenanceMap.put(
p, new VolcanoPlanner.DirectProvenance(pOld));
}
+ visited.put(pId, p); // memoize result for pId
Review Comment:
As it's a recursive method (line 739), it may be not so simple ? at least,
should have a unit test with recursion, at most; move the content of method in
new private one; and keep memoize in public one :
```java
public RelNode visit( RelNode p, int ordinal, @Nullable RelNode parent) {
final int pId = p.getId();
if (visited.containsKey(pId)) {
// return memoized result if available
return visited.get(pId);
}
RelNode result = visit_inside( p, ordinal, parent);
visited.put(pId, result);
}
private RelNode visit_inside( RelNode p, int ordinal, @Nullable RelNode
parent) {
...
RelNode input = visit_inside(oldInput, i, p);
...
}
```
--
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]
