[
https://issues.apache.org/jira/browse/CALCITE-2223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16759052#comment-16759052
]
Vladimir Sitnikov commented on CALCITE-2223:
--------------------------------------------
{quote}If I understand the intention of PR-1019 correctly, it will
significantly reduce matching rules by allowing applying rules to the rel nodes
with concrete convention either specified explicitly or deduced from passed
RelBuilderFactory.
{quote}
The intention for PR-1019 is to **always** have at least a single trait for the
top-most rule operand.
For instance, if root rule operand is LogicalProject, then PR-1019 makes
Calcite know that LogicalProject implies Convention.NONE trait.
On the other hand, if rule is declared as super(operand(Project.class,... then
no traits is known. Then Calcite tries see other sources.
For instance, if operand was declared as super(operand(Project.class,
DrillConvention.DRILL_LOGICAL, ... then PR-1019 knows the rule is "good enough"
as it has at least DRILL_LOGICAL trait.
Then Calcite considers RelBuilderFactory in case it can supply traits.
This enables to declare a single rule class, and instantiate multiple rules out
of it by supplying builderfactories that provide different traits.
For instance:
{code:java}
public FilterMergeRule(RelBuilderFactory relBuilderFactory) {
super(
operand(Filter.class,
operand(Filter.class, any())),...
FilterMergeRule FILTER_MERGE_RULE =
new FilterMergeRule(DrillRelFactories.LOGICAL_BUILDER);
FilterMergeRule DRILL_FILTER_MERGE_RULE =
new
FilterMergeRule(DrillRelBuilder.proto(DrillRelFactories.DRILL_LOGICAL_FILTER_FACTORY));
{code}
Even though {{FilterMergeRule}} is declared as {{Filter.class}}, the first rule
would match just filters that have Convention.NONE (which is effectively
LogicalFilter), and the second one would match DrillFilterRel only since the
second rule would infer "DRILL_LOGICAL" trait for the rule root.
{quote}But the same rules which are used for the logical stage are also applied
at the physical stage to allow optimizing the plan after applying physical
rules.
{quote}
Note: are you sure the very same rules are used? I guess you use different rule
instances.
Does it really make much sense in creating Logical* relations at physical
planning phase?
I think at physical stage the rules should use PHYSICAL_BUILDER or something
like that.
{quote}Not sure that the case LogicalProject(FilterPrel(...)) is applicable for
the rules, but there are converters, which may convert LogicalProject to
DrillProjectRel and then to ProjectPrel, so the rule may be applied at least to
ProjectPrel(FilterPrel(...))
{quote}
Apparently converters work since lots of tests pass (everything in Calcite
works, and lots of Drill tests work as well).
{quote}In particular, I think the problem here is with joins reordering
{quote}
I believe the main reason tests fail is because I've disabled Join to MultiJoin
rule in Drill (since MultiJoin always produces Convention.NONE and it can't be
used at physical stage).
I'm going to try adding a DrillMultiJoinRel so multijoins work properly for
different conventions.
{quote}From the logs for failing
TestExampleQueries.testMultipleComparisonWithSingleValueSubQuery test I see a
lot of the following entries:
Rel {} does not match rule {} since the relation does not match implicit traits
{}{quote}
That is more or less fine. It would be nice to optimize planner as well.
Currently it picks rules based on class only, however it could use <Class,
TraitSet> combo as well.
> ProjectMergeRule is infinitely matched when is applied after
> ProjectReduceExpressionsRule
> -----------------------------------------------------------------------------------------
>
> Key: CALCITE-2223
> URL: https://issues.apache.org/jira/browse/CALCITE-2223
> Project: Calcite
> Issue Type: Bug
> Reporter: Volodymyr Vysotskyi
> Assignee: Julian Hyde
> Priority: Critical
> Attachments:
> TestLimitWithExchanges_testPushLimitPastUnionExchange.png, heap_overview.png,
> provenance_contents.png
>
>
> For queries like this:
> {code:sql}
> select t1.f from (select cast(f as int) f, f from (select cast(f as int) f
> from (values('1')) t(f))) as t1
> {code}
> OOM is thrown when {{ProjectMergeRule}} is applied before applying
> {{ProjectReduceExpressionsRule}} in VolcanoPlanner.
> A simple test to reproduce this issue (in {{RelOptRulesTest}}):
> {code:java}
> @Test public void testOomProjectMergeRule() {
> RelBuilder relBuilder =
> RelBuilder.create(RelBuilderTest.config().build());
> RelNode relNode = relBuilder
> .values(new String[]{"f"}, "1")
> .project(
> relBuilder.alias(
> relBuilder.cast(relBuilder.field(0), SqlTypeName.INTEGER),
> "f"))
> .project(
> relBuilder.alias(
> relBuilder.cast(relBuilder.field(0), SqlTypeName.INTEGER),
> "f0"),
> relBuilder.alias(relBuilder.field(0), "f"))
> .project(
> relBuilder.alias(relBuilder.field(0), "f"))
> .build();
> RelOptPlanner planner = relNode.getCluster().getPlanner();
> RuleSet ruleSet =
> RuleSets.ofList(
> ReduceExpressionsRule.PROJECT_INSTANCE,
> new ProjectMergeRuleWithLongerName(),
> EnumerableRules.ENUMERABLE_PROJECT_RULE,
> EnumerableRules.ENUMERABLE_VALUES_RULE);
> Program program = Programs.of(ruleSet);
> RelTraitSet toTraits =
> relNode.getCluster().traitSet()
> .replace(0, EnumerableConvention.INSTANCE);
> RelNode output = program.run(planner, relNode, toTraits,
> ImmutableList.<RelOptMaterialization>of(),
> ImmutableList.<RelOptLattice>of());
> // check for output
> }
> /**
> * ProjectMergeRule inheritor which has
> * class name greater than ProjectReduceExpressionsRule class name
> (String.compareTo()).
> *
> * It is needed for RuleQueue.popMatch() method
> * to apply this rule before ProjectReduceExpressionsRule.
> */
> private static class ProjectMergeRuleWithLongerName extends
> ProjectMergeRule {
> public ProjectMergeRuleWithLongerName() {
> super(true, RelFactories.LOGICAL_BUILDER);
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
