Possible patch to remove "triggered data change" support - Mailing list pgsql-patches

From Tom Lane
Subject Possible patch to remove "triggered data change" support
Date
Msg-id 29224.1005608477@sss.pgh.pa.us
Whole thread Raw
Responses Re: Possible patch to remove "triggered data change" support
List pgsql-patches
Attached is a patch to remove the "triggered data change" error check
from trigger.c, as well as pick up the performance improvements that
that allows (due to not having to save deferred trigger events as long).

I am not yet proposing to actually commit this, since the discussion
about whether it's a good idea is still ongoing in -hackers.  But
Stephan seemed to want to have a copy for testing purposes, and I
thought other people might be interested too.

            regards, tom lane

*** src/backend/commands/trigger.c.orig    Sun Nov 11 19:00:55 2001
--- src/backend/commands/trigger.c    Mon Nov 12 18:31:58 2001
***************
*** 935,944 ****
  {
      TriggerDesc *trigdesc = relinfo->ri_TrigDesc;

!     /* Must save info if there are any deferred triggers on this rel */
!     if (trigdesc->n_after_row[TRIGGER_EVENT_INSERT] > 0 ||
!         trigdesc->n_after_row[TRIGGER_EVENT_UPDATE] > 0 ||
!         trigdesc->n_after_row[TRIGGER_EVENT_DELETE] > 0)
          DeferredTriggerSaveEvent(relinfo, TRIGGER_EVENT_INSERT,
                                   NULL, trigtuple);
  }
--- 935,941 ----
  {
      TriggerDesc *trigdesc = relinfo->ri_TrigDesc;

!     if (trigdesc->n_after_row[TRIGGER_EVENT_INSERT] > 0)
          DeferredTriggerSaveEvent(relinfo, TRIGGER_EVENT_INSERT,
                                   NULL, trigtuple);
  }
