[ 
https://issues.apache.org/jira/browse/CALCITE-2456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16573827#comment-16573827
 ] 

Julian Hyde commented on CALCITE-2456:
--------------------------------------

I can well believe that there is a bug in Volcano for handling "unordered".

I'll just note that HepPlanner seems to handle "unordered" correctly. 
{{RelOptRulesTest.testEmptyFilterProjectUnion}} would fail if it did not work: 
a union has two inputs, the second is empty, and gets pruned.

> VolcanoRuleCall#match works incorrectly for unordered child operand
> -------------------------------------------------------------------
>
>                 Key: CALCITE-2456
>                 URL: https://issues.apache.org/jira/browse/CALCITE-2456
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.17.0
>         Environment: Test on Calcite master branch as of 8/7/2018 with Java 8.
>            Reporter: Zuozhi Wang
>            Assignee: Julian Hyde
>            Priority: Major
>         Attachments: UnorderedBugTest.java
>
>
> h2. Bug Description:
> This bug occurs when there'a rule matching a Union operator with an unordered 
> operand: 
> `operand(Union.class, unordered(operand(RelTypeB.class, any())))`,
> with an plan tree that the operand to match is not the first input to the 
> union:
> LogicalUnion
>    RelTypeA
>    RelTypeB
> The expected behavior is that this plan tree should fire the match, because 
> `unordered` means matching any child of the union operator.
> The bug is that the tree is *not* matched as expected, either matching in 
> descending order (rule is triggered from LogicalUnion) or in ascending order 
> (rule is triggered by adding another node equivalent to RelTypeB).
> h2. How to reproduce:
> I have attached a Java file `UnorderedBugTest.java` that contains two test 
> cases to reproduce this issue. The Java file can be copied under folder 
> `calcite/core/src/test/java/org/apache/calcite/plan/volcano/` (same folder as 
> `VolcanoPlannerTest`).
> h2. Bug cause:
> The cause of this issue is that VolcanoRuleCall doesn't handle `unordered` 
> child operand at all. It uses `operand.ordinalInParent` to check if the 
> matched RelNode matches the operand's ordinal(position) in parent's inputs. 
> The value of `ordinalInParent` is always `0` in this case, requires the 
> matched RelNode to also be the first input.
> However, that only makes sense when child policy is `some`, which strictly 
> requires to match in order. For child policy `unordered`, it should match 
> regardless of the position of RelNode in the inputs.
> h2. Bug fix:
> This bug can be fixed with changes to function`VolcanoRuleCall#matchRecurse`:
>  In line 288: (fix matching in descending order)
>  
> {code:java}
> if (parentOperand.childPolicy.equals(RelOptRuleOperandChildPolicy.UNORDERED)) 
> {
>   List<RelNode> allRelsInAllSubsets = new ArrayList<>();
>   for (RelNode input : inputs) {
>     RelSubset inputSubset = (RelSubset) input;
>     List<RelNode> subsetRels = inputSubset.getRelList();
>     allRelsInAllSubsets.addAll(inputSubset.getRelList());
>   }
>   successors = allRelsInAllSubsets;
> } else {
>   // original code in here
>   if (operand.ordinalInParent < inputs.size())
>   ...
> }
> {code}
> In original line 307: (fix matching in ascending order)
>  change original 
>  `if (ascending)`
>  to
>  `if (ascending && ! 
> operand.childPolicy.equals(RelOptRuleOperandChildPolicy.UNORDERED))`
>  
> Thanks.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to