[enhancement](Nereids): Plan equals() hashcode() don't need LogicalPr…

…oprties (#22774) - deepEquals don't need to compare LogicalProperties - Plan equals() hashcode() don't need logicalProperty
apache · Aug 11, 2023 · 72837a3 · 72837a3
1 parent 209f36f
commit 72837a3
Show file tree

Hide file tree

Showing 14 changed files with 24 additions and 88 deletions.
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java
@@ -22,7 +22,6 @@
 import org.apache.doris.nereids.properties.LogicalProperties;
 import org.apache.doris.nereids.properties.UnboundLogicalProperties;
 import org.apache.doris.nereids.trees.AbstractTreeNode;
-import org.apache.doris.nereids.trees.TreeNode;
 import org.apache.doris.nereids.trees.expressions.ExprId;
 import org.apache.doris.nereids.trees.expressions.Slot;
 import org.apache.doris.nereids.util.MutableState;
@@ -140,25 +139,6 @@ public JSONObject toJson() {
         return json;
     }
 
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) {
-            return true;
-        }
-        if (o == null || getClass() != o.getClass()) {
-            return false;
-        }
-        AbstractPlan that = (AbstractPlan) o;
-        // stats should don't need.
-        return Objects.equals(getLogicalProperties(), that.getLogicalProperties());
-    }
-
-    @Override
-    public int hashCode() {
-        // stats should don't need.
-        return Objects.hash(getLogicalProperties());
-    }
-
     @Override
     public List<Slot> getOutput() {
         return getLogicalProperties().getOutput();
@@ -208,13 +188,4 @@ public Optional<Object> getMutableState(String key) {
     public void setMutableState(String key, Object state) {
         this.mutableState = this.mutableState.set(key, state);
     }
-
-    @Override
-    public boolean deepEquals(TreeNode o) {
-        AbstractPlan that = (AbstractPlan) o;
-        if (Objects.equals(getLogicalProperties(), that.getLogicalProperties())) {
-            return super.deepEquals(o);
-        }
-        return false;
-    }
 }
diff --git a/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCTEProducer.java b/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCTEProducer.java
@@ -102,15 +102,12 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        if (!super.equals(o)) {
-            return false;
-        }
         LogicalCTEProducer<?> that = (LogicalCTEProducer<?>) o;
         return Objects.equals(cteId, that.cteId);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), cteId);
+        return Objects.hash(cteId);
     }
 }
diff --git a/.../java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeResultSink.java b/.../java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeResultSink.java
@@ -122,17 +122,14 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        if (!super.equals(o)) {
-            return false;
-        }
         LogicalDeferMaterializeResultSink<?> that = (LogicalDeferMaterializeResultSink<?>) o;
         return selectedIndexId == that.selectedIndexId && Objects.equals(logicalResultSink,
                 that.logicalResultSink) && Objects.equals(olapTable, that.olapTable);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), logicalResultSink, olapTable, selectedIndexId);
+        return Objects.hash(logicalResultSink, olapTable, selectedIndexId);
     }
 
     @Override

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalGenerate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalGenerate.java
@@ -130,16 +130,13 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        if (!super.equals(o)) {
-            return false;
-        }
         LogicalGenerate<?> that = (LogicalGenerate<?>) o;
         return generators.equals(that.generators)
                 && generatorOutput.equals(that.generatorOutput);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), generators, generatorOutput);
+        return Objects.hash(generators, generatorOutput);
     }
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalJoin.java
@@ -234,9 +234,6 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        if (!super.equals(o)) {
-            return false;
-        }
         LogicalJoin<?, ?> that = (LogicalJoin<?, ?>) o;
         return joinType == that.joinType
                 && hint == that.hint

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalLimit.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalLimit.java
@@ -104,7 +104,7 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        LogicalLimit that = (LogicalLimit) o;
+        LogicalLimit<?> that = (LogicalLimit<?>) o;
         return limit == that.limit && offset == that.offset && phase == that.phase;
     }
 
@@ -117,10 +117,6 @@ public List<? extends Expression> getExpressions() {
         return ImmutableList.of();
     }
 
-    public LogicalLimit<Plan> withLimitPhase(LimitPhase phase) {
-        return new LogicalLimit<>(limit, offset, phase, child());
-    }
-
     @Override
     public Plan withGroupExpression(Optional<GroupExpression> groupExpression) {
         return new LogicalLimit<>(limit, offset, phase, groupExpression, Optional.of(getLogicalProperties()), child());

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalResultSink.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalResultSink.java
@@ -108,15 +108,12 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        if (!super.equals(o)) {
-            return false;
-        }
         LogicalResultSink<?> that = (LogicalResultSink<?>) o;
         return Objects.equals(outputExprs, that.outputExprs);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), outputExprs);
+        return Objects.hash(outputExprs);
     }
 }
