[
https://issues.apache.org/jira/browse/DRILL-5159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15795476#comment-15795476
]
ASF GitHub Bot commented on DRILL-5159:
---------------------------------------
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/705#discussion_r94437725
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java
---
@@ -48,6 +50,12 @@ public boolean matches(RelOptRuleCall call) {
Project topProject = call.rel(0);
Project bottomProject = call.rel(1);
+ // Make sure both projects be LogicalProject.
+ if (topProject.getTraitSet().getTrait(ConventionTraitDef.INSTANCE) !=
Convention.NONE ||
--- End diff --
At first glance, it is non-intuitive that a Drill specific MergeProjectRule
returns False if a DRILL_LOGICAL convention is encountered. However,
currently, this rule only checks for the complex expression (e.g
convert_to_json), so your change is fine in the context of the current rule.
In future, would we consider allowing merging 2 DrillProjectRels ? (I think
yes).
> ProjectMergeRule in Drill should operate on RelNodes with same convention
> trait.
> --------------------------------------------------------------------------------
>
> Key: DRILL-5159
> URL: https://issues.apache.org/jira/browse/DRILL-5159
> Project: Apache Drill
> Issue Type: Bug
> Components: Query Planning & Optimization
> Reporter: Jinfeng Ni
> Assignee: Aman Sinha
>
> Drill extended version of Calcite's ProjectMergeRule is used in a
> VolcanoPlanner where RelNodes with different convention could match with this
> rule.
> For instance, we could see this rule could be invoked when a DrillProject on
> top of a LogicalProject. Also, since the output RelNode is built from the
> default Project RelFactory, such rule execution could end up with a
> LogicalProject.
> {code}
> DrillProject transform
> \ ===> LogicalProject
> LogicalProject
> {code}
>
> This leads to un-necessary rule execution, or in certain case could lead to
> an infinite loop.
> The proposed fix is to check matched RelNodes to make sure that they do have
> Calcite Logical convention. That way, both inputs and output of this rule
> would have same convention trait. This should reduce planning time, and
> avoid the possiblity of loop.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
