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

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

Since [~vlsi] has reviewed this, I think we're nearly there. I would like two 
things changed. First, resolve the issue that Vladimir raised about successors 
when the operand matched on RelSubset. That is a very particular case that does 
not behave like other cases.

Second, Make sure that the commit comment doesn't start with the word "Fix". 
I'd prefer that it contains a good description of the problem, which as it 
happens this Jira case already does. In subsequent lines of the commit message 
you can describe what you did to solve the problem, if you like, but more 
important is a description of the problem.

With those, I am +1.

> 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:
> See the test cases in https://github.com/apache/calcite/pull/784
> 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`:
> See [https://github.com/apache/calcite/pull/784]
>  
> Thanks.



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

Reply via email to