|
Thanks for catching the regression and resolving it so quickly. Is
there anything you cann't do? :-) Yes, I goofed in creating a new copy of the BinaryRelationalOperator. Not sure why I put BINARY_EQUALS operator always, may be initial cut_and_paste from some other part of the code. I thought the reason for having TRUE constant node was because of need to have the predicate in CNF form... AndNode is always at the top, which assists (or simplifies) logic in the optimizer. I am still not done with changes in 10.1 branch. I will add a comment for the need to have constant node. Other change I would like to make is how the patch finds matching column reference. (findColumnReferenceInResult) Instead of matching by column name, I think, it should be benificial to match by columnPosition. This increases chances of finding a matching column. For example: Select * from (SELECT a, b from T1 union SELECT c, d from T2) tab(col1, col2) where col1=5 The predicate "col1=5" once gets pushed into ProjectRestrictNode, becomes "SQLCol<num>=5" and trying to find a match in SELECT statements would fail. If we match by columnNumber instead, it should be possible to transform "col1=5" into "SQLCol<num>=5" to finally "a=5" or "c=5". I am still investigating this... currently busy baby sitting. Sorry for the trouble... Satheesh Daniel John Debrunner (JIRA) wrote: [ http://issues.apache.org/jira/browse/DERBY-649?page=comments#action_12360997 ]Daniel John Debrunner commented on DERBY-649: --------------------------------------------- I think I've found the bug in the patch. I'd appreciate any optimizer experts looking at this. In PredicateList.pushExpressionsIntoSelect when the predicate is copied, any type of binary relational node can be copied, but the new relational node create is always an quality node, it is not based upon the type being pushed. I'm replacing this code, difference is first argument to getNode() : around line 1438 - changes would also be made to te variable name, to correctly represent its use. BinaryRelationalOperatorNode newEquals = (BinaryRelationalOperatorNode) getNodeFactory().getNode( C_NodeTypes.BINARY_EQUALS_OPERATOR_NODE, newCRNode, opNode.getRightOperand(), getContextManager()); with BinaryRelationalOperatorNode newEquals = (BinaryRelationalOperatorNode) getNodeFactory().getNode( opNode.getNodeType(), newCRNode, opNode.getRightOperand(), getContextManager()); I'd incorrectly assumed in the review that the (incorrect) new equality node was related to the boolean constant TRUE being created. So sort of '(pushed _expression_) = TRUE' was being pushed and required for some reason. I knew there was a good reason I'd asked for comments on this code section: |
- Re: [jira] Commented: (DERBY-649) Useful inde... Satheesh Bandaram
- Re: [jira] Commented: (DERBY-649) Useful... Daniel John Debrunner
- [jira] Commented: (DERBY-649) Useful ind... Daniel John Debrunner (JIRA)
- [jira] Commented: (DERBY-649) Useful ind... Satheesh Bandaram (JIRA)
