I found a corner case in which it is possible for RI_FKey_check's call

of HeapTupleSatisfiesItself() to trigger a hint-bit update on the tuple:
if the row was updated or deleted by a subtransaction of my own transaction
that was later rolled back.  This cannot occur in pre-8.0 of course, so
the hint-bit patch applied a couple weeks ago is OK for existing releases.
But for 8.0 it seems we had better fix things so that RI_FKey_check can
pass the correct buffer number to HeapTupleSatisfiesItself.  Accordingly,
add fields to the TriggerData struct to carry the buffer ID(s) for the
old and new tuple(s).  There are other possible solutions but this one
seems cleanest; it will allow other AFTER-trigger functions to safely
do tqual.c calls if they want to.  Put new fields at end of struct so
that there is no API breakage.

diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml
index 56dfec80e6cd9715ff3e4f8479fd6863524d70a5..942eb0c128a47bd747117a92caeb753b20608445 100644 (file)
--- a/doc/src/sgml/trigger.sgml
+++ b/doc/src/sgml/trigger.sgml
@@ -1,5 +1,5 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/trigger.sgml,v 1.36 2004/08/13 16:17:19 tgl Exp $
+$PostgreSQL: pgsql/doc/src/sgml/trigger.sgml,v 1.37 2004/10/30 20:52:46 tgl Exp $
 -->
 
  <chapter id="triggers">
@@ -256,6 +256,8 @@ typedef struct TriggerData
     HeapTuple     tg_trigtuple;
     HeapTuple     tg_newtuple;
     Trigger      *tg_trigger;
+    Buffer        tg_trigtuplebuf;
+    Buffer        tg_newtuplebuf;
 } TriggerData;
 </programlisting>
 
@@ -427,6 +429,27 @@ typedef struct Trigger
        </para>
       </listitem>
      </varlistentry>
