Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/797#discussion_r108094902
--- 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 --
No doubt that time can be saved by avoiding repeated work. When I was
tinkering with some Calcite rules I saw the same node being analyzed many, many
times for even a simple query.
Since you want to use identify (not equality) to compare hash map entries,
consider using `Sets.newIdentityHashSet()` in place of the equality-based
`Sets.newHashSet()`.
More efficient would be if we knew the type of the `RelNode`s. If we did,
and they all derive from a single Drill type, we could just add an
`isTransformed` flag to avoid the set overhead.
---
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.
---
