Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 10.10.1.1
    • Fix Version/s: 10.12.1.2, 10.13.1.0
    • Component/s: SQL
    • Labels:
      None

      Description

      Saw this strange exception when doing an insert to a table with a trigger

      Tue Sep 02 13:39:09 BST 2014 Thread[SQLExecution,1,system] (XID = 62693), (SESSIONID = 1), (DATABASE = C:/Users/timbo/Documents/IJCProjects/mini-regs/Vanilla Oracle/.config/derby-minireg-01-sep/db), (DRDAID = null), Failed Statement is: UPDATE samples SET sample_code = 'S123456' WHERE sample_id = CAST (org.apache.derby.iapi.db.Factory::getTriggerExecutionContext().getNewRow().getObject(1) AS INTEGER)
      java.lang.NullPointerException
          at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.getTriggerActionString(Unknown Source)
          at org.apache.derby.iapi.sql.dictionary.TriggerDescriptor.getActionSPS(Unknown Source)
          at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.getAction(Unknown Source)
          at org.apache.derby.impl.sql.execute.RowTriggerExecutor.fireTrigger(Unknown Source)
          at org.apache.derby.impl.sql.execute.TriggerEventActivator.notifyEvent(Unknown Source)
          at org.apache.derby.impl.sql.execute.UpdateResultSet.fireAfterTriggers(Unknown Source)
          at org.apache.derby.impl.sql.execute.UpdateResultSet.open(Unknown Source)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(Unknown Source)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeSubStatement(Unknown Source)
          at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.executeSPS(Unknown Source)
          at org.apache.derby.impl.sql.execute.RowTriggerExecutor.fireTrigger(Unknown Source)
          at org.apache.derby.impl.sql.execute.TriggerEventActivator.notifyEvent(Unknown Source)
          at org.apache.derby.impl.sql.execute.InsertResultSet.normalInsertCore(Unknown Source)
          at org.apache.derby.impl.sql.execute.InsertResultSet.open(Unknown Source)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(Unknown Source)
          at org.apache.derby.impl.sql.GenericPreparedStatement.execute(Unknown Source)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(Unknown Source)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(Unknown Source)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(Unknown Source)
      

      The trigger definition is this:

      CREATE TRIGGER samples_code_trg
      AFTER INSERT ON samples
      REFERENCING NEW AS newrow FOR EACH ROW MODE DB2SQL
      UPDATE samples SET sample_code = 'S123456'
      WHERE samples.sample_id = newrow.sample_id;
      

      As mentioned here: http://mail-archives.apache.org/mod_mbox/db-derby-user/201408.mbox/%3Cltq5hl$kps$1@ger.gmane.org%3E
      it could be that its caused by another AFTER UPDATE trigger that's on the table.

      Unfortunately I rebuilt all the tables and triggers and not the problem doesn't happen, so I can't provide a test case.

      1. derbytrig.zip
        2.52 MB
        Bernd Ruehlicke
      2. fixesRepro.diff
        8 kB
        Bryan Pendleton
      3. getTableDescriptor.diff
        8 kB
        Bryan Pendleton
      4. TriggerTest.diff
        7 kB
        Bryan Pendleton

        Issue Links

          Activity

          Hide
          dagw Dag H. Wanvik added a comment - - edited

          Working from the DDL shown, I have not been able to reproduce this on trunk yet.

          Show
          dagw Dag H. Wanvik added a comment - - edited Working from the DDL shown, I have not been able to reproduce this on trunk yet.
          Hide
          bruehlicke Bernd Ruehlicke added a comment -

          Just had similar stack trace when using derby 10.8.1.2 or newer. Works fine with 10.7.1.1 or below. Fails on an UPDATE trigger. I am working on a test app to reproduce the error.

          Show
          bruehlicke Bernd Ruehlicke added a comment - Just had similar stack trace when using derby 10.8.1.2 or newer. Works fine with 10.7.1.1 or below. Fails on an UPDATE trigger. I am working on a test app to reproduce the error.
          Hide
          bruehlicke Bernd Ruehlicke added a comment -

          NetBeans 8.2RC project with a single JUnit test showing this error when using 10.8.1.2. Replacing the jars with 10.7.1.1 will make the code working. Using any version higher than 10.8 will also fail.

          Show
          bruehlicke Bernd Ruehlicke added a comment - NetBeans 8.2RC project with a single JUnit test showing this error when using 10.8.1.2. Replacing the jars with 10.7.1.1 will make the code working. Using any version higher than 10.8 will also fail.
          Hide
          bruehlicke Bernd Ruehlicke added a comment -

          I was able to crystallize out the code causing the error in a JUNIT 4 test. Attached a NetBeans 8.2RC project with all setup causing the error. The README.txt file also tells how to make the error go away, i.e. which lines in the code causing it.

          I hope this will help to find the problem. The code was tested with JDK 1.8.0_102 64 bit.

          Show
          bruehlicke Bernd Ruehlicke added a comment - I was able to crystallize out the code causing the error in a JUNIT 4 test. Attached a NetBeans 8.2RC project with all setup causing the error. The README.txt file also tells how to make the error go away, i.e. which lines in the code causing it. I hope this will help to find the problem. The code was tested with JDK 1.8.0_102 64 bit.
          Hide
          bruehlicke Bernd Ruehlicke added a comment -

          The Netbeans project is is the file "derbytrig.zip"

          Show
          bruehlicke Bernd Ruehlicke added a comment - The Netbeans project is is the file "derbytrig.zip"
          Hide
          bryanpendleton Bryan Pendleton added a comment -

          Thank you for the test case. It reproduces the problem for me!

          Attached 'TriggerTest.diff' is your same test case (I believe),
          edited into the format used by the Derby regression test suites.

          When I run the modified TriggerTest on the current Derby trunk, I get:

          Caused by: java.lang.NullPointerException
          at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.examineTriggerNodeAndCols(DataDictionaryImpl.java:4773)
          at org.apache.derby.iapi.sql.dictionary.TriggerDescriptor.getSPS(TriggerDescriptor.java:407)
          at org.apache.derby.iapi.sql.dictionary.TriggerDescriptor.getActionSPS(TriggerDescriptor.java:339)
          at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.getAction(GenericTriggerExecutor.java:120)
          at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.executeWhenClauseAndAction(GenericTriggerExecutor.java:346)
          at org.apache.derby.impl.sql.execute.RowTriggerExecutor.fireTrigger(RowTriggerExecutor.java:113)
          at org.apache.derby.impl.sql.execute.TriggerEventActivator.notifyEvent(TriggerEventActivator.java:272)
          at org.apache.derby.impl.sql.execute.UpdateResultSet.fireAfterTriggers(UpdateResultSet.java:870)
          at org.apache.derby.impl.sql.execute.UpdateResultSet.open(UpdateResultSet.java:289)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:472)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeSubStatement(GenericPreparedStatement.java:338)
          at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.executeSPS(GenericTriggerExecutor.java:216)
          at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.executeWhenClauseAndAction(GenericTriggerExecutor.java:346)
          at org.apache.derby.impl.sql.execute.RowTriggerExecutor.fireTrigger(RowTriggerExecutor.java:113)
          at org.apache.derby.impl.sql.execute.TriggerEventActivator.notifyEvent(TriggerEventActivator.java:272)
          at org.apache.derby.impl.sql.execute.UpdateResultSet.fireAfterTriggers(UpdateResultSet.java:870)
          at org.apache.derby.impl.sql.execute.UpdateResultSet.open(UpdateResultSet.java:289)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:472)
          at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:351)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1339)

          Show
          bryanpendleton Bryan Pendleton added a comment - Thank you for the test case. It reproduces the problem for me! Attached 'TriggerTest.diff' is your same test case (I believe), edited into the format used by the Derby regression test suites. When I run the modified TriggerTest on the current Derby trunk, I get: Caused by: java.lang.NullPointerException at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.examineTriggerNodeAndCols(DataDictionaryImpl.java:4773) at org.apache.derby.iapi.sql.dictionary.TriggerDescriptor.getSPS(TriggerDescriptor.java:407) at org.apache.derby.iapi.sql.dictionary.TriggerDescriptor.getActionSPS(TriggerDescriptor.java:339) at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.getAction(GenericTriggerExecutor.java:120) at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.executeWhenClauseAndAction(GenericTriggerExecutor.java:346) at org.apache.derby.impl.sql.execute.RowTriggerExecutor.fireTrigger(RowTriggerExecutor.java:113) at org.apache.derby.impl.sql.execute.TriggerEventActivator.notifyEvent(TriggerEventActivator.java:272) at org.apache.derby.impl.sql.execute.UpdateResultSet.fireAfterTriggers(UpdateResultSet.java:870) at org.apache.derby.impl.sql.execute.UpdateResultSet.open(UpdateResultSet.java:289) at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:472) at org.apache.derby.impl.sql.GenericPreparedStatement.executeSubStatement(GenericPreparedStatement.java:338) at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.executeSPS(GenericTriggerExecutor.java:216) at org.apache.derby.impl.sql.execute.GenericTriggerExecutor.executeWhenClauseAndAction(GenericTriggerExecutor.java:346) at org.apache.derby.impl.sql.execute.RowTriggerExecutor.fireTrigger(RowTriggerExecutor.java:113) at org.apache.derby.impl.sql.execute.TriggerEventActivator.notifyEvent(TriggerEventActivator.java:272) at org.apache.derby.impl.sql.execute.UpdateResultSet.fireAfterTriggers(UpdateResultSet.java:870) at org.apache.derby.impl.sql.execute.UpdateResultSet.open(UpdateResultSet.java:289) at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:472) at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:351) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1339)
          Hide
          bruehlicke Bernd Ruehlicke added a comment -

          Super ! Thanx, it is always great to hear if a problem can be reproduced. This stack trace I get using 10.12.1.1. When using 10.8.1.2 - 10.11.1.1 a get a slightly different and with 10.7.1.1 and lower version, all works perfect without any error.

          Show
          bruehlicke Bernd Ruehlicke added a comment - Super ! Thanx, it is always great to hear if a problem can be reproduced. This stack trace I get using 10.12.1.1. When using 10.8.1.2 - 10.11.1.1 a get a slightly different and with 10.7.1.1 and lower version, all works perfect without any error.
          Hide
          bruehlicke Bernd Ruehlicke added a comment - - edited

          Trying to understand more on this and going through the Derby code, it looks to me that the initial guess of "it could be that its caused by another AFTER UPDATE trigger that's on the table." could be a guess in the right direction. In the test I provided there are 2 AFTER UPDATE triggers - both on the same table. Disabling any of them makes the code work - but having both make the code fail.

          Show
          bruehlicke Bernd Ruehlicke added a comment - - edited Trying to understand more on this and going through the Derby code, it looks to me that the initial guess of "it could be that its caused by another AFTER UPDATE trigger that's on the table." could be a guess in the right direction. In the test I provided there are 2 AFTER UPDATE triggers - both on the same table. Disabling any of them makes the code work - but having both make the code fail.
          Hide
          bryanpendleton Bryan Pendleton added a comment -

          That seems like a very important insight, thanks. It seems to me that this
          indicates that the TriggerEventActivator is possibly the important class
          to investigate, as that appears to be the frame in the call stack where
          the different triggers are being tracked, ordered, and scheduled for execution.

          Show
          bryanpendleton Bryan Pendleton added a comment - That seems like a very important insight, thanks. It seems to me that this indicates that the TriggerEventActivator is possibly the important class to investigate, as that appears to be the frame in the call stack where the different triggers are being tracked, ordered, and scheduled for execution.
          Hide
          bruehlicke Bernd Ruehlicke added a comment - - edited

          I have tracked down the problem,

          First I was comparing sources changes between 7.1.1 and 8.1.2 to reduce amount of source to be checked and as I know it worked in 7.1.1. and failed in 8.1.2. GenericTriggerExecutor, RowTriggerExecutor. TriggerEventActivator, UpdateResultSet and GenericPreparedStatement where the same - so no problem should be in those.

          The problem happens in : iapi/sql/dictionary/TriggerDescriptor in the method getActionSPS. (In 12.1.1. this is getSPS).

          In 8.1.2 code was added to the method as part of DERBY-4874. When I comment out this extra code in the 8.1.2 code base - the Test works ! I do not currently understand what this extra code is doing, but it is for sure the culprit of the problem.

          Now looking at the 12.1.1 code we have the same or similar code in TriggerDescriptor.getSPS(...). Again, commenting out the addition made for DERBY-4874 and just have it do the same as back in the 7.1.1 days - the code works also on 12.1.1

          I will try to understand what the getSPS(..) is trying to do and why it falls over having a double set of AFTER UPDATE triggers, but maybe someone closer to the code can help with ideas as the problem is now pinned down to a few lines of source.

          I will now go into the 12.1.1 code base and try to understand the logic here

          Show
          bruehlicke Bernd Ruehlicke added a comment - - edited I have tracked down the problem, First I was comparing sources changes between 7.1.1 and 8.1.2 to reduce amount of source to be checked and as I know it worked in 7.1.1. and failed in 8.1.2. GenericTriggerExecutor, RowTriggerExecutor. TriggerEventActivator, UpdateResultSet and GenericPreparedStatement where the same - so no problem should be in those. The problem happens in : iapi/sql/dictionary/TriggerDescriptor in the method getActionSPS. (In 12.1.1. this is getSPS). In 8.1.2 code was added to the method as part of DERBY-4874 . When I comment out this extra code in the 8.1.2 code base - the Test works ! I do not currently understand what this extra code is doing, but it is for sure the culprit of the problem. Now looking at the 12.1.1 code we have the same or similar code in TriggerDescriptor.getSPS(...). Again, commenting out the addition made for DERBY-4874 and just have it do the same as back in the 7.1.1 days - the code works also on 12.1.1 I will try to understand what the getSPS(..) is trying to do and why it falls over having a double set of AFTER UPDATE triggers, but maybe someone closer to the code can help with ideas as the problem is now pinned down to a few lines of source. I will now go into the 12.1.1 code base and try to understand the logic here
          Hide
          bruehlicke Bernd Ruehlicke added a comment -

          Looks like the rabbit hole goes deeper ... looks like the core of the problem is hidden way deeper but hard to pin down not knowing the code. I debugged the test and found that the TriggerEventActivator seem to do its job correctly. It finds both triggers and executes both after each other. Going up the stack trace it become more blurry. I will try to debug, below what I have found so far ... maybe someone has a light bulb going on after seeing this ... for me it is still deep darkness what is going on.

          TriggerDescriptor.getSPS fails because the TableDescriptor is null
          it worked in 7.1.1 as no TableDescriptor was needed

          TriggerEventActivator correctly identified both triggers as eventNumber = 5 with i=0 the first trigger and i=2 the second.
          .notifyEvent(..) line 272 though calls the fireTrigger with "colsReadFromTable" which is null and which should contain the columns required from the Trigger Table

          UpdateResultSet.fireAfterTriggers() calls the TriggerEventActivator passing down "constants.getBaseRowReadMap()" which is null
          "constants" is a UpdateConstantAction which is a WriteCursorConstantAction which is a ConstantAction (Interface) and is passed into the class as an ConstantAction from a GenericResultSetFactory.getUpdateResultSet it gets created new UpdateResultSet(source, generationClauses, checkGM, activation);

          UpdateResultSet constructor will call its this with the activation as well as activation.getConstantAction() as "passedInConstantAction" which will be the "constants" which returns null when called on getBaseRowReadMap().

          Show
          bruehlicke Bernd Ruehlicke added a comment - Looks like the rabbit hole goes deeper ... looks like the core of the problem is hidden way deeper but hard to pin down not knowing the code. I debugged the test and found that the TriggerEventActivator seem to do its job correctly. It finds both triggers and executes both after each other. Going up the stack trace it become more blurry. I will try to debug, below what I have found so far ... maybe someone has a light bulb going on after seeing this ... for me it is still deep darkness what is going on. TriggerDescriptor.getSPS fails because the TableDescriptor is null it worked in 7.1.1 as no TableDescriptor was needed TriggerEventActivator correctly identified both triggers as eventNumber = 5 with i=0 the first trigger and i=2 the second. .notifyEvent(..) line 272 though calls the fireTrigger with "colsReadFromTable" which is null and which should contain the columns required from the Trigger Table UpdateResultSet.fireAfterTriggers() calls the TriggerEventActivator passing down "constants.getBaseRowReadMap()" which is null "constants" is a UpdateConstantAction which is a WriteCursorConstantAction which is a ConstantAction (Interface) and is passed into the class as an ConstantAction from a GenericResultSetFactory.getUpdateResultSet it gets created new UpdateResultSet(source, generationClauses, checkGM, activation); UpdateResultSet constructor will call its this with the activation as well as activation.getConstantAction() as "passedInConstantAction" which will be the "constants" which returns null when called on getBaseRowReadMap().
          Hide
          bruehlicke Bernd Ruehlicke added a comment - - edited

          OK - after 7 hours debugging the reason is found - Bryan your instincts serve you well ... The main error is in TriggerEventActivator.

          for 10.12.1.1 it is in the line 86 which has
          tableName = triggerInfo.triggerArray[0].getTableDescriptor().getQualifiedName();

          Note that the triggerInfo.triggerArray is HARDCODED to use index 0. Now this gives the needed tableName, BUT the "getTableDiscriptor()" method in TriggerDescriptor is actually a SETTER !!! It SETS td if td is null, That's why it works when just one trigger is in the test code and fails if 2 as the second trigger's td is never set. This bug must have been here for a long time and never surfaced.

          So, in the TriggerEventActivator line 86 the triggerInfo.triggerArray has correctly the 2 triggers but onlyindex 0 gets its "td" set via the "getTableDescriptor()". To test this just add code which will loop for the length of the triggerArray and call the getTableDescritptor on each element.

          I.e; HACK to make it work (assuming the tableName is the same for each of the triggerInfo TableDescription ), replace line 86 with this chunk of code and it will work, it is of course the "setting" behavior of the getTableDescriptor which is the ugly part here,

          tableName = triggerInfo.triggerArray[0].getTableDescriptor().getQualifiedName();
          for(int i=1;i<triggerInfo.triggerArray.length;i++)

          { triggerInfo.triggerArray[i].getTableDescriptor(); }
          Show
          bruehlicke Bernd Ruehlicke added a comment - - edited OK - after 7 hours debugging the reason is found - Bryan your instincts serve you well ... The main error is in TriggerEventActivator. for 10.12.1.1 it is in the line 86 which has tableName = triggerInfo.triggerArray [0] .getTableDescriptor().getQualifiedName(); Note that the triggerInfo.triggerArray is HARDCODED to use index 0. Now this gives the needed tableName, BUT the "getTableDiscriptor()" method in TriggerDescriptor is actually a SETTER !!! It SETS td if td is null, That's why it works when just one trigger is in the test code and fails if 2 as the second trigger's td is never set. This bug must have been here for a long time and never surfaced. So, in the TriggerEventActivator line 86 the triggerInfo.triggerArray has correctly the 2 triggers but onlyindex 0 gets its "td" set via the "getTableDescriptor()". To test this just add code which will loop for the length of the triggerArray and call the getTableDescritptor on each element. I.e; HACK to make it work (assuming the tableName is the same for each of the triggerInfo TableDescription ), replace line 86 with this chunk of code and it will work, it is of course the "setting" behavior of the getTableDescriptor which is the ugly part here, tableName = triggerInfo.triggerArray [0] .getTableDescriptor().getQualifiedName(); for(int i=1;i<triggerInfo.triggerArray.length;i++) { triggerInfo.triggerArray[i].getTableDescriptor(); }
          Hide
          bryanpendleton Bryan Pendleton added a comment -

          Bernd, firstly, thank you VERY much for putting this energy into this issue! Also, thank
          you for the clear and detailed notes about your investigations, it is extremely helpful.

          I will try to find some time this weekend to replicate your findings in detail, and to see
          if I can add any additional clarity to your observations so far.

          My instinct is that the initialization loop that you describe is not really a hack, it is a
          reasonable approach to correctly setting up the runtime data structures, including
          the table descriptors.

          I agree with you that it is disturbing that "getTableDescriptor()" is the routine that
          gets used to initialize the table descriptor, although this sort of "initialize upon first
          reference" is not all that uncommon in the Derby codebase.

          The other question is where, precisely, we want that initialization loop to be, but as
          a first guess, line 86 of TriggerEventActivator seems like a place to start.

          As I said, I will try to find a few hours soon to dig through the code and catch up
          with your findings and give you some more feedback about whatever I might learn.

          Show
          bryanpendleton Bryan Pendleton added a comment - Bernd, firstly, thank you VERY much for putting this energy into this issue! Also, thank you for the clear and detailed notes about your investigations, it is extremely helpful. I will try to find some time this weekend to replicate your findings in detail, and to see if I can add any additional clarity to your observations so far. My instinct is that the initialization loop that you describe is not really a hack, it is a reasonable approach to correctly setting up the runtime data structures, including the table descriptors. I agree with you that it is disturbing that "getTableDescriptor()" is the routine that gets used to initialize the table descriptor, although this sort of "initialize upon first reference" is not all that uncommon in the Derby codebase. The other question is where, precisely, we want that initialization loop to be, but as a first guess, line 86 of TriggerEventActivator seems like a place to start. As I said, I will try to find a few hours soon to dig through the code and catch up with your findings and give you some more feedback about whatever I might learn.
          Hide
          bryanpendleton Bryan Pendleton added a comment -

          With Bernd's proposed change to the TriggerEventActivator constructor,
          the reproduction script passes, and the other tests in TriggerTest
          pass as well. Attached 'fixesRepro.diff' contains that diff.

          I'll do some more testing with this patch proposal.

          Show
          bryanpendleton Bryan Pendleton added a comment - With Bernd's proposed change to the TriggerEventActivator constructor, the reproduction script passes, and the other tests in TriggerTest pass as well. Attached 'fixesRepro.diff' contains that diff. I'll do some more testing with this patch proposal.
          Hide
          bryanpendleton Bryan Pendleton added a comment -

          The entire regression suite passes with 'fixesRepro.diff'.

          Show
          bryanpendleton Bryan Pendleton added a comment - The entire regression suite passes with 'fixesRepro.diff'.
          Hide
          bryanpendleton Bryan Pendleton added a comment -

          It seems like another potential way to fix this problem is tied to Bernd's observation
          that TriggerDescriptor's 'td' member is a lazy-initialize field, which is set when it
          is first referenced by the getTableDescriptor() method.

          But in the TriggerDescriptor code itself, we don't always go through getTableDescriptor();
          sometimes we directly reference 'td'. Such code will fail if td is still NULL at that point,
          which is precisely the description of the test case scenario.

          So perhaps, instead of the initialization loop in TriggerEventActivator, we could
          go through all of TriggerDescriptor, and every time we reference 'td', we instead
          reference it by calling getTableDescriptor(), thus allowing 'td' to auto-initialize properly.

          I'll give that a try and see how it behaves.

          bryan

          Show
          bryanpendleton Bryan Pendleton added a comment - It seems like another potential way to fix this problem is tied to Bernd's observation that TriggerDescriptor's 'td' member is a lazy-initialize field, which is set when it is first referenced by the getTableDescriptor() method. But in the TriggerDescriptor code itself, we don't always go through getTableDescriptor(); sometimes we directly reference 'td'. Such code will fail if td is still NULL at that point, which is precisely the description of the test case scenario. So perhaps, instead of the initialization loop in TriggerEventActivator, we could go through all of TriggerDescriptor, and every time we reference 'td', we instead reference it by calling getTableDescriptor(), thus allowing 'td' to auto-initialize properly. I'll give that a try and see how it behaves. bryan
          Hide
          bruehlicke Bernd Ruehlicke added a comment -

          Yep - sounds better to me. I did not like my little hack. It is at the "wrong" place and the loop starts with i=1 and no-one would understand what is going on when reading it (if no comments are added why this was added). How would the td auto-initialization approach work in a multi threaded environment ? Is Derby actually (in theory) thread safe ?

          Thinking about the stack trace I cannot see any other point where td is used directly and not just passed along. I wonder if this idea will work with the above test case. Exciting to hear if it worked for you.

          Show
          bruehlicke Bernd Ruehlicke added a comment - Yep - sounds better to me. I did not like my little hack. It is at the "wrong" place and the loop starts with i=1 and no-one would understand what is going on when reading it (if no comments are added why this was added). How would the td auto-initialization approach work in a multi threaded environment ? Is Derby actually (in theory) thread safe ? Thinking about the stack trace I cannot see any other point where td is used directly and not just passed along. I wonder if this idea will work with the above test case. Exciting to hear if it worked for you.
          Hide
          bryanpendleton Bryan Pendleton added a comment -

          Attached 'getTableDescriptor.diff' is an alternate patch proposal.

          In some ways, this patch seems more precise and desirable, as it
          fixes the problem closer to the symptom, that is, inside TriggerDescriptor
          itself, which is the class that *ought* to encapsulate the knowledge of
          the lazy-initialization of the 'td' member field. The other patch could be
          critiqued because it allowed that knowledge to "leak out" into the
          TriggerEventActivator class.

          The new patch passes the reproduction script; I'll do more testing.

          Show
          bryanpendleton Bryan Pendleton added a comment - Attached 'getTableDescriptor.diff' is an alternate patch proposal. In some ways, this patch seems more precise and desirable, as it fixes the problem closer to the symptom, that is, inside TriggerDescriptor itself, which is the class that * ought * to encapsulate the knowledge of the lazy-initialization of the 'td' member field. The other patch could be critiqued because it allowed that knowledge to "leak out" into the TriggerEventActivator class. The new patch passes the reproduction script; I'll do more testing.
          Hide
          bruehlicke Bernd Ruehlicke added a comment -

          Looked at the getTableDescriptor.diff - if all tests pass, it for sure has my vote. As you are stating so correctly it keeps the knowledge of the initialization internal allowing to user to be "clueless" which is good for any API development. Thank you for this Bryan ! I can now use 12.1.1 + patch and move forward.

          Show
          bruehlicke Bernd Ruehlicke added a comment - Looked at the getTableDescriptor.diff - if all tests pass, it for sure has my vote. As you are stating so correctly it keeps the knowledge of the initialization internal allowing to user to be "clueless" which is good for any API development. Thank you for this Bryan ! I can now use 12.1.1 + patch and move forward.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1763024 from Bryan Pendleton in branch 'code/trunk'
          [ https://svn.apache.org/r1763024 ]

          DERBY-6726: NPE from trigger

          When there are multiple triggers on the same table, it is crucial that the
          TriggerDescriptor class always uses the getTableDescriptor() getter method
          to access its 'td' member field, so that the field can be lazy-initialized
          if it has not yet been set.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1763024 from Bryan Pendleton in branch 'code/trunk' [ https://svn.apache.org/r1763024 ] DERBY-6726 : NPE from trigger When there are multiple triggers on the same table, it is crucial that the TriggerDescriptor class always uses the getTableDescriptor() getter method to access its 'td' member field, so that the field can be lazy-initialized if it has not yet been set.
          Hide
          bryanpendleton Bryan Pendleton added a comment -

          Thank you for the code review Bernd, and thanks once again for all the help on
          tracking this down. It seems that we've had this issue for a while, and it's nice to
          feel like we may have addressed it.

          Please do let us know of your further experiences running with this fix in your environment.

          I've left the issue open for now, because I intend to back-port it to (at least) the
          10.12 and 10.13 branches. We could certainly back-port it all the way back to the 10.8
          branch, but I'm not planning to do that at this time.

          Show
          bryanpendleton Bryan Pendleton added a comment - Thank you for the code review Bernd, and thanks once again for all the help on tracking this down. It seems that we've had this issue for a while, and it's nice to feel like we may have addressed it. Please do let us know of your further experiences running with this fix in your environment. I've left the issue open for now, because I intend to back-port it to (at least) the 10.12 and 10.13 branches. We could certainly back-port it all the way back to the 10.8 branch, but I'm not planning to do that at this time.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1763034 from Bryan Pendleton in branch 'code/branches/10.13'
          [ https://svn.apache.org/r1763034 ]

          DERBY-6726: NPE from trigger

          Merged from main via svn merge of 1763024 from trunk (no conflicts)

          When there are multiple triggers on the same table, it is crucial that the
          TriggerDescriptor class always uses the getTableDescriptor() getter method
          to access its 'td' member field, so that the field can be lazy-initialized
          if it has not yet been set.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1763034 from Bryan Pendleton in branch 'code/branches/10.13' [ https://svn.apache.org/r1763034 ] DERBY-6726 : NPE from trigger Merged from main via svn merge of 1763024 from trunk (no conflicts) When there are multiple triggers on the same table, it is crucial that the TriggerDescriptor class always uses the getTableDescriptor() getter method to access its 'td' member field, so that the field can be lazy-initialized if it has not yet been set.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1763083 from Bryan Pendleton in branch 'code/branches/10.12'
          [ https://svn.apache.org/r1763083 ]

          DERBY-6726: NPE from trigger

          Merged from main via svn merge of 1763024 from trunk (no conflicts)

          When there are multiple triggers on the same table, it is crucial that the
          TriggerDescriptor class always uses the getTableDescriptor() getter method
          to access its 'td' member field, so that the field can be lazy-initialized
          if it has not yet been set.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1763083 from Bryan Pendleton in branch 'code/branches/10.12' [ https://svn.apache.org/r1763083 ] DERBY-6726 : NPE from trigger Merged from main via svn merge of 1763024 from trunk (no conflicts) When there are multiple triggers on the same table, it is crucial that the TriggerDescriptor class always uses the getTableDescriptor() getter method to access its 'td' member field, so that the field can be lazy-initialized if it has not yet been set.
          Hide
          bryanpendleton Bryan Pendleton added a comment -

          We've completed all the intended work on this issue. Resolving.

          Show
          bryanpendleton Bryan Pendleton added a comment - We've completed all the intended work on this issue. Resolving.

            People

            • Assignee:
              bryanpendleton Bryan Pendleton
              Reporter:
              tdudgeon Tim Dudgeon
            • Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development