Github user jinfengni commented on the pull request:
https://github.com/apache/drill/pull/444#issuecomment-201151771
Adding assertion to DrillAggregateRel did not show new failure when using
the default FilterAggregateTransposeRule. In the rule, copy() method is used to
create new instance of aggregate. So, the aggregate created by this rule should
not violate the convention check.
The patten of mixed rels does happen and it's the expected thing, since
Drill's logical planning is dealing with two convention: NONE and LOGICAL. So,
at any time, the RelSet would have RelSubSets for both conventions. It's normal
to see a LogicalFilter whose input is a RelSubset with a DrillAggregate as a
member.
The problem is not we should not have such mixed patten. In stead, the
problem is that the Rule should not match such pattern. If the rule matches
such mixed Rel, then looks like it could cause problem. With the default rule,
the trace of running 50 seconds shows FilterAggregateTransposeRule is fired
abnormally high (it could be even high if it does not timeout)
1055 [FilterAggregateTransposeRule]
45 [DrillPushProjectPastFilterRule]
38 [ProjectMergeRule:force_mode]
32 [DrillReduceExpressionsRule(Filter)]
30 [ReduceExpressionsRule_Project]
30 [DrillProjectRule]
26 [DrillAggregateRule]
If we add the restriction, then the new rule was just fired coupled of
times.
---
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.
---
