Github user chunhui-shi commented on a diff in the pull request:
https://github.com/apache/drill/pull/797#discussion_r108052498
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/SubsetTransformer.java
---
@@ -45,15 +48,20 @@ public RelTraitSet newTraitSet(RelTrait... traits) {
}
- boolean go(T n, RelNode candidateSet) throws E {
+ public boolean go(T n, RelNode candidateSet) throws E {
if ( !(candidateSet instanceof RelSubset) ) {
return false;
}
boolean transform = false;
+ Set<RelNode> transformedRels = Sets.newHashSet();
for (RelNode rel : ((RelSubset)candidateSet).getRelList()) {
if (isPhysical(rel)) {
RelNode newRel = RelOptRule.convert(candidateSet,
rel.getTraitSet().plus(Prel.DRILL_PHYSICAL));
+ if(transformedRels.contains(newRel)) {
--- End diff --
this " if(transformedRels.contains(newRel))" is to check if the newRel is
the _same_ object we got before, it is not to check a 'equal' node. So no
hashCode or equal function required.
And yes, I did step into this code and it is doing what we want: for the
same node(n) to be converted in below convertChild() and the same equivalent
set(newRel), we don't need to run the rule again, since we are going to get the
same result, and that result will be added to the same set. So we are very sure
we could skip convertChild() for this pair of input(n, newRel) since this has
been called before.
In some cases it can reduce the same rule from running 16 times to 4 times
and saved hundreds ms.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
