[
https://issues.apache.org/jira/browse/DRILL-5286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15949726#comment-15949726
]
ASF GitHub Bot commented on DRILL-5286:
---------------------------------------
Github user chunhui-shi commented on a diff in the pull request:
https://github.com/apache/drill/pull/797#discussion_r109025164
--- 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 --
Change to use newIdentityHashSet.
And we could not simply mark a node is isTransformed, since there could be
many rules transforming the same node. Maintaining a map of applied rules in
relNode might not be a good idea since remembering what rule has been applied
should not be the job of a relNode(set). So what about maintaining such map in
planner? Not a good idea since the same node might need to be converted into
different sets. So I would think leaving this part as is.
> When rel and target candidate set is the same, planner should not need to do
> convert for the relNode since it must have been done
> ---------------------------------------------------------------------------------------------------------------------------------
>
> Key: DRILL-5286
> URL: https://issues.apache.org/jira/browse/DRILL-5286
> Project: Apache Drill
> Issue Type: Bug
> Reporter: Chunhui Shi
> Assignee: Chunhui Shi
>
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
