[GitHub] [calcite] zabetak commented on a change in pull request #1097: [CALCITE-2902] Improve performance of AbstractRelNode#computeDigest

[GitBox <gi...@apache.org>]

zabetak commented on a change in pull request #1097: [CALCITE-2902] Improve performance of AbstractRelNode#computeDigest
URL: https://github.com/apache/calcite/pull/1097#discussion_r264065951
 
 

 ##########
 File path: core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
 ##########
 @@ -389,42 +384,68 @@ public RelOptTable getTable() {
    * @return Digest
    */
   protected String computeDigest() {
-    StringWriter sw = new StringWriter();
-    RelWriter pw =
-        new RelWriterImpl(
-            new PrintWriter(sw),
-            SqlExplainLevel.DIGEST_ATTRIBUTES, false) {
-          protected void explain_(
-              RelNode rel, List<Pair<String, Object>> values) {
-            pw.write(getRelTypeName());
-
-            for (RelTrait trait : traitSet) {
-              pw.write(".");
-              pw.write(trait.toString());
-            }
-
-            pw.write("(");
-            int j = 0;
-            for (Pair<String, Object> value : values) {
-              if (j++ > 0) {
-                pw.write(",");
-              }
-              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(")");
-          }
-        };
-    explain(pw);
-    return sw.toString();
+    RelDigestWriter rdw = new RelDigestWriter();
+    explain(rdw);
+    return rdw.digest;
+  }
+
+  /**
+   * A writer object used exclusively for computing the digest of a RelNode.
+   *
+   * <p>The writer is meant to be used only for computing a single digest and then thrown away.
+   * After calling {@link #done(RelNode)} the writer should be used only to obtain the computed
 
 Review comment:
   Thanks for having a look @asereda-gs ! I would rather avoid extra flags and checks to avoid any potential impact on performance. As you said the class is private and is used for a very particular purpose so I think it is safe to omit additional checks. Other than that how do you feel about this PR? I would like to merge it if you don't have any objections.  

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services