Mulavar commented on a change in pull request #2501:
URL: https://github.com/apache/calcite/pull/2501#discussion_r703987485



##########
File path: core/src/test/java/org/apache/calcite/test/HepPlannerTest.java
##########
@@ -267,6 +271,38 @@ private void assertIncludesExactlyOnce(String message, 
String digest, String sub
     assertThat(listener.getApplyTimes() == 1, is(true));
   }
 
+  @Test void testCleanEdges() {
+    // this case is designed to test
+    // CALCITE-4747:https://issues.apache.org/jira/browse/CALCITE-4747
+    // before the improvement, the following optimization will have 7 edges in 
the graph,
+    // and now there's only 4 edges.
+    HepProgramBuilder programBuilder = HepProgram.builder();
+    programBuilder.addRuleInstance(CoreRules.FILTER_INTO_JOIN);
+
+    final HepTestListener listener = new HepTestListener(0);
+    HepPlanner planner = new HepPlanner(programBuilder.build());
+    planner.addListener(listener);
+
+    final String sql = "select * from emp e "
+        + "join dept d on e.deptno=d.deptno "
+        + "where e.empno>100";
+
+    try {
+      planner.setRoot(tester.convertSqlToRel(sql).rel);
+      planner.findBestExp();
+
+      // the field graph is private, so we use reflect to get it.
+      Field graphField = planner.getClass().getDeclaredField("graph");
+      graphField.setAccessible(true);

Review comment:
       exactly, there are 4 edges before the operation, but when u execute the 
optimization, it will add some new edges and cause the edges to grow before 
this commit, but in this commit, I remove the outdated edges, so the number of 
edges will keep 4, I think it's better not remove this integrate case, because 
this case describes the whole optimization, it may mean more than 
DirectedGraphTest.




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to