hbtoo commented on a change in pull request #1260: [CALCITE-3118] fix
VolcanoRuleCall match parent child ordinal check
URL: https://github.com/apache/calcite/pull/1260#discussion_r291593237
##########
File path:
core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTest.java
##########
@@ -122,6 +125,35 @@ public VolcanoPlannerTest() {
assertTrue(result instanceof PhysSingleRel);
}
+ @Test public void testMatchedOperandsDifferent() {
+ VolcanoPlanner planner = new VolcanoPlanner();
+ planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
+ RelOptCluster cluster = newCluster(planner);
+
+ // The rule that triggers the assert rule
+ planner.addRule(new PhysLeafRule());
+
+ // The rule asserting that the matched operands are different
+ planner.addRule(new AssertOperandsDifferentRule());
+
+ // Construct two children in the same set and a parent RelNode
+ NoneLeafRel leftRel = new NoneLeafRel(cluster, "a");
+ RelNode leftPhy = planner
+ .changeTraits(leftRel, cluster.traitSetOf(PHYS_CALLING_CONVENTION));
+ PhysLeafRel rightPhy =
+ new PhysLeafRel(cluster, PHYS_CALLING_CONVENTION_2, "b");
+
+ PhysBiRel parent =
+ new PhysBiRel(cluster, cluster.traitSetOf(PHYS_CALLING_CONVENTION),
+ leftPhy, rightPhy);
+ planner.setRoot(parent);
+
+ // Make sure both RelNodes are in the same set, but different subset
+ planner.ensureRegistered(leftPhy, rightPhy);
+
Review comment:
Thanks @danny0405 for reviewing! Having two relNodes of the same class in
two RelSubsets in the same RelSet (thus the equivalent line) is the necessary
condition to trigger this bug. However, using convention in the unit test is
not a must. It's merely to produce the two subSets. Any two traits that are
different would suffice.
Also as in AssertOperandsDifferentRule, it is not trying to match any
convention. We are simply trying to match a parent with its first and second
children being PhysLeafRel.class, it is wrong to return a match with a parent
relNode and both its second children.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
