jackwener commented on code in PR #14438:
URL: https://github.com/apache/doris/pull/14438#discussion_r1030067787


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/joinreorder/hypergraph/CircleDetector.java:
##########
@@ -39,18 +39,16 @@ public class CircleDetector {
     List<Integer> nodes = new ArrayList<>();
     // stored the dependency of each node
     List<BitSet> directedEdges = new ArrayList<>();
-    List<BitSet> forbiddenEdges = new ArrayList<>();
+    List<BitSet> subNodes = new ArrayList<>();
     // whether the node has been visited in dfs

Review Comment:
   comment location wrong.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/joinreorder/hypergraph/CircleDetector.java:
##########
@@ -62,19 +60,41 @@ public class CircleDetector {
      */
     public boolean tryAddDirectedEdge(int node1, int node2) {
         // Add dependency node1 -> node2
-        if (!checkCircleWithEdge(node1, node2)) {
-            Bitmap.set(directedEdges.get(node1), node2);
-            Bitmap.set(forbiddenEdges.get(node2), node1);
-            return true;
+        if (checkCircleWithEdge(node1, node2)) {
+            return false;
+        }
+        Bitmap.set(directedEdges.get(node1), node2);
+        int order1 = orders.get(node1);
+        int order2 = orders.get(node2);
+        if (order1 >= order2) {
+            shift(order2, order1 + 1, subNodes.get(order2));
+        }
+        for (BitSet nodes : subNodes) {
+            if (Bitmap.get(nodes, node1)) {
+                Bitmap.or(nodes, subNodes.get(node2));
+            }

Review Comment:
   add comment
   // add all subNodes which contains node1 into subNodes of node2.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/joinreorder/hypergraph/CircleDetector.java:
##########
@@ -62,19 +60,41 @@ public class CircleDetector {
      */
     public boolean tryAddDirectedEdge(int node1, int node2) {
         // Add dependency node1 -> node2
-        if (!checkCircleWithEdge(node1, node2)) {
-            Bitmap.set(directedEdges.get(node1), node2);
-            Bitmap.set(forbiddenEdges.get(node2), node1);
-            return true;
+        if (checkCircleWithEdge(node1, node2)) {
+            return false;
+        }
+        Bitmap.set(directedEdges.get(node1), node2);
+        int order1 = orders.get(node1);
+        int order2 = orders.get(node2);
+        if (order1 >= order2) {
+            shift(order2, order1 + 1, subNodes.get(order2));

Review Comment:
   look like subNodes index is `order` instead of `node`



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to