|
Hi Yip, thank you for the patch! I will have a look at your changes this weekend.
The patch looks quite good to me. I have two small comments, and a request:
1) The original reporter already added a test case for the bug to triggerGeneral.sql. I think the new test cases are quite good and should be retained, but I think we should also enable the existing test case. It might also be reasonable to put the test cases together in the file, either by moving the existing test case to the end where the new cases are, or by adding the new cases in the middle, next to the existing test case. 2) There appears to be an unnecessary whitespace-only diff change in CreateTriggerNode. The request: I think that incorrect handling of the intermediate nodes in the UnionNode tree that is constructed for a INSERT statement with a multi-row VALUES clause. Although the two proposed fixes are completely independent, I was hoping that you could have a look at and perhaps see what you think of my proposed change. I'm a bit concerned that my proposed change turned out to be more complicated than was necessary, and perhaps you will see a simpler way to fix the problem if you have a look at that issue. Hi Bryan. Thanks for reviewing this patch. Yes, I'll be happy to review the proposed changes for
I'll also address the two comments you made and resubmit a follow-up patch when I return. =) Committed revision 2 of the patch to subversion as revision 482358.
I intend to merge this change to the 10.2 branch as well. Unfortunately, the merge to 10.2 is not trivial, I think because there have been intervening
changes to triggerGeneral.sql and its out file. Yip, can you have a look at the merge and see if you can construct a clean patch for 10.2? If you can do so and can attach a 10.2 diff to this issue, I can have a look at it. Thanks for providing the 10.2 patch. My build and test runs were clean, and I
committed the change to the 10.2 branch as revision 482484. This issue has been resolved for over a year with no further movement. Closing.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Major
CREATE TRIGGER with an INSERT action statement with multiple rows and a referenced column throws a StringIndexOutOfBoundsException
create trigger tgood after insert on x
referencing new as n
for each row mode db2sql insert into x values (n.x), (999), (333);
because the bindReferencesClause() method in CreateTriggerNode did not filter those column references that are not relevent to the conversion processing for OLD/NEW transition variables. The above trigger action will have the following parse tree: (roughly)
InsertNode
|
UnionNode
/ \
UnionNode RowRSNode
/ \ |
RowRSNode RowRSNode NumericConstantNode
| | |
ColRef NumericConstantNode 333
| |
n.i 999
Note that the CollectNodesVisitor collects *ALL* the ColumnReferences in the trigger action. In the above case, it returned 3 column references nodes. One in the top level Union node, one in top level Union Node's
left child which it is also an UnionNode and the last one is found in the RowResultSetNode.
The UnionNodes will have a copy of the CRs from its left child and those CRs will not have its beginOffset set which indicates they are not relevant for the conversion processing here, so the corrective action here is to skip these entries.