[ 
https://issues.apache.org/jira/browse/CALCITE-4224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17193112#comment-17193112
 ] 

Julian Hyde commented on CALCITE-4224:
--------------------------------------

I think it should be a {{default final}} method on {{interface RelNode}}, not 
{{AbstractRelNode}}.

The implementation should continue to live in {{RelOptUtil}} or wherever. We 
don't want a bunch of extra code and imports in {{RelNode}}.

bq. On the other hand, having a public overridable method to go over the whole 
tree seems too broad (does the method needs to access internal state? is it 
okay for subclasses to override?)

I agree. It should be {{final}}, non-{{static}}. It has access to {{this}} but 
no other internal state. Sub-classes cannot override.

I don't like the name 'digest'; digests are for computers, but this output is 
for humans. I think I'd call it {{RelNode.tree()}}.

I like to see javadoc in the PR. Javadoc is an key part of an API.

> Add an method for RelNode to output its tree digest like RelOptUtil.toString
> ----------------------------------------------------------------------------
>
>                 Key: CALCITE-4224
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4224
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>            Reporter: Jiatao Tao
>            Assignee: Jiatao Tao
>            Priority: Minor
>              Labels: pull-request-available
>          Time Spent: 40m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to