[
https://issues.apache.org/jira/browse/DERBY-4087?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12740760#action_12740760
]
Dag H. Wanvik commented on DERBY-4087:
--------------------------------------
I have found some further issues regarding toString, treePrint and
printSubNodes.
- Some class variables' value are printed with a leading "<label>: "
in front of the value, others do not have a label, the value is
printed alone. Usually, the label is the same as the class
variable's name. Shouldn't this always be the case for uniformity?
- Some class variable labels are printed even if the values is not,
in cases where the variables value is null, e.g. "preJoinFL: " in
SelectNode. I suggest not printing in such cases to save space.
- Some toString methods call super.toString *before* formatting the
class's own member variables, others do it the other way around. Is
there a rationale for this I wonder? Or is it just accidental? It
seems the established pattern is that super be called *after*, except
for DDL operations, which always call super *first*, so I suggest
adjusting toString to those two patterns throughout.
Non-DDL classes that call super.toString first:
CoalesceFunctionNode
CurrentDatetimeOperatorNode
SpecialFunctionNode
ExtractOperatorNode
- Some usages of toString terminate with a newline, most do not. I
think they should either end with a newline (DDL) or newline + call
to super.toString (DML). At the root (qua anchor),
QueryTreeNode.toString just prints "", so effectively for DML, the
last thing printed will be the newline also.
- I think all nodes should call super.toString. This could be because
the author "knew" they are right under QueryTreeNode, and so has
chosen to optimize the call away (QueryTreeNode.toString prints the
empty string), or it could be an omission. I think it would be good
to be consistent here, too.
Example of nodes that do not call super.toString at all:
GenerationClauseNode
OrderByColumn
TableName
TableName is possibly an allowable exception, I think, because its
toString will return a string even in insane builds, so I not sure
it can tolerate a terminating newline, so I intend to not touch
this.
- Some nodes call toString of underlying nodes in their own toString,
but such subnodes should, according to the contract defined in
QueryTreeNode, be handled by PrintSubNodes/TreePrint:
DistinctNode (formats childResult)
GroupByNode (formats childResult)
OrderByNode (formats childResult)
OrderByColumn (formats expression)
I suggest bringing such instances into line.
- Vectors are handled specially. Vectors with tree elements are
subclasses of QueryTreeNodeVector, which contains a generic toString
method which calls toString of the elements of the vector and
concatenates them. I suggest changing this to be printSubNodes and
use series of calls to treePrint instead to comply with the general
pattern. Subclasses which have no additional members need not
override toString either.
- Some subclasses of QueryTreeNodeVector needlessly(*) define their own
printSubNodes method. I propose to remove those:
ValueNodeList
SubqueryList
PredicateList
FromList
(*) this now inherit QueryTreeNodeVector's printSubNodes.
- One subclasses of QueryTreeNodeVector (TableElementList) rolls its
own toString to print the elements. I suggest to remove this an
dinstead rely on the general mechanism outlined above.
Some subclasses of QueryTreeNodeVector do *not* define their own
printSubNodes, e.g:
ResultColumnList
- Some subclasses of QueryTreeNodeVector define their own treePrint
method, e.g.
ResultColumnList
This seems to be a remnant from a time when ResultColumnList was not
a subclass of QueryTreeNode, c.f this comment in
/**
* This class needs a treePrint method, even though it is not a
* descendant of QueryTreeNode, because its members contain tree..
so I intend to remove this.
> Clean up debug printing of the abstract syntax trees after parsing, binding
> and optimization
> ---------------------------------------------------------------------------------------------
>
> Key: DERBY-4087
> URL: https://issues.apache.org/jira/browse/DERBY-4087
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous, SQL
> Reporter: Dag H. Wanvik
> Assignee: Dag H. Wanvik
> Priority: Trivial
>
> Currently, the printing is often inconsistent:
> - some subtrees not printed
> - wrong indentation due to missing newlines, or lacking level increments
> - redundant printing of subtrees (AST is really a DAG, would be nice to print
> only once and then refer back to show aliasing)
> - some items printed twice due to inconsistent usage of pattern
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
