zabetak commented on a change in pull request #2143:
URL: https://github.com/apache/calcite/pull/2143#discussion_r485442570



##########
File path: core/src/main/java/org/apache/calcite/rel/RelNode.java
##########
@@ -246,6 +246,8 @@ default String getDigest() {
     return getRelDigest().toString();
   }
 
+  String getTreeDigest();

Review comment:
       This is what I meant that is not very neat; adding a new API in 
`RelNode` just for debugging purposes. 
   Nevertheless if we decide to go this way I think it would be better to use 
the following signature:
   `String explain();`
   
   Going one step further we could make it a default method with an 
implementation similar to the one below:
   ```
   default String explain() {
       StringWriter sw = new StringWriter();
       RelWriter planWriter =
           new RelWriterImpl(
               new PrintWriter(sw), SqlExplainLevel.EXPPLAN_ATTRIBUTES, false);
       explain(planWriter);
       return sw.toString();
   }
   ```
   Even more we could deprecate/remove `RelOptUtil#toString(RelNode)` variants 
in favor of this one.




----------------------------------------------------------------
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:
[email protected]


Reply via email to