[CALCITE-6846] fix code style

apache · Feb 21, 2025 · c62182e · c62182e
1 parent 658371b
commit c62182e
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 37 deletions.
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/DpHyp.java b/core/src/main/java/org/apache/calcite/rel/rules/DpHyp.java
@@ -16,6 +16,7 @@
  */
 package org.apache.calcite.rel.rules;
 
+import org.apache.calcite.plan.RelOptCost;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.metadata.RelMetadataQuery;
@@ -27,8 +28,6 @@
 import java.util.HashMap;
 import java.util.List;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
 /**
  * The core process of dphyp enumeration algorithm.
  */
@@ -127,10 +126,12 @@ private void enumerateCmpRec(long csg, long cmp, long forbidden) {
   }
 
   private void emitCsgCmp(long csg, long cmp, List<HyperEdge> edges) {
-    checkArgument(dpTable.containsKey(csg));
-    checkArgument(dpTable.containsKey(cmp));
     RelNode child1 = dpTable.get(csg);
     RelNode child2 = dpTable.get(cmp);
+    if (child1 == null || child2 == null) {
+      throw new IllegalArgumentException(
+          "csg and cmp were not enumerated in the previous dp process");
+    }
 
     JoinRelType joinType = hyperGraph.extractJoinType(edges);
     if (joinType == null) {
@@ -150,18 +151,13 @@ private void emitCsgCmp(long csg, long cmp, List<HyperEdge> edges) {
         .push(child1)
         .join(joinType, joinCond2)
         .build();
-    RelNode winPlan = mq.getCumulativeCost(newPlan1).isLt(mq.getCumulativeCost(newPlan2))
-        ? newPlan1
-        : newPlan2;
+    RelNode winPlan = chooseBetterPlan(newPlan1, newPlan2);
 
-    if (!dpTable.containsKey(csg | cmp)) {
-      dpTable.put(csg | cmp, winPlan);
-    } else {
-      RelNode oriPlan = dpTable.get(csg | cmp);
-      if (mq.getCumulativeCost(winPlan).isLt(mq.getCumulativeCost(oriPlan))) {
-        dpTable.put(csg | cmp, winPlan);
-      }
+    RelNode oriPlan = dpTable.get(csg | cmp);
+    if (oriPlan != null) {
+      winPlan = chooseBetterPlan(winPlan, oriPlan);
     }
+    dpTable.put(csg | cmp, winPlan);
   }
 
   public @Nullable RelNode getBestPlan() {
@@ -170,4 +166,14 @@ private void emitCsgCmp(long csg, long cmp, List<HyperEdge> edges) {
     return dpTable.get(wholeGraph);
   }
 
+  private RelNode chooseBetterPlan(RelNode plan1, RelNode plan2) {
+    RelOptCost cost1 = mq.getCumulativeCost(plan1);
+    RelOptCost cost2 = mq.getCumulativeCost(plan2);
+    if (cost1 == null || cost2 == null) {
+      return plan1;
+    } else {
+      return cost1.isLt(cost2) ? plan1 : plan2;
+    }
+  }
+
 }
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/HyperGraph.java b/core/src/main/java/org/apache/calcite/rel/rules/HyperGraph.java
@@ -51,25 +51,27 @@
  */
 public class HyperGraph extends AbstractRelNode {
 
-  private List<RelNode> inputs;
+  private final List<RelNode> inputs;
 
-  private List<HyperEdge> edges;
+  @SuppressWarnings("HidingField")
+  private final RelDataType rowType;
+
+  private final List<HyperEdge> edges;
 
   // key is the bitmap for inputs, value is the hyper edge bitmap in edges
-  private HashMap<Long, BitSet> simpleEdgesMap;
+  private final HashMap<Long, BitSet> simpleEdgesMap;
 
-  private HashMap<Long, BitSet> complexEdgesMap;
+  private final HashMap<Long, BitSet> complexEdgesMap;
 
   // node bitmap overlaps edge's leftNodeBits or rightNodeBits, but does not completely cover
-  private HashMap<Long, BitSet> overlapEdgesMap;
+  private final HashMap<Long, BitSet> overlapEdgesMap;
 
   protected HyperGraph(RelOptCluster cluster,
       RelTraitSet traitSet,
       List<RelNode> inputs,
       List<HyperEdge> edges,
       RelDataType rowType) {
     super(cluster, traitSet);
-    checkArgument(rowType != null);
     this.inputs = inputs;
     this.edges = edges;
     this.rowType = rowType;
@@ -87,7 +89,6 @@ protected HyperGraph(RelOptCluster cluster,
       HashMap<Long, BitSet> complexEdgesMap,
       HashMap<Long, BitSet> overlapEdgesMap) {
     super(cluster, traitSet);
-    checkArgument(rowType != null);
     this.inputs = inputs;
     this.edges = edges;
     this.rowType = rowType;
@@ -173,14 +174,14 @@ public List<HyperEdge> connectCsgCmp(long csg, long cmp) {
     checkArgument(simpleEdgesMap.containsKey(cmp));
     List<HyperEdge> connectedEdges = new ArrayList<>();
     BitSet connectedEdgesBitmap = new BitSet();
-    connectedEdgesBitmap.or(simpleEdgesMap.get(csg));
-    connectedEdgesBitmap.or(complexEdgesMap.get(csg));
-    connectedEdgesBitmap.or(overlapEdgesMap.get(csg));
+    connectedEdgesBitmap.or(simpleEdgesMap.getOrDefault(csg, new BitSet()));
+    connectedEdgesBitmap.or(complexEdgesMap.getOrDefault(csg, new BitSet()));
+    connectedEdgesBitmap.or(overlapEdgesMap.getOrDefault(csg, new BitSet()));
 
     BitSet cmpEdgesBitmap = new BitSet();
-    cmpEdgesBitmap.or(simpleEdgesMap.get(cmp));
-    cmpEdgesBitmap.or(complexEdgesMap.get(cmp));
-    cmpEdgesBitmap.or(overlapEdgesMap.get(cmp));
+    cmpEdgesBitmap.or(simpleEdgesMap.getOrDefault(cmp, new BitSet()));
+    cmpEdgesBitmap.or(complexEdgesMap.getOrDefault(cmp, new BitSet()));
+    cmpEdgesBitmap.or(overlapEdgesMap.getOrDefault(cmp, new BitSet()));
 
     connectedEdgesBitmap.and(cmpEdgesBitmap);
     connectedEdgesBitmap.stream()
@@ -224,16 +225,16 @@ public void updateEdgesForUnion(long subset1, long subset2) {
     }
 
     BitSet unionSimpleBitSet = new BitSet();
-    unionSimpleBitSet.or(simpleEdgesMap.get(subset1));
-    unionSimpleBitSet.or(simpleEdgesMap.get(subset2));
+    unionSimpleBitSet.or(simpleEdgesMap.getOrDefault(subset1, new BitSet()));
+    unionSimpleBitSet.or(simpleEdgesMap.getOrDefault(subset2, new BitSet()));
 
     BitSet unionComplexBitSet = new BitSet();
-    unionComplexBitSet.or(complexEdgesMap.get(subset1));
-    unionComplexBitSet.or(complexEdgesMap.get(subset2));
+    unionComplexBitSet.or(complexEdgesMap.getOrDefault(subset1, new BitSet()));
+    unionComplexBitSet.or(complexEdgesMap.getOrDefault(subset2, new BitSet()));
 
     BitSet unionOverlapBitSet = new BitSet();
-    unionOverlapBitSet.or(overlapEdgesMap.get(subset1));
-    unionOverlapBitSet.or(overlapEdgesMap.get(subset2));
+    unionOverlapBitSet.or(overlapEdgesMap.getOrDefault(subset1, new BitSet()));
+    unionOverlapBitSet.or(overlapEdgesMap.getOrDefault(subset2, new BitSet()));
 
     // the overlaps edge that belongs to subset1/subset2
     // may be complex edge for subset1 union subset2

diff --git a/core/src/main/java/org/apache/calcite/rel/rules/JoinToHyperGraphRule.java b/core/src/main/java/org/apache/calcite/rel/rules/JoinToHyperGraphRule.java
@@ -35,8 +35,6 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
 /** Rule that flattens a tree of {@link LogicalJoin}s
  * into a single {@link HyperGraph} with N inputs.
  *
@@ -120,8 +118,11 @@ protected JoinToHyperGraphRule(Config config) {
 
       RexVisitorImpl visitor = new RexVisitorImpl<Void>(true) {
         @Override public Void visitInputRef(RexInputRef inputRef) {
-          checkArgument(fieldIndexToNodeIndexMap.containsKey(inputRef.getIndex()));
-          int nodeIndex = fieldIndexToNodeIndexMap.get(inputRef.getIndex());
+          Integer nodeIndex = fieldIndexToNodeIndexMap.get(inputRef.getIndex());
+          if (nodeIndex == null) {
+            throw new IllegalArgumentException("RexInputRef refers a dummy field: "
+                + inputRef + ", rowType is: " + origJoin.getRowType());
+          }
           if (nodeIndex < leftNodeCount) {
             leftRefs.add(nodeIndex);
           } else {