[
https://issues.apache.org/jira/browse/DERBY-4421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12770477#action_12770477
]
Knut Anders Hatlen commented on DERBY-4421:
-------------------------------------------
I didn't check all the nodes (QueryTreeNode has 159 sub-classes), but here are
some examples on classes that should have an empty implementation of
acceptChildren() because they have no visitable children:
UserTypeConstantNode
SQLBooleanConstantNode
UntypedNullConstantNode
BitConstantNode
XMLConstantNode
BooleanConstantNode
VarbitConstantNode
NumericConstantNode
SpecialFunctionNode
TableName
TableElementNode
WindowReferenceNode
CurrentDatetimeOperatorNode
CurrentRowLocationNode
NOPStatementNode
SetTransactionIsolationNode
The following classes should also have an empty acceptChildren() method if they
currently implement accept() correctly (they do have visitable children,
though, so it might be that they actually should have had a non-empty
acceptChildren() method):
GenerationClassNode
DefaultNode
PrivilegeNode
TablePrivilegeNode
WindowDefinitionNode
BaseColumnNode
VirtualColumnNode
ParameterNode
ColumnReference
ExecSPSNode
I see the point that making acceptChildren() abstract could help us detect some
cases of missing overrides. Apart from slightly reducing the amount of code
needed, one advantage of having an empty method in QTN instead of an abstract
method is that all the overrides will have the same structure (call
super.acceptChildren() and then accept() on all added children). With the
abstract method, the structure will be different depending on where in the
class tree the node is located (super.acceptChildren() cannot be called if none
of the super-classes implement it, but it has to be called if one of them
does). This means that each time you implement an acceptChildren() method,
you'll have to check all sub-classes of the node in which you implement it, and
add a call to super.acceptChildren() in each of the sub-classes'
implementations. This could be easy to forget.
The two approaches protect against different classes of errors, so I can be
convinced either way. For now, I'll just commit the patch as it is, since most
of it will stay the same regardless of which approach we choose.
> Allow Visitors to process the nodes bottom-up
> ---------------------------------------------
>
> Key: DERBY-4421
> URL: https://issues.apache.org/jira/browse/DERBY-4421
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.6.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: d4421-1a.diff, d4421-1a.stat
>
>
> Currently, QueryTreeNode.accept() walks the tree top-down, always calling
> visit() on the parent before it calls visit() on the children. Although this
> is fine in most cases, there are use cases where visiting the nodes
> bottom-up would be better. One example is mentioned in DERBY-4416. The
> visitor posted there looks for binary comparison operators and checks
> whether both operands are constants. If they are, the operator is replaced
> with a boolean constant.
> Take this expression as an example: (1<2)=(2>1)
> The query tree looks like this:
> =
> / \
> / \
> < >
> / \ / \
> / \ / \
> 1 2 2 1
> If we walk the tree top-down with the said visitor, the = node doesn't have
> constant operands when it's visited. The < and > operators do have constant
> operands, and they're both replaced with constant TRUE. This means the
> expression (1<2)=(2>1) is rewritten to TRUE=TRUE, and that's how far the
> transformation goes.
> If the tree had been processed bottom-up, we would start with the < and >
> operators, and again replace them with TRUE. The query tree would therefore
> have been transformed to this intermediate form when the = operator was
> visited:
> =
> / \
> / \
> TRUE TRUE
> This is the same as the end result when visiting top-down, but now the =
> operator hasn't been visited yet. Since both the operands of the = operator
> are constants, the visitor will perform yet another transformation so the
> tree is simplified further and ends up as:
> TRUE
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
