[
https://issues.apache.org/jira/browse/CALCITE-2456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Zuozhi Wang updated CALCITE-2456:
---------------------------------
Description:
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.
was:
h3. 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).
h3. 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`).
h3. 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.
h3. 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.
> 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
> 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)