+
+     <varlistentry>
+      <term><structfield>tg_trigtuplebuf</></term>
+      <listitem>
+       <para>
+        The buffer containing tg_trigtuple, or InvalidBuffer if there
+        is no such tuple or it is not stored in a disk buffer.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><structfield>tg_newtuplebuf</></term>
+      <listitem>
+       <para>
+        The buffer containing tg_newtuple, or InvalidBuffer if there
+        is no such tuple or it is not stored in a disk buffer.
+       </para>
+      </listitem>
+     </varlistentry>
+
     </variablelist>
    </para>
 

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3616275858392c370bdc5457557b48f701d08101..4593327d35d1749787cb2a7da5b7817a35960cda 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.137 2004/10/22 17:20:04 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.138 2004/10/30 20:52:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -4256,6 +4256,8 @@ validateForeignKeyConstraint(FkConstraint *fkconstraint,
                trigdata.tg_trigtuple = tuple;
                trigdata.tg_newtuple = NULL;
                trigdata.tg_trigger = &trig;
+               trigdata.tg_trigtuplebuf = scan->rs_cbuf;
+               trigdata.tg_newtuplebuf = InvalidBuffer;
 
                fcinfo.context = (Node *) &trigdata;
 

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 7b1bdddf0baf3ac316ef01109224b07b26447021..a477fc8469577be4782e95bbce384369fd9b45b1 100644 (file)
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.173 2004/10/21 21:33:59 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.174 2004/10/30 20:52:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1192,8 +1192,10 @@ ExecBSInsertTriggers(EState *estate, ResultRelInfo *relinfo)
        LocTriggerData.tg_event = TRIGGER_EVENT_INSERT |
                TRIGGER_EVENT_BEFORE;
        LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
-       LocTriggerData.tg_newtuple = NULL;
        LocTriggerData.tg_trigtuple = NULL;
+       LocTriggerData.tg_newtuple = NULL;
+       LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
+       LocTriggerData.tg_newtuplebuf = InvalidBuffer;
        for (i = 0; i < ntrigs; i++)
        {
                Trigger    *trigger = &trigdesc->triggers[tgindx[i]];
@@ -1246,6 +1248,7 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
                TRIGGER_EVENT_BEFORE;
        LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
        LocTriggerData.tg_newtuple = NULL;
+       LocTriggerData.tg_newtuplebuf = InvalidBuffer;
        for (i = 0; i < ntrigs; i++)
        {
                Trigger    *trigger = &trigdesc->triggers[tgindx[i]];
@@ -1253,6 +1256,7 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
                if (!trigger->tgenabled)
                        continue;
                LocTriggerData.tg_trigtuple = oldtuple = newtuple;
+               LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
                LocTriggerData.tg_trigger = trigger;
                newtuple = ExecCallTriggerFunc(&LocTriggerData,
                                                                   relinfo->ri_TrigFunctions + tgindx[i],
@@ -1305,8 +1309,10 @@ ExecBSDeleteTriggers(EState *estate, ResultRelInfo *relinfo)
        LocTriggerData.tg_event = TRIGGER_EVENT_DELETE |
                TRIGGER_EVENT_BEFORE;
        LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
-       LocTriggerData.tg_newtuple = NULL;
        LocTriggerData.tg_trigtuple = NULL;
+       LocTriggerData.tg_newtuple = NULL;
+       LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
+       LocTriggerData.tg_newtuplebuf = InvalidBuffer;
        for (i = 0; i < ntrigs; i++)
        {
                Trigger    *trigger = &trigdesc->triggers[tgindx[i]];
@@ -1365,6 +1371,7 @@ ExecBRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
                TRIGGER_EVENT_BEFORE;
        LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
        LocTriggerData.tg_newtuple = NULL;
+       LocTriggerData.tg_newtuplebuf = InvalidBuffer;
        for (i = 0; i < ntrigs; i++)
        {
                Trigger    *trigger = &trigdesc->triggers[tgindx[i]];
@@ -1372,6 +1379,7 @@ ExecBRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
                if (!trigger->tgenabled)
                        continue;
                LocTriggerData.tg_trigtuple = trigtuple;
+               LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
                LocTriggerData.tg_trigger = trigger;
                newtuple = ExecCallTriggerFunc(&LocTriggerData,
                                                                   relinfo->ri_TrigFunctions + tgindx[i],
@@ -1434,8 +1442,10 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
        LocTriggerData.tg_event = TRIGGER_EVENT_UPDATE |
                TRIGGER_EVENT_BEFORE;
        LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
-       LocTriggerData.tg_newtuple = NULL;
        LocTriggerData.tg_trigtuple = NULL;
+       LocTriggerData.tg_newtuple = NULL;
+       LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
+       LocTriggerData.tg_newtuplebuf = InvalidBuffer;
        for (i = 0; i < ntrigs; i++)
        {
                Trigger    *trigger = &trigdesc->triggers[tgindx[i]];
@@ -1509,6 +1519,8 @@ ExecBRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
                        continue;
                LocTriggerData.tg_trigtuple = trigtuple;
                LocTriggerData.tg_newtuple = oldtuple = newtuple;
+               LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
+               LocTriggerData.tg_newtuplebuf = InvalidBuffer;
                LocTriggerData.tg_trigger = trigger;
                newtuple = ExecCallTriggerFunc(&LocTriggerData,
                                                                   relinfo->ri_TrigFunctions + tgindx[i],
@@ -1896,8 +1908,8 @@ AfterTriggerExecute(AfterTriggerEvent event,
        HeapTupleData oldtuple;
        HeapTupleData newtuple;
        HeapTuple       rettuple;
-       Buffer          oldbuffer;
-       Buffer          newbuffer;
+       Buffer          oldbuffer = InvalidBuffer;
+       Buffer          newbuffer = InvalidBuffer;
        int                     tgindx;
 
        /*
@@ -1942,16 +1954,22 @@ AfterTriggerExecute(AfterTriggerEvent event,
                case TRIGGER_EVENT_INSERT:
                        LocTriggerData.tg_trigtuple = &newtuple;
                        LocTriggerData.tg_newtuple = NULL;
+                       LocTriggerData.tg_trigtuplebuf = newbuffer;
+                       LocTriggerData.tg_newtuplebuf = InvalidBuffer;
                        break;
 
                case TRIGGER_EVENT_UPDATE:
                        LocTriggerData.tg_trigtuple = &oldtuple;
                        LocTriggerData.tg_newtuple = &newtuple;
+                       LocTriggerData.tg_trigtuplebuf = oldbuffer;
+                       LocTriggerData.tg_newtuplebuf = newbuffer;
                        break;
 
                case TRIGGER_EVENT_DELETE:
                        LocTriggerData.tg_trigtuple = &oldtuple;
                        LocTriggerData.tg_newtuple = NULL;
+                       LocTriggerData.tg_trigtuplebuf = oldbuffer;
+                       LocTriggerData.tg_newtuplebuf = InvalidBuffer;
                        break;
        }
 
@@ -1970,9 +1988,9 @@ AfterTriggerExecute(AfterTriggerEvent event,
        /*
         * Release buffers
         */
-       if (ItemPointerIsValid(&(event->ate_oldctid)))
+       if (oldbuffer != InvalidBuffer)
                ReleaseBuffer(oldbuffer);
-       if (ItemPointerIsValid(&(event->ate_newctid)))
+       if (newbuffer != InvalidBuffer)
                ReleaseBuffer(newbuffer);
 }
 
@@ -2916,6 +2934,14 @@ AfterTriggerSaveEvent(ResultRelInfo *relinfo, int event, bool row_trigger,
                                LocTriggerData.tg_trigtuple = oldtup;
                                LocTriggerData.tg_newtuple = newtup;
                                LocTriggerData.tg_trigger = trigger;
+                               /*
+                                * We do not currently know which buffers the passed tuples
+                                * are in, but it does not matter because RI_FKey_keyequal_upd
+                                * does not care.  We could expand the API of this function
+                                * if it becomes necessary to set these fields accurately.
+                                */
+                               LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
+                               LocTriggerData.tg_newtuplebuf = InvalidBuffer;
 
                                if (RI_FKey_keyequal_upd(&LocTriggerData))
                                {

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index b195e243393fcf09cf5efc02f9b609d8b9c496ee..a727aee1c0ee9162e7f70403d38b5c396c7081f0 100644 (file)
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -17,7 +17,7 @@
  *
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.74 2004/10/15 22:40:11 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.75 2004/10/30 20:52:59 tgl Exp $
  *
  * ----------
  */
@@ -186,6 +186,7 @@ RI_FKey_check(PG_FUNCTION_ARGS)
        Relation        pk_rel;
        HeapTuple       new_row;
        HeapTuple       old_row;
+       Buffer          new_row_buf;
        RI_QueryKey qkey;
        void       *qplan;
        int                     i;
@@ -213,35 +214,30 @@ RI_FKey_check(PG_FUNCTION_ARGS)
        {
                old_row = trigdata->tg_trigtuple;
                new_row = trigdata->tg_newtuple;
+               new_row_buf = trigdata->tg_newtuplebuf;
        }
        else
        {
                old_row = NULL;
                new_row = trigdata->tg_trigtuple;
+               new_row_buf = trigdata->tg_trigtuplebuf;
        }
 
        /*
         * We should not even consider checking the row if it is no longer
         * valid since it was either deleted (doesn't matter) or updated (in
         * which case it'll be checked with its final values).
-        *
-        * We do not know what buffer the new_row is in, but it doesn't matter
-        * since it's not possible for a hint-bit update to occur here (the
-        * new_row could only contain our own XID, and we haven't yet committed
-        * or aborted...)
         */
-       if (new_row)
+       Assert(new_row_buf != InvalidBuffer);
+       if (!HeapTupleSatisfiesItself(new_row->t_data, new_row_buf))
        {
-               if (!HeapTupleSatisfiesItself(new_row->t_data, InvalidBuffer))
-               {
-                       heap_close(pk_rel, RowShareLock);
-                       return PointerGetDatum(NULL);
-               }
+               heap_close(pk_rel, RowShareLock);
+               return PointerGetDatum(NULL);
        }
 
        /* ----------
         * SQL3 11.9 <referential constraint definition>
-        *      Gereral rules 2) a):
+        *      General rules 2) a):
         *              If Rf and Rt are empty (no columns to compare given)
         *              constraint is true if 0 < (SELECT COUNT(*) FROM T)
         *

diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 7d5569018ae3eac0db6da173bf8e0b714455b72e..a271a2a3eaaad731bb2b8b48d64fc82ad5a56eab 100644 (file)
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.49 2004/09/10 18:40:06 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.50 2004/10/30 20:53:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -34,6 +34,8 @@ typedef struct TriggerData
        HeapTuple       tg_trigtuple;
        HeapTuple       tg_newtuple;
        Trigger    *tg_trigger;
+       Buffer          tg_trigtuplebuf;
+       Buffer          tg_newtuplebuf;
 } TriggerData;
 
 /* TriggerEvent bit flags */