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