[
https://issues.apache.org/jira/browse/CALCITE-2456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Zuozhi Wang updated CALCITE-2456:
---------------------------------
Description:
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.
was:
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).
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`).
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.
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
>
>
> 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.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
