Hi Bryan, The changes you suggested fixes the Derby5578 test. I've changed the return type of examineTriggerNode to int[] and I've run quite a few test suites without encountering any bugs.
The problem that I was facing while running the tests suites was because of the "junit-html" in the command for executing tests suites, it was causing the tests to fail, so I couldn't find a solution to fix this and I ran the tests without it. Is there anything else that needs to be done before this patch is accepted ? On Wed, Jun 24, 2015 at 10:38 AM, Bryan Pendleton < [email protected]> wrote: > Hi Abhinav, > > I played around with TriggerTest.testDerby5578InvalidateAllStatementsProc() > for a while. > > It seems like this test forces us through a slightly different code path > with respect to how we call DataDictionaryImpl.getTriggerActionString(). > > The trigger we are working with in that test case is: > > s.executeUpdate("create trigger atdc_16_trigger_1 "+ > "after update of b1 on atdc_16_tab1 " + > "REFERENCING NEW AS newt "+ > "for each row "+ > "update atdc_16_tab2 set c2 = newt.c1"); > > and the table we are working with is: > > s.executeUpdate("create table atdc_16_tab1 (a1 integer, b1 > integer, c1 integer)"); > > As we can see, this trigger references "b1" ("after update of b1") and > "c1" ("c2 = newt.c1"), but does not reference column "a1" at all. > > We seem to call getTriggerActionString multiple times when running the > test. > > The first time we call it, we have a column set that specifies that we > will be > using columns 2 and 3, which is correct because our trigger references > columns b1 and c1, and we end up building the action string: > > update atdc_16_tab2 set c2 = CAST > (org.apache.derby.iapi.db.Factory::getTriggerExecutionContext().getNewRow().getObject(2) > A > S INTEGER) > > which correctly references "c1" as column 2 in the new row. > > But the SECOND time we call getTriggerActionString, the 'cols' value is > null, > so we go into the "else" clause in getTriggerActionString() and that code > thinks we will be retrieving the entire table as the new row, and so we > end up building the action string: > > update "APP"."ATDC_16_TAB2" set c2 = CAST > (org.apache.derby.iapi.db.Factory::getTriggerExecutionContext().getNewRow().getObj > ect(3) AS INTEGER) > > which is incorrect as now we are referencing column "c1" as the third > column, but we are only retrieving columns b1 and c1 from the table, > not column a1, so c1 is really the second column. > > I think that the problem is that when we call getTriggerActionString > at runtime, with cols = null, we need to instead use our > examineTriggerNodeAndCols() method to figure out the correct set of > columns, > instead of assuming that all the columns will be retrieved. > > I think the code that does this is in TriggerDescriptor.getSPS, at around > line 407 of TriggerDescriptor.java. > > Perhaps you could experiment with trying to have that code call > examineTriggerNodeAndCols before it calls getTriggerActionString, > and see if that gives us the right column set and affects the > behavior of TriggerTest.testDerby5578InvalidateAllStatementsProc() > > thanks, > > bryan > >
