danny0405 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_r291459410
##########
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:
I don't know why you make `leftPhy` and `rightPhy` equivalent, If they
were, the behavior is correct without your fix, because RelOptRule only matches
the RelNode class, not the convention, if you really want to match specific
convention, add a predicate on it :)
----------------------------------------------------------------
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