You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by hy...@apache.org on 2019/11/05 04:23:04 UTC
[calcite] branch master updated: [CALCITE-3458] Remove desc in
AbstractRelNode
This is an automated email from the ASF dual-hosted git repository.
hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new e2c8e68 [CALCITE-3458] Remove desc in AbstractRelNode
e2c8e68 is described below
commit e2c8e68f4ac31498a9a41fb45ead2f64f04b1c36
Author: Haisheng Yuan <h....@alibaba-inc.com>
AuthorDate: Tue Oct 29 23:09:26 2019 -0700
[CALCITE-3458] Remove desc in AbstractRelNode
If the query is super large, e.g. contains tens of thousands of nodes or
expressions, the RelNode digest and desc become very large. The content of desc
and digest are almost the same, except that desc consists of id plus digest. So
remove desc, just use id + digest to produce description.
Also deprecated method rel.getDescription(), use rel.toString() instead.
Close #1546
---
.../java/org/apache/calcite/plan/RelOptNode.java | 1 +
.../java/org/apache/calcite/plan/RelOptUtil.java | 7 ++++++
.../org/apache/calcite/plan/volcano/RelSubset.java | 3 +--
.../calcite/plan/volcano/VolcanoPlanner.java | 22 +++++++++---------
.../calcite/plan/volcano/VolcanoRuleMatch.java | 3 ++-
.../org/apache/calcite/rel/AbstractRelNode.java | 26 +++++++++-------------
.../apache/calcite/sql2rel/RelDecorrelator.java | 4 ++--
.../org/apache/calcite/test/HepPlannerTest.java | 2 +-
8 files changed, 35 insertions(+), 33 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptNode.java b/core/src/main/java/org/apache/calcite/plan/RelOptNode.java
index 9df7fba..9db9cb1 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptNode.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptNode.java
@@ -73,6 +73,7 @@ public interface RelOptNode {
* @return String which describes the relational expression and, unlike
* {@link #getDigest()}, also includes the identity
*/
+ @Deprecated // to be removed before 2.0
String getDescription();
/**
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index bccb0f5..44fa568 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -1895,6 +1895,13 @@ public abstract class RelOptUtil {
}
+ public static StringBuilder appendRelDescription(
+ StringBuilder sb, RelNode rel) {
+ sb.append("rel#").append(rel.getId())
+ .append(':').append(rel.getDigest());
+ return sb;
+ }
+
/**
* Dumps a plan as a string.
*
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java b/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
index bf95211..426a558 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
@@ -178,8 +178,7 @@ public class RelSubset extends AbstractRelNode {
@Override public void explain(RelWriter pw) {
// Not a typical implementation of "explain". We don't gather terms &
// values to be printed later. We actually do the work.
- String s = getDescription();
- pw.item("subset", s);
+ pw.item("subset", toString());
final AbstractRelNode input =
(AbstractRelNode) Util.first(getBest(), getOriginal());
if (input == null) {
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
index 2068cd1..d28b5ce 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
@@ -885,7 +885,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
for (RelSubset subset : set.subsets) {
if (subset.set != set) {
return litmus.fail("subset [{}] is in wrong set [{}]",
- subset.getDescription(), set);
+ subset, set);
}
if (subset.best != null) {
@@ -893,14 +893,14 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
// Make sure best RelNode is valid
if (!subset.set.rels.contains(subset.best)) {
return litmus.fail("RelSubset [{}] does not contain its best RelNode [{}]",
- subset.getDescription(), subset.best.getDescription());
+ subset, subset.best);
}
// Make sure bestCost is up-to-date
try {
RelOptCost bestCost = getCost(subset.best, metaQuery);
if (!subset.bestCost.equals(bestCost)) {
- return litmus.fail("RelSubset [" + subset.getDescription()
+ return litmus.fail("RelSubset [" + subset
+ "] has wrong best cost "
+ subset.bestCost + ". Correct cost is " + bestCost);
}
@@ -915,7 +915,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
if (relCost.isLt(subset.bestCost)) {
return litmus.fail("rel [{}] has lower cost {} than "
+ "best cost {} of subset [{}]",
- rel.getDescription(), relCost, subset.bestCost, subset.getDescription());
+ rel, relCost, subset.bestCost, subset);
}
} catch (CyclicMetadataException e) {
// ignore
@@ -1172,7 +1172,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
* @see #normalizePlan(String)
*/
public void dump(PrintWriter pw) {
- pw.println("Root: " + root.getDescription());
+ pw.println("Root: " + root);
pw.println("Original rel:");
if (originalRoot != null) {
@@ -1213,7 +1213,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
for (RelSubset subset : set.subsets) {
++j;
pw.println(
- "\t" + subset.getDescription() + ", best="
+ "\t" + subset + ", best="
+ ((subset.best == null) ? "null"
: ("rel#" + subset.best.getId())) + ", importance="
+ ruleQueue.getImportance(subset));
@@ -1224,7 +1224,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
}
for (RelNode rel : subset.getRels()) {
// "\t\trel#34:JavaProject(rel#32:JavaFilter(...), ...)"
- pw.print("\t\t" + rel.getDescription());
+ pw.print("\t\t" + rel);
for (RelNode input : rel.getInputs()) {
RelSubset inputSubset =
getSubset(
@@ -1282,7 +1282,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
// It can happen due to RelTraitset#simplify
// If the traits are different, we want to keep them on a graph
String traits = "." + getSubset(rel).getTraitSet().toString();
- String title = rel.getDescription().replace(traits, "");
+ String title = rel.toString().replace(traits, "");
if (title.endsWith(")")) {
int openParen = title.indexOf('(');
if (openParen != -1) {
@@ -1317,7 +1317,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
pw.print("\t\tsubset");
pw.print(subset.getId());
pw.print(" [label=");
- Util.printJavaString(pw, subset.getDescription(), false);
+ Util.printJavaString(pw, subset.toString(), false);
boolean empty = !nonEmptySubsets.contains(subset);
if (empty) {
// We don't want to iterate over rels when we know the set is not empty
@@ -1729,7 +1729,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
RelSet equivSet = getSet(equivExp);
if (equivSet != null) {
LOGGER.trace(
- "Register: rel#{} is equivalent to {}", rel.getId(), equivExp.getDescription());
+ "Register: rel#{} is equivalent to {}", rel.getId(), equivExp);
return registerSubset(set, getSubset(equivExp));
}
}
@@ -1799,7 +1799,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
final RelNode xx = mapDigestToRel.put(key, rel);
assert xx == null || xx == rel : rel.getDigest();
- LOGGER.trace("Register {} in {}", rel.getDescription(), subset.getDescription());
+ LOGGER.trace("Register {} in {}", rel, subset);
// This relational expression may have been registered while we
// recursively registered its children. If this is the case, we're done.
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleMatch.java b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleMatch.java
index d08451a..aa2272a 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleMatch.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleMatch.java
@@ -17,6 +17,7 @@
package org.apache.calcite.plan.volcano;
import org.apache.calcite.plan.RelOptRuleOperand;
+import org.apache.calcite.plan.RelOptUtil;
import org.apache.calcite.plan.RelTrait;
import org.apache.calcite.plan.RelTraitSet;
import org.apache.calcite.rel.RelNode;
@@ -144,7 +145,7 @@ class VolcanoRuleMatch extends VolcanoRuleCall {
if (i > 0) {
buf.append(", ");
}
- buf.append(rels[i].toString());
+ RelOptUtil.appendRelDescription(buf, rels[i]);
}
buf.append("]");
return buf.toString();
diff --git a/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java b/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
index 20f264f..34c54eb 100644
--- a/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
+++ b/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
@@ -65,11 +65,6 @@ public abstract class AbstractRelNode implements RelNode {
//~ Instance fields --------------------------------------------------------
/**
- * Description, consists of id plus digest.
- */
- private String desc;
-
- /**
* Cached type of this relational expression.
*/
protected RelDataType rowType;
@@ -80,8 +75,6 @@ public abstract class AbstractRelNode implements RelNode {
* is equivalent if and only if it has the same value. Computed by
* {@link #computeDigest}, assigned by {@link #onRegister}, returned by
* {@link #getDigest()}.
- *
- * @see #desc
*/
protected String digest;
@@ -109,7 +102,6 @@ public abstract class AbstractRelNode implements RelNode {
this.traitSet = traitSet;
this.id = NEXT_ID.getAndIncrement();
this.digest = getRelTypeName() + "#" + id;
- this.desc = digest;
LOGGER.trace("new {}", digest);
}
@@ -348,12 +340,9 @@ public abstract class AbstractRelNode implements RelNode {
}
public String recomputeDigest() {
- String tempDigest = computeDigest();
- assert tempDigest != null : "computeDigest() should be non-null";
-
- this.desc = "rel#" + id + ":" + tempDigest;
- this.digest = tempDigest;
- return this.digest;
+ digest = computeDigest();
+ assert digest != null : "computeDigest() should be non-null";
+ return digest;
}
public void replaceInput(
@@ -362,12 +351,17 @@ public abstract class AbstractRelNode implements RelNode {
throw new UnsupportedOperationException("replaceInput called on " + this);
}
+ /* Description, consists of id plus digest */
public String toString() {
- return desc;
+ StringBuilder sb = new StringBuilder();
+ sb = RelOptUtil.appendRelDescription(sb, this);
+ return sb.toString();
}
+ /* Description, consists of id plus digest */
+ @Deprecated // to be removed before 2.0
public final String getDescription() {
- return desc;
+ return this.toString();
}
public final String getDigest() {
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
index 5219c29..fbfc8bf 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
@@ -1931,7 +1931,7 @@ public class RelDecorrelator implements ReflectiveVisitor {
if (!RelMdUtil.areColumnsDefinitelyUniqueWhenNullsFiltered(mq, right,
rightJoinKeys)) {
SQL2REL_LOGGER.debug("{} are not unique keys for {}",
- rightJoinKeys.toString(), right.toString());
+ rightJoinKeys, right);
return;
}
@@ -2147,7 +2147,7 @@ public class RelDecorrelator implements ReflectiveVisitor {
if (!RelMdUtil.areColumnsDefinitelyUniqueWhenNullsFiltered(mq, left,
correlatedInputRefJoinKeys)) {
SQL2REL_LOGGER.debug("{} are not unique keys for {}",
- correlatedJoinKeys.toString(), left.toString());
+ correlatedJoinKeys, left);
return;
}
diff --git a/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java b/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java
index a88a136..7802b43 100644
--- a/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java
+++ b/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java
@@ -150,7 +150,7 @@ public class HepPlannerTest extends RelOptTestBase {
// Bad digest includes full tree like rel#66:LogicalProject(input=rel#64:LogicalUnion(...))
// So the assertion is to ensure digest includes LogicalUnion exactly once
- assertIncludesExactlyOnce("best.getDescription()", best.getDescription(), "LogicalUnion");
+ assertIncludesExactlyOnce("best.getDescription()", best.toString(), "LogicalUnion");
assertIncludesExactlyOnce("best.getDigest()", best.getDigest(), "LogicalUnion");
}