diff --git a/...ore/src/main/java/org/apache/doris/nereids/trees/plans/physical/AbstractPhysicalJoin.java b/...ore/src/main/java/org/apache/doris/nereids/trees/plans/physical/AbstractPhysicalJoin.java
@@ -146,9 +146,6 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        if (!super.equals(o)) {
-            return false;
-        }
         AbstractPhysicalJoin<?, ?> that = (AbstractPhysicalJoin<?, ?>) o;
         return joinType == that.joinType
                 && hashJoinConjuncts.equals(that.hashJoinConjuncts)
@@ -159,7 +156,7 @@ public boolean equals(Object o) {
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), joinType, hashJoinConjuncts, otherJoinConjuncts, markJoinSlotReference);
+        return Objects.hash(joinType, hashJoinConjuncts, otherJoinConjuncts, markJoinSlotReference);
     }
 
     /**

diff --git a/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEAnchor.java b/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEAnchor.java
@@ -77,18 +77,16 @@ public boolean equals(Object o) {
         if (this == o) {
             return true;
         }
-
-        if (!super.equals(o)) {
+        if (o == null || getClass() != o.getClass()) {
             return false;
         }
-
-        PhysicalCTEAnchor that = (PhysicalCTEAnchor) o;
-        return Objects.equals(cteId, that.cteId);
+        PhysicalCTEAnchor<?, ?> that = (PhysicalCTEAnchor<?, ?>) o;
+        return cteId.equals(that.cteId);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), cteId);
+        return Objects.hash(cteId);
     }
 
     @Override

diff --git a/...core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEProducer.java b/...core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEProducer.java
@@ -74,18 +74,16 @@ public boolean equals(Object o) {
         if (this == o) {
             return true;
         }
-
-        if (!super.equals(o)) {
+        if (o == null || getClass() != o.getClass()) {
             return false;
         }
-
-        PhysicalCTEProducer that = (PhysicalCTEProducer) o;
-        return Objects.equals(cteId, that.cteId);
+        PhysicalCTEProducer<?> that = (PhysicalCTEProducer<?>) o;
+        return cteId.equals(that.cteId);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), cteId);
+        return Objects.hash(cteId);
     }
 
     @Override

diff --git a/...ava/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeResultSink.java b/...ava/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeResultSink.java
@@ -143,17 +143,14 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        if (!super.equals(o)) {
-            return false;
-        }
         PhysicalDeferMaterializeResultSink<?> that = (PhysicalDeferMaterializeResultSink<?>) o;
-        return selectedIndexId == that.selectedIndexId && Objects.equals(physicalResultSink,
-                that.physicalResultSink) && Objects.equals(olapTable, that.olapTable);
+        return selectedIndexId == that.selectedIndexId && physicalResultSink.equals(that.physicalResultSink)
+                && olapTable.equals(that.olapTable);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), physicalResultSink, olapTable, selectedIndexId);
+        return Objects.hash(physicalResultSink, olapTable, selectedIndexId);
     }
 
     @Override

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java
@@ -106,17 +106,14 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        if (!super.equals(o)) {
-            return false;
-        }
         PhysicalGenerate<?> that = (PhysicalGenerate<?>) o;
         return generators.equals(that.generators)
                 && generatorOutput.equals(that.generatorOutput);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), generators, generatorOutput);
+        return Objects.hash(generators, generatorOutput);
     }
 
     @Override

diff --git a/...-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalResultSink.java b/...-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalResultSink.java
@@ -109,16 +109,13 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        if (!super.equals(o)) {
-            return false;
-        }
         PhysicalResultSink<?> that = (PhysicalResultSink<?>) o;
-        return Objects.equals(outputExprs, that.outputExprs);
+        return outputExprs.equals(that.outputExprs);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), outputExprs);
+        return Objects.hash(outputExprs);
     }
 
     @Override

diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/util/MemoValidator.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/util/MemoValidator.java
@@ -17,6 +17,7 @@
 
 package org.apache.doris.nereids.util;
 
+import org.apache.doris.nereids.analyzer.UnboundResultSink;
 import org.apache.doris.nereids.memo.Group;
 import org.apache.doris.nereids.memo.GroupExpression;
 import org.apache.doris.nereids.memo.GroupId;
@@ -40,10 +41,6 @@ public class MemoValidator {
     public final IdentityHashMap<Group, Void> visitedGroups = new IdentityHashMap<>();
     public final IdentityHashMap<GroupExpression, Void> visitedExpressions = new IdentityHashMap<>();
 
-    public static MemoValidator validateInitState(Memo memo) {
-        return validateInitState(memo, null);
-    }
-
     public static MemoValidator validateInitState(Memo memo, Plan initPlan) {
         Assertions.assertEquals(memo.getGroups().size(), memo.getGroupExpressions().size());
 
@@ -55,6 +52,9 @@ public static MemoValidator validateInitState(Memo memo, Plan initPlan) {
 
         MemoValidator validator = validate(memo);
         if (initPlan != null) {
+            if (initPlan instanceof UnboundResultSink) {
+                return validator;
+            }
             Assertions.assertEquals(initPlan, memo.getRoot().getLogicalExpression().getPlan());
         }
         return validator;