[calcite] branch master updated: [CALCITE-2471] Shorten AbstractRelNode#computeDigest() digest (Laurent Goujon)

[vl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

vladimirsitnikov 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 9667f85  [CALCITE-2471] Shorten AbstractRelNode#computeDigest() digest (Laurent Goujon)
9667f85 is described below

commit 9667f8537010d21556a592bd0dfbf20f7d0a4411
Author: Laurent Goujon <la...@apache.org>
AuthorDate: Thu Aug 16 08:54:16 2018 -0700

    [CALCITE-2471] Shorten AbstractRelNode#computeDigest() digest (Laurent Goujon)
    
    Digest/description created by AbstractRelNode#computeDigest includes
    the description of each of its inputs, causing the final string to
    be excessively long (e.g. the top level node of a tree containing
    the string representation of the whole tree).
    
    Change the function to not use the input's description but instead
    its original description which is <typeName>#<id>.
    
    fixes #795
---
 .../org/apache/calcite/rel/AbstractRelNode.java    | 20 ++++++----
 .../org/apache/calcite/test/HepPlannerTest.java    | 44 ++++++++++++++++++++++
 2 files changed, 57 insertions(+), 7 deletions(-)

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 6518067..59a9179 100644
--- a/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
+++ b/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
@@ -354,13 +354,10 @@ public abstract class AbstractRelNode implements RelNode {
 
   public String recomputeDigest() {
     String tempDigest = computeDigest();
-    assert tempDigest != null : "post: return != null";
-    String prefix = "rel#" + id + ":";
+    assert tempDigest != null : "computeDigest() should be non-null";
 
-    // Substring uses the same underlying array of chars, so saves a bit
-    // of memory.
-    this.desc = prefix + tempDigest;
-    this.digest = this.desc.substring(prefix.length());
+    this.desc = "rel#" + id + ":" + tempDigest;
+    this.digest = tempDigest;
     return this.digest;
   }
 
@@ -412,7 +409,16 @@ public abstract class AbstractRelNode implements RelNode {
               if (j++ > 0) {
                 pw.write(",");
               }
-              pw.write(value.left + "=" + value.right);
+              pw.write(value.left);
+              pw.write("=");
+              if (value.right instanceof RelNode) {
+                RelNode input = (RelNode) value.right;
+                pw.write(input.getRelTypeName());
+                pw.write("#");
+                pw.write(Integer.toString(input.getId()));
+              } else {
+                pw.write(String.valueOf(value.right));
+              }
             }
             pw.write(")");
           }
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 8eb78d5..2e354e0 100644
--- a/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java
+++ b/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java
@@ -23,6 +23,7 @@ import org.apache.calcite.plan.hep.HepPlanner;
 import org.apache.calcite.plan.hep.HepProgram;
 import org.apache.calcite.plan.hep.HepProgramBuilder;
 import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelRoot;
 import org.apache.calcite.rel.core.RelFactories;
 import org.apache.calcite.rel.logical.LogicalIntersect;
 import org.apache.calcite.rel.logical.LogicalUnion;
@@ -125,6 +126,49 @@ public class HepPlannerTest extends RelOptTestBase {
         "select name from sales.dept where deptno=12");
   }
 
+  /**
+   * Ensures {@link org.apache.calcite.rel.AbstractRelNode} digest does not include
+   * full digest tree.
+   */
+  @Test public void relDigestLength() {
+    HepProgramBuilder programBuilder = HepProgram.builder();
+    HepPlanner planner =
+        new HepPlanner(
+            programBuilder.build());
+    StringBuilder sb = new StringBuilder();
+    final int n = 10;
+    sb.append("select * from (");
+    sb.append("select name from sales.dept");
+    for (int i = 0; i < n; i++) {
+      sb.append(" union all select name from sales.dept");
+    }
+    sb.append(")");
+    RelRoot root = tester.convertSqlToRel(sb.toString());
+    planner.setRoot(root.rel);
+    RelNode best = planner.findBestExp();
+
+    // Good digest should look like rel#66:LogicalProject(input=rel#64:LogicalUnion)
+    // 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.getDigest()", best.getDigest(), "LogicalUnion");
+  }
+
+  private void assertIncludesExactlyOnce(String message, String digest, String substring) {
+    int pos = 0;
+    int cnt = 0;
+    while (pos >= 0) {
+      pos = digest.indexOf(substring, pos + 1);
+      if (pos > 0) {
+        cnt++;
+      }
+    }
+    assertEquals(
+        message + " should include <<" + substring + ">> exactly once, actual value is " + digest,
+        1, cnt);
+  }
+
   @Test public void testMatchLimitOneTopDown() throws Exception {
     // Verify that only the top union gets rewritten.