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

Danny Chan commented on CALCITE-3118:
-------------------------------------

I'm -1 on this fix.

Just like you said, _AssertOperandsDifferentRule's_ match pattern is: 
{code:java}
    PhysBiRel
    /       \
leftPhy  rightPhy{code}
 

Firstly you construct a RelNode tree like what the pattern describes, then you 
register that the `leftPhy` and `rightPhy` are equivalent. It's not that 
surprising you got a tree like:
{code:java}
    PhysBiRel
    /       \
rightPhy  rightPhy
{code}
This tree also satisfied _AssertOperandsDifferentRule_ patttern. 

This is Volcano Planner so the possibility space boost. BTW, the tree you build 
manually would still be matched.

We have contract that the `RelOptRule` must match same convention nodes which 
would reduce many promotion possibilities.

> VolcanoRuleCall match parent child ordinal not properly checked
> ---------------------------------------------------------------
>
>                 Key: CALCITE-3118
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3118
>             Project: Calcite
>          Issue Type: Bug
>            Reporter: Botong Huang
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> In VolcanoRuleCall.matchRecurse(), when ascending (child operand is matched, 
> looking for parent operand match), we want to check that the matched parent 
> indeed has the previously matched child RelNode as a child with the expected 
> ordinal. However, there is a bug in this check. As a result, some incorrect 
> parent is not skipped as expected and matched incorrectly. See unit test 
> included in PR for a case that triggers this bug, where a second child 
> RelNode get matched as first child of the parent RelNode. 



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

Reply via email to