***************
*** 1000,1008 ****
  {
      TriggerDesc *trigdesc = relinfo->ri_TrigDesc;

!     /* Must save info if there are upd/del deferred triggers on this rel */
!     if (trigdesc->n_after_row[TRIGGER_EVENT_UPDATE] > 0 ||
!         trigdesc->n_after_row[TRIGGER_EVENT_DELETE] > 0)
      {
          HeapTuple    trigtuple = GetTupleForTrigger(estate, relinfo,
                                                     tupleid, NULL);
--- 997,1003 ----
  {
      TriggerDesc *trigdesc = relinfo->ri_TrigDesc;

!     if (trigdesc->n_after_row[TRIGGER_EVENT_DELETE] > 0)
      {
          HeapTuple    trigtuple = GetTupleForTrigger(estate, relinfo,
                                                     tupleid, NULL);
***************
*** 1077,1085 ****
  {
      TriggerDesc *trigdesc = relinfo->ri_TrigDesc;

!     /* Must save info if there are upd/del deferred triggers on this rel */
!     if (trigdesc->n_after_row[TRIGGER_EVENT_UPDATE] > 0 ||
!         trigdesc->n_after_row[TRIGGER_EVENT_DELETE] > 0)
      {
          HeapTuple    trigtuple = GetTupleForTrigger(estate, relinfo,
                                                     tupleid, NULL);
--- 1072,1078 ----
  {
      TriggerDesc *trigdesc = relinfo->ri_TrigDesc;

!     if (trigdesc->n_after_row[TRIGGER_EVENT_UPDATE] > 0)
      {
          HeapTuple    trigtuple = GetTupleForTrigger(estate, relinfo,
                                                     tupleid, NULL);
***************
*** 1208,1224 ****
  static List *deftrig_trigstates;

  /* ----------
!  * The list of events during the entire transaction.  deftrig_events
!  * is the head, deftrig_event_tail is the last entry.  Because this can
!  * grow pretty large, we don't use separate List nodes, but instead thread
!  * the list through the dte_next fields of the member nodes.  Saves just a
!  * few bytes per entry, but that adds up.
   *
   * XXX Need to be able to shove this data out to a file if it grows too
   *       large...
   * ----------
   */
- static int    deftrig_n_events;
  static DeferredTriggerEvent deftrig_events;
  static DeferredTriggerEvent deftrig_event_tail;

--- 1201,1217 ----
  static List *deftrig_trigstates;

  /* ----------
!  * The list of pending deferred trigger events during the current transaction.
!  *
!  * deftrig_events is the head, deftrig_event_tail is the last entry.
!  * Because this can grow pretty large, we don't use separate List nodes,
!  * but instead thread the list through the dte_next fields of the member
!  * nodes.  Saves just a few bytes per entry, but that adds up.
   *
   * XXX Need to be able to shove this data out to a file if it grows too
   *       large...
   * ----------
   */
  static DeferredTriggerEvent deftrig_events;
  static DeferredTriggerEvent deftrig_event_tail;

***************
*** 1284,1290 ****
   * deferredTriggerAddEvent()
   *
   *    Add a new trigger event to the queue.
-  *    Caller must have switched into appropriate memory context!
   * ----------
   */
  static void
--- 1277,1282 ----
***************
*** 1307,1350 ****
          deftrig_event_tail->dte_next = event;
          deftrig_event_tail = event;
      }
-     deftrig_n_events++;
- }
-
-
- /* ----------
-  * deferredTriggerGetPreviousEvent()
-  *
-  *    Scan the eventlist to find the event a given OLD tuple
-  *    resulted from in the same transaction.
-  * ----------
-  */
- static DeferredTriggerEvent
- deferredTriggerGetPreviousEvent(Oid relid, ItemPointer ctid)
- {
-     DeferredTriggerEvent previous = NULL;
-     DeferredTriggerEvent prev;
-
-     /* Search the list to find the last event affecting this tuple */
-     for (prev = deftrig_events; prev != NULL; prev = prev->dte_next)
-     {
-         if (prev->dte_relid != relid)
-             continue;
-         if (prev->dte_event & TRIGGER_DEFERRED_CANCELED)
-             continue;
-
-         if (ItemPointerGetBlockNumber(ctid) ==
-             ItemPointerGetBlockNumber(&(prev->dte_newctid)) &&
-             ItemPointerGetOffsetNumber(ctid) ==
-             ItemPointerGetOffsetNumber(&(prev->dte_newctid)))
-             previous = prev;
-     }
-
-     if (previous == NULL)
-         elog(ERROR,
-          "deferredTriggerGetPreviousEvent: event for tuple %s not found",
-              DatumGetCString(DirectFunctionCall1(tidout,
-                                                  PointerGetDatum(ctid))));
-     return previous;
  }


--- 1299,1304 ----
***************
*** 1473,1490 ****
  static void
  deferredTriggerInvokeEvents(bool immediate_only)
  {
!     DeferredTriggerEvent event;
      MemoryContext per_tuple_context;
      Relation    rel = NULL;
      FmgrInfo   *finfo = NULL;

      /*
!      * For now we process all events - to speedup transaction blocks we
!      * need to remember the actual end of the queue at EndQuery and
!      * process only events that are newer. On state changes we simply
!      * reset the position to the beginning of the queue and process all
!      * events once with the new states when the SET CONSTRAINTS ...
!      * command finishes and calls EndQuery.
       */

      /* Make a per-tuple memory context for trigger function calls */
--- 1427,1451 ----
  static void
  deferredTriggerInvokeEvents(bool immediate_only)
  {
!     DeferredTriggerEvent event,
!                 prev_event = NULL;
      MemoryContext per_tuple_context;
      Relation    rel = NULL;
      FmgrInfo   *finfo = NULL;

      /*
!      * If immediate_only is true, we remove fully-processed events from
!      * the event queue to recycle space.  If immediate_only is false,
!      * we are going to discard the whole event queue on return anyway,
!      * so no need to bother with "retail" pfree's.
!      *
!      * In a scenario with many commands in a transaction and many
!      * deferred-to-end-of-transaction triggers, it could get annoying
!      * to rescan all the deferred triggers at each command end.
!      * To speed this up, we could remember the actual end of the queue at
!      * EndQuery and examine only events that are newer. On state changes
!      * we simply reset the saved position to the beginning of the queue
!      * and process all events once with the new states.
       */

      /* Make a per-tuple memory context for trigger function calls */
***************
*** 1495,1574 ****
                                ALLOCSET_DEFAULT_INITSIZE,
                                ALLOCSET_DEFAULT_MAXSIZE);

!     for (event = deftrig_events; event != NULL; event = event->dte_next)
      {
!         bool        still_deferred_ones;
          int            i;

          /*
!          * Check if event is completely done.
!          */
!         if (event->dte_event & (TRIGGER_DEFERRED_DONE |
!                                 TRIGGER_DEFERRED_CANCELED))
!             continue;
!
!         MemoryContextReset(per_tuple_context);
!
!         /*
!          * Check each trigger item in the event.
           */
!         still_deferred_ones = false;
!         for (i = 0; i < event->dte_n_items; i++)
          {
!             if (event->dte_item[i].dti_state & TRIGGER_DEFERRED_DONE)
!                 continue;
!
!             /*
!              * This trigger item hasn't been called yet. Check if we
!              * should call it now.
!              */
!             if (immediate_only && deferredTriggerCheckState(
!                                             event->dte_item[i].dti_tgoid,
!                                            event->dte_item[i].dti_state))
!             {
!                 still_deferred_ones = true;
!                 continue;
!             }

              /*
!              * So let's fire it... but first, open the correct relation if
!              * this is not the same relation as before.
               */
!             if (rel == NULL || rel->rd_id != event->dte_relid)
              {
!                 if (rel)
!                     heap_close(rel, NoLock);
!                 if (finfo)
!                     pfree(finfo);

                  /*
!                  * We assume that an appropriate lock is still held by the
!                  * executor, so grab no new lock here.
                   */
!                 rel = heap_open(event->dte_relid, NoLock);

                  /*
!                  * Allocate space to cache fmgr lookup info for triggers
!                  * of this relation.
                   */
!                 finfo = (FmgrInfo *)
!                     palloc(rel->trigdesc->numtriggers * sizeof(FmgrInfo));
!                 MemSet(finfo, 0,
!                        rel->trigdesc->numtriggers * sizeof(FmgrInfo));
!             }

!             DeferredTriggerExecute(event, i, rel, finfo, per_tuple_context);

!             event->dte_item[i].dti_state |= TRIGGER_DEFERRED_DONE;
          }

          /*
!          * Remember in the event itself if all trigger items are done.
           */
!         if (!still_deferred_ones)
!             event->dte_event |= TRIGGER_DEFERRED_DONE;
      }

      if (rel)
          heap_close(rel, NoLock);
      if (finfo)
--- 1456,1573 ----
                                ALLOCSET_DEFAULT_INITSIZE,
                                ALLOCSET_DEFAULT_MAXSIZE);

!     event = deftrig_events;
!     while (event != NULL)
      {
!         bool        still_deferred_ones = false;
!         DeferredTriggerEvent next_event;
          int            i;

          /*
!          * Check if event is already completely done.
           */
!         if (! (event->dte_event & (TRIGGER_DEFERRED_DONE |
!                                    TRIGGER_DEFERRED_CANCELED)))
          {
!             MemoryContextReset(per_tuple_context);

              /*
!              * Check each trigger item in the event.
               */
!             for (i = 0; i < event->dte_n_items; i++)
              {
!                 if (event->dte_item[i].dti_state & TRIGGER_DEFERRED_DONE)
!                     continue;

                  /*
!                  * This trigger item hasn't been called yet. Check if we
!                  * should call it now.
                   */
!                 if (immediate_only &&
!                     deferredTriggerCheckState(event->dte_item[i].dti_tgoid,
!                                               event->dte_item[i].dti_state))
!                 {
!                     still_deferred_ones = true;
!                     continue;
!                 }

                  /*
!                  * So let's fire it... but first, open the correct relation
!                  * if this is not the same relation as before.
                   */
!                 if (rel == NULL || rel->rd_id != event->dte_relid)
!                 {
!                     if (rel)
!                         heap_close(rel, NoLock);
!                     if (finfo)
!                         pfree(finfo);

!                     /*
!                      * We assume that an appropriate lock is still held by the
!                      * executor, so grab no new lock here.
!                      */
!                     rel = heap_open(event->dte_relid, NoLock);

!                     /*
!                      * Allocate space to cache fmgr lookup info for triggers
!                      * of this relation.
!                      */
!                     finfo = (FmgrInfo *)
!                         palloc(rel->trigdesc->numtriggers * sizeof(FmgrInfo));
!                     MemSet(finfo, 0,
!                            rel->trigdesc->numtriggers * sizeof(FmgrInfo));
!                 }
!
!                 DeferredTriggerExecute(event, i, rel, finfo,
!                                        per_tuple_context);
!
!                 event->dte_item[i].dti_state |= TRIGGER_DEFERRED_DONE;
!             } /* end loop over items within event */
          }

          /*
!          * If it's now completely done, throw it away.
!          *
!          * NB: it's possible the trigger calls above added more events to the
!          * queue, or that calls we will do later will want to add more,
!          * so we have to be careful about maintaining list validity here.
           */
!         next_event = event->dte_next;
!
!         if (still_deferred_ones)
!         {
!             /* Not done, keep in list */
!             prev_event = event;
!         }
!         else
!         {
!             /* Done */
!             if (immediate_only)
!             {
!                 /* delink it from list and free it */
!                 if (prev_event)
!                     prev_event->dte_next = next_event;
!                 else
!                     deftrig_events = next_event;
!                 pfree(event);
!             }
!             else
!             {
!                 /*
!                  * We will clean up later, but just for paranoia's sake,
!                  * mark the event done.
!                  */
!                 event->dte_event |= TRIGGER_DEFERRED_DONE;
!             }
!         }
!
!         event = next_event;
      }

+     /* Update list tail pointer in case we just deleted tail event */
+     deftrig_event_tail = prev_event;
+
+     /* Release working resources */
      if (rel)
          heap_close(rel, NoLock);
      if (finfo)
***************
*** 1649,1655 ****

      MemoryContextSwitchTo(oldcxt);

-     deftrig_n_events = 0;
      deftrig_events = NULL;
      deftrig_event_tail = NULL;
  }
--- 1648,1653 ----
***************
*** 1986,1994 ****
   *    Called by ExecAR...Triggers() to add the event to the queue.
   *
   *    NOTE: should be called only if we've determined that an event must
!  *    be added to the queue.    We must save *all* events if there is either
!  *    an UPDATE or a DELETE deferred trigger; see uses of
!  *    deferredTriggerGetPreviousEvent.
   * ----------
   */
  static void
--- 1984,1990 ----
   *    Called by ExecAR...Triggers() to add the event to the queue.
   *
   *    NOTE: should be called only if we've determined that an event must
!  *    be added to the queue.
   * ----------
   */
  static void
***************
*** 1999,2005 ****
      TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
      MemoryContext oldcxt;
      DeferredTriggerEvent new_event;
-     DeferredTriggerEvent prev_event;
      int            new_size;
      int            i;
      int            ntriggers;
--- 1995,2000 ----
***************
*** 2060,2085 ****
      switch (event & TRIGGER_EVENT_OPMASK)
      {
          case TRIGGER_EVENT_INSERT:
!             new_event->dte_event |= TRIGGER_DEFERRED_ROW_INSERTED;
!             new_event->dte_event |= TRIGGER_DEFERRED_KEY_CHANGED;
              break;

          case TRIGGER_EVENT_UPDATE:
-
              /*
!              * On UPDATE check if the tuple updated has been inserted or a
!              * foreign referenced key value that's changing now has been
!              * updated once before in this transaction.
!              */
!             if (!TransactionIdEquals(oldtup->t_data->t_xmin,
!                                      GetCurrentTransactionId()))
!                 prev_event = NULL;
!             else
!                 prev_event =
!                     deferredTriggerGetPreviousEvent(rel->rd_id, &oldctid);
!
!             /*
!              * Now check if one of the referenced keys is changed.
               */
              for (i = 0; i < ntriggers; i++)
              {
--- 2055,2066 ----
      switch (event & TRIGGER_EVENT_OPMASK)
      {
          case TRIGGER_EVENT_INSERT:
!             /* nothing to do */
              break;

          case TRIGGER_EVENT_UPDATE:
              /*
!              * Check if one of the referenced keys is changed.
               */
              for (i = 0; i < ntriggers; i++)
              {
***************
*** 2120,2228 ****
                  {
                      /*
                       * The key hasn't changed, so no need later to invoke
!                      * the trigger at all. But remember other states from
!                      * the possible earlier event.
                       */
                      new_event->dte_item[i].dti_state |= TRIGGER_DEFERRED_DONE;
-
-                     if (prev_event)
-                     {
-                         if (prev_event->dte_event &
-                             TRIGGER_DEFERRED_ROW_INSERTED)
-                         {
-                             /*
-                              * This is a row inserted during our
-                              * transaction. So any key value is considered
-                              * changed.
-                              */
-                             new_event->dte_event |=
-                                 TRIGGER_DEFERRED_ROW_INSERTED;
-                             new_event->dte_event |=
-                                 TRIGGER_DEFERRED_KEY_CHANGED;
-                             new_event->dte_item[i].dti_state |=
-                                 TRIGGER_DEFERRED_KEY_CHANGED;
-                         }
-                         else
-                         {
-                             /*
-                              * This is a row, previously updated. So if
-                              * this key has been changed before, we still
-                              * remember that it happened.
-                              */
-                             if (prev_event->dte_item[i].dti_state &
-                                 TRIGGER_DEFERRED_KEY_CHANGED)
-                             {
-                                 new_event->dte_item[i].dti_state |=
-                                     TRIGGER_DEFERRED_KEY_CHANGED;
-                                 new_event->dte_event |=
-                                     TRIGGER_DEFERRED_KEY_CHANGED;
-                             }
-                         }
-                     }
-                 }
-                 else
-                 {
-                     /*
-                      * Bomb out if this key has been changed before.
-                      * Otherwise remember that we do so.
-                      */
-                     if (prev_event)
-                     {
-                         if (prev_event->dte_event &
-                             TRIGGER_DEFERRED_ROW_INSERTED)
-                             elog(ERROR, "triggered data change violation "
-                                  "on relation \"%s\"",
-                              DatumGetCString(DirectFunctionCall1(nameout,
-                                 NameGetDatum(&(rel->rd_rel->relname)))));
-
-                         if (prev_event->dte_item[i].dti_state &
-                             TRIGGER_DEFERRED_KEY_CHANGED)
-                             elog(ERROR, "triggered data change violation "
-                                  "on relation \"%s\"",
-                              DatumGetCString(DirectFunctionCall1(nameout,
-                                 NameGetDatum(&(rel->rd_rel->relname)))));
-                     }
-
-                     /*
-                      * This is the first change to this key, so let it
-                      * happen.
-                      */
-                     new_event->dte_item[i].dti_state |=
-                         TRIGGER_DEFERRED_KEY_CHANGED;
-                     new_event->dte_event |= TRIGGER_DEFERRED_KEY_CHANGED;
                  }
              }

              break;

          case TRIGGER_EVENT_DELETE:
!
!             /*
!              * On DELETE check if the tuple deleted has been inserted or a
!              * possibly referenced key value has changed in this
!              * transaction.
!              */
!             if (!TransactionIdEquals(oldtup->t_data->t_xmin,
!                                      GetCurrentTransactionId()))
!                 break;
!
!             /*
!              * Look at the previous event to the same tuple.
!              */
!             prev_event = deferredTriggerGetPreviousEvent(rel->rd_id, &oldctid);
!             if (prev_event->dte_event & TRIGGER_DEFERRED_KEY_CHANGED)
!                 elog(ERROR, "triggered data change violation "
!                      "on relation \"%s\"",
!                      DatumGetCString(DirectFunctionCall1(nameout,
!                                 NameGetDatum(&(rel->rd_rel->relname)))));
!
              break;
      }

      /*
!      * Anything's fine up to here. Add the new event to the queue.
       */
-     oldcxt = MemoryContextSwitchTo(deftrig_cxt);
      deferredTriggerAddEvent(new_event);
-     MemoryContextSwitchTo(oldcxt);
  }
--- 2101,2121 ----
                  {
                      /*
                       * The key hasn't changed, so no need later to invoke
!                      * the trigger at all.
                       */
                      new_event->dte_item[i].dti_state |= TRIGGER_DEFERRED_DONE;
                  }
              }

              break;

          case TRIGGER_EVENT_DELETE:
!             /* nothing to do */
              break;
      }

      /*
!      * Add the new event to the queue.
       */
      deferredTriggerAddEvent(new_event);
  }
*** src/include/commands/trigger.h.orig    Sun Nov 11 19:46:36 2001
--- src/include/commands/trigger.h    Mon Nov 12 18:11:31 2001
***************
*** 50,58 ****
  #define TRIGGER_DEFERRED_DEFERRABLE        0x00000040
  #define TRIGGER_DEFERRED_INITDEFERRED    0x00000080
  #define TRIGGER_DEFERRED_HAS_BEFORE        0x00000100
- #define TRIGGER_DEFERRED_ROW_INSERTED    0x00000200
- #define TRIGGER_DEFERRED_KEY_CHANGED    0x00000400
- #define TRIGGER_DEFERRED_MASK            0x000007F0

  #define TRIGGER_FIRED_BY_INSERT(event)    \
          (((TriggerEvent) (event) & TRIGGER_EVENT_OPMASK) == \
--- 50,55 ----

pgsql-patches by date:

Previous
From: Hiroshi Inoue
Date:
Subject: Re: ALTER TABLE RENAME fix
Next
From: Hiroshi Inoue
Date:
Subject: Re: [ODBC] MD5 support for ODBC