Derby
  1. Derby
  2. DERBY-4538

If the CREATE TRIGGER does not have the REFERENCING clause, then there is no need to keep before and after values for the triggering table

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.6.1.0
    • Fix Version/s: 10.3.3.1, 10.4.2.1, 10.5.3.2, 10.6.1.0
    • Component/s: SQL
    • Labels:
      None
    • Bug behavior facts:
      Performance

      Description

      In order for the trigger action to have access to before and after values of the triggering table, the CREATE TRIGGER should use the REFERENCING clause. Without the REFERENCING clause, old and new values of triggering table can't be accessed by the trigger action. Based on this, we can improve Derby memory utilization by not keeping old and new values if REFERENCING clause is missing. It will be good to see if the code already does this optimization and if not, then introducing this optimization will definitely be very useful when the triggering table could have LOB columns.

      1. DERBY4538_NoReferencingClause_stat_v2.txt
        0.4 kB
        Mamta A. Satoor
      2. DERBY4538_NoReferencingClause_stat_v1.txt
        0.1 kB
        Mamta A. Satoor
      3. DERBY4538_NoReferencingClause_diff_v2.txt
        25 kB
        Mamta A. Satoor
      4. DERBY4538_NoReferencingClause_diff_v1.txt
        5 kB
        Mamta A. Satoor

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.
          Hide
          Mamta A. Satoor added a comment - - edited

          Knut, thanks for reviwing the patch. I committed the changes(with revision 917771) after removing the redundant ;

          Will go ahead and close this issue.

          Show
          Mamta A. Satoor added a comment - - edited Knut, thanks for reviwing the patch. I committed the changes(with revision 917771) after removing the redundant ; Will go ahead and close this issue.
          Hide
          Knut Anders Hatlen added a comment -

          v2 looks like a good incremental improvement. +1 to commit.

          (Nit: The declarations of needToIncludeAllColumns end with two semi-colons.)

          Show
          Knut Anders Hatlen added a comment - v2 looks like a good incremental improvement. +1 to commit. (Nit: The declarations of needToIncludeAllColumns end with two semi-colons.)
          Hide
          Mamta A. Satoor added a comment -

          Attaching patch DERBY4538_NoReferencingClause_diff_v2.txt which is ready for commit. This changes the UPDATE and DELETE statement codes to be little bit smarter when they decide what columns should be part of the read map. Currently, as soon as these 2 nodes find that there are relevant triggers on the table, we decide to read all the columns from the table. I am changing code to check if all the relevant tiggers have missing REFERENCING clause. If yes, then do not need to read all the columns. Just the columns needed by the UPDATE/DELETE statement. This will get rid of OOM we run into when the table has LOB columns BUT only in the case when the UPDATE/DELETE statement does not reference the LOB column and all the triggers defined on them have missing REFERENCING clause. I have enabled the TriggerTests in lowmem suite with the missing REFERENCING clause cases enabled. For all the other test cases, I simply return from those test cases without actually testing it because we do not have fix for those cases yet. The lowmem suite does not get regularly and when it is run, as the name indicates, it runs with limited heap. I wanted us to be able to run these tests with default heap as well. To achieve that, I am including the TriggerTests in lang suite too. Please let me know if there are any questions. I will commit this patch sometime next week.

          The INSERT table with INSERT triggers work fine already without my changes as long as the INSERT statement does not reference the LOB column.

          Show
          Mamta A. Satoor added a comment - Attaching patch DERBY4538_NoReferencingClause_diff_v2.txt which is ready for commit. This changes the UPDATE and DELETE statement codes to be little bit smarter when they decide what columns should be part of the read map. Currently, as soon as these 2 nodes find that there are relevant triggers on the table, we decide to read all the columns from the table. I am changing code to check if all the relevant tiggers have missing REFERENCING clause. If yes, then do not need to read all the columns. Just the columns needed by the UPDATE/DELETE statement. This will get rid of OOM we run into when the table has LOB columns BUT only in the case when the UPDATE/DELETE statement does not reference the LOB column and all the triggers defined on them have missing REFERENCING clause. I have enabled the TriggerTests in lowmem suite with the missing REFERENCING clause cases enabled. For all the other test cases, I simply return from those test cases without actually testing it because we do not have fix for those cases yet. The lowmem suite does not get regularly and when it is run, as the name indicates, it runs with limited heap. I wanted us to be able to run these tests with default heap as well. To achieve that, I am including the TriggerTests in lang suite too. Please let me know if there are any questions. I will commit this patch sometime next week. The INSERT table with INSERT triggers work fine already without my changes as long as the INSERT statement does not reference the LOB column.
          Hide
          Mamta A. Satoor added a comment -

          I am attaching a patch (not ready for commit yet), DERBY4538_NoReferencingClause_diff_v1.txt. This patch takes care of UPDATE triggers with no REFERENCING clause defined on them. During Update, Derby tries to determine which columns need to be read from the triggering table. As soon as we find out that the update table has triggers defined on it, we decide to read all the columns. We can improve on this algorithm My patch is attempting to be more intelligant about what columns should be read. The new logic is as follows.
          /*

            • If we have any triggers, then do one of the following
            • 1)If all of the triggers have MISSING referencing clause, then that
            • means that the trigger actions do not have access to before and
            • after values. In that case, there is no need to blanketly decide to
            • include all the columns in the read map just because there are
            • triggers defined on the table.
            • 2)Since one/more triggers have REFERENCING clause on them, get all
            • the columns because we don't know what the user will ultimately reference.
              */

          Would love to hear if anyone has any feedback on this approach. Similar changes need to go in for INSERT and UPDATE statements. I will post a patch which will include those changes once i have it ready.

          Show
          Mamta A. Satoor added a comment - I am attaching a patch (not ready for commit yet), DERBY4538_NoReferencingClause_diff_v1.txt. This patch takes care of UPDATE triggers with no REFERENCING clause defined on them. During Update, Derby tries to determine which columns need to be read from the triggering table. As soon as we find out that the update table has triggers defined on it, we decide to read all the columns. We can improve on this algorithm My patch is attempting to be more intelligant about what columns should be read. The new logic is as follows. /* If we have any triggers, then do one of the following 1)If all of the triggers have MISSING referencing clause, then that means that the trigger actions do not have access to before and after values. In that case, there is no need to blanketly decide to include all the columns in the read map just because there are triggers defined on the table. 2)Since one/more triggers have REFERENCING clause on them, get all the columns because we don't know what the user will ultimately reference. */ Would love to hear if anyone has any feedback on this approach. Similar changes need to go in for INSERT and UPDATE statements. I will post a patch which will include those changes once i have it ready.

            People

            • Assignee:
              Mamta A. Satoor
              Reporter:
              Mamta A. Satoor
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development