From 72837a3ab44124f774dcf5bb86a6c77fb867e675 Mon Sep 17 00:00:00 2001 From: jakevin Date: Fri, 11 Aug 2023 14:53:47 +0800 Subject: [PATCH] [enhancement](Nereids): Plan equals() hashcode() don't need LogicalProprties (#22774) - deepEquals don't need to compare LogicalProperties - Plan equals() hashcode() don't need logicalProperty --- .../nereids/trees/plans/AbstractPlan.java | 29 ------------------- .../plans/logical/LogicalCTEProducer.java | 5 +--- .../LogicalDeferMaterializeResultSink.java | 5 +--- .../trees/plans/logical/LogicalGenerate.java | 5 +--- .../trees/plans/logical/LogicalJoin.java | 3 -- .../trees/plans/logical/LogicalLimit.java | 6 +--- .../plans/logical/LogicalResultSink.java | 5 +--- .../plans/physical/AbstractPhysicalJoin.java | 5 +--- .../plans/physical/PhysicalCTEAnchor.java | 10 +++---- .../plans/physical/PhysicalCTEProducer.java | 10 +++---- .../PhysicalDeferMaterializeResultSink.java | 9 ++---- .../plans/physical/PhysicalGenerate.java | 5 +--- .../plans/physical/PhysicalResultSink.java | 7 ++--- .../doris/nereids/util/MemoValidator.java | 8 ++--- 14 files changed, 24 insertions(+), 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 index cfe4b31aaf4841..839fd425e5ec1c 100644 --- 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 getOutput() { return getLogicalProperties().getOutput(); @@ -208,13 +188,4 @@ public Optional 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/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCTEProducer.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCTEProducer.java index 089f1ec9fa9aba..64f2831d0791fc 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCTEProducer.java +++ b/fe/fe-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/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeResultSink.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeResultSink.java index 48ea0720455114..5dfbd4eba9554c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeResultSink.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeResultSink.java @@ -122,9 +122,6 @@ 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); @@ -132,7 +129,7 @@ public boolean equals(Object o) { @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 index c35879f96937f6..8036afe2563e27 100644 --- 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,9 +130,6 @@ 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); @@ -140,6 +137,6 @@ public boolean equals(Object o) { @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 index 98a86a43dcb1f1..485111ff0e4c19 100644 --- 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 index f03f335f0182bd..ef6175808365ff 100644 --- 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 getExpressions() { return ImmutableList.of(); } - public LogicalLimit withLimitPhase(LimitPhase phase) { - return new LogicalLimit<>(limit, offset, phase, child()); - } - @Override public Plan withGroupExpression(Optional 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 index eb7b2556d367a4..cc2eb0cf377446 100644 --- 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/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/AbstractPhysicalJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/AbstractPhysicalJoin.java index f67123522c3daf..eb9ed7cfc975d1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/AbstractPhysicalJoin.java +++ b/fe/fe-core/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/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEAnchor.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEAnchor.java index 88908abe30ee81..d3659e7177192f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEAnchor.java +++ b/fe/fe-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/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEProducer.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEProducer.java index 8614aa26d8f5d5..53ff3e3025742d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEProducer.java +++ b/fe/fe-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/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeResultSink.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeResultSink.java index ee0713300655c4..955c4411570d49 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeResultSink.java +++ b/fe/fe-core/src/main/java/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 index d75402fb2167bb..5cd8bd238c295f 100644 --- 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,9 +106,6 @@ 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); @@ -116,7 +113,7 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(super.hashCode(), generators, generatorOutput); + return Objects.hash(generators, generatorOutput); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalResultSink.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalResultSink.java index 9126ccc3bac373..553b47d29b346b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalResultSink.java +++ b/fe/fe-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 index 5e8fe59db4e68b..1bd8f2aaf8f99e 100644 --- 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 visitedGroups = new IdentityHashMap<>(); public final IdentityHashMap 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;