[
https://issues.apache.org/jira/browse/CALCITE-3118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16859078#comment-16859078
]
Danny Chan edited comment on CALCITE-3118 at 6/8/19 2:20 AM:
-------------------------------------------------------------
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 no contract that the `RelOptRule` must match same convention nodes
which would reduce many promotion possibilities.
was (Author: danny0405):
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)
