db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Knut Anders Hatlen (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-4421) Allow Visitors to process the nodes bottom-up
Date Tue, 27 Oct 2009 12:10:59 GMT

    [ 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.


Mime
View raw message