Clean up memory-context stuff, other minor infelicities.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 3 Jul 2000 03:57:03 +0000 (03:57 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 3 Jul 2000 03:57:03 +0000 (03:57 +0000)
src/backend/commands/trigger.c

index 075f3a508b5520143ad4d9031dfa773b4e59ba48..36b2019bd9931fcf13707df57356d99d6f7220b1 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.71 2000/06/30 07:04:17 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.72 2000/07/03 03:57:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -37,7 +37,6 @@ static HeapTuple GetTupleForTrigger(EState *estate, ItemPointer tid,
                   TupleTableSlot **newSlot);
 static HeapTuple ExecCallTriggerFunc(Trigger *trigger,
                                     TriggerData *trigdata);
-
 static void DeferredTriggerSaveEvent(Relation rel, int event,
                         HeapTuple oldtup, HeapTuple newtup);
 
@@ -58,12 +57,13 @@ CreateTrigger(CreateTrigStmt *stmt)
    Relation    idescs[Num_pg_trigger_indices];
    Relation    ridescs[Num_pg_class_indices];
    Oid         fargtypes[FUNC_MAX_ARGS];
+   Oid         funcoid;
    Oid         funclang;
    int         found = 0;
    int         i;
    char        constrtrigname[NAMEDATALEN];
    char       *constrname = "";
-   Oid         constrrelid = 0;
+   Oid         constrrelid = InvalidOid;
 
    if (!allowSystemTableMods && IsSystemRelationName(stmt->relname))
        elog(ERROR, "CreateTrigger: can't create trigger for system relation %s", stmt->relname);
@@ -82,12 +82,15 @@ CreateTrigger(CreateTrigStmt *stmt)
    {
        constrname = stmt->trigname;
        stmt->trigname = constrtrigname;
-       sprintf(constrtrigname, "RI_ConstraintTrigger_%d", newoid());
+       sprintf(constrtrigname, "RI_ConstraintTrigger_%u", newoid());
 
        if (strcmp(stmt->constrrelname, "") == 0)
-           constrrelid = 0;
+           constrrelid = InvalidOid;
        else
        {
+           /* NoLock is probably sufficient here, since we're only
+            * interested in getting the relation's OID...
+            */
            rel = heap_openr(stmt->constrrelname, NoLock);
            if (rel == NULL)
                elog(ERROR, "table \"%s\" does not exist",
@@ -132,7 +135,11 @@ CreateTrigger(CreateTrigStmt *stmt)
        }
    }
 
-   /* Scan pg_trigger */
+   /*
+    * Scan pg_trigger for existing triggers on relation.  NOTE that this
+    * is cool only because we have AccessExclusiveLock on the relation,
+    * so the trigger set won't be changing underneath us.
+    */
    tgrel = heap_openr(TriggerRelationName, RowExclusiveLock);
    ScanKeyEntryInitialize(&key, 0, Anum_pg_trigger_tgrelid,
                           F_OIDEQ, RelationGetRelid(rel));
@@ -144,11 +151,13 @@ CreateTrigger(CreateTrigStmt *stmt)
        if (namestrcmp(&(pg_trigger->tgname), stmt->trigname) == 0)
            elog(ERROR, "CreateTrigger: trigger %s already defined on relation %s",
                 stmt->trigname, stmt->relname);
-       else
-           found++;
+       found++;
    }
    heap_endscan(tgscan);
 
+   /*
+    * Find and validate the trigger function.
+    */
    MemSet(fargtypes, 0, FUNC_MAX_ARGS * sizeof(Oid));
    tuple = SearchSysCacheTuple(PROCNAME,
                                PointerGetDatum(stmt->funcname),
@@ -161,6 +170,7 @@ CreateTrigger(CreateTrigStmt *stmt)
    if (((Form_pg_proc) GETSTRUCT(tuple))->prorettype != 0)
        elog(ERROR, "CreateTrigger: function %s() must return OPAQUE",
             stmt->funcname);
+   funcoid = tuple->t_data->t_oid;
    funclang = ((Form_pg_proc) GETSTRUCT(tuple))->prolang;
    if (funclang != ClanguageId &&
        funclang != NEWClanguageId &&
@@ -179,19 +189,21 @@ CreateTrigger(CreateTrigStmt *stmt)
            elog(ERROR, "CreateTrigger: only builtin, C and PL functions are supported");
    }
 
+   /*
+    * Build the new pg_trigger tuple.
+    */
    MemSet(nulls, ' ', Natts_pg_trigger * sizeof(char));
 
    values[Anum_pg_trigger_tgrelid - 1] = ObjectIdGetDatum(RelationGetRelid(rel));
    values[Anum_pg_trigger_tgname - 1] = NameGetDatum(namein(stmt->trigname));
-   values[Anum_pg_trigger_tgfoid - 1] = ObjectIdGetDatum(tuple->t_data->t_oid);
+   values[Anum_pg_trigger_tgfoid - 1] = ObjectIdGetDatum(funcoid);
    values[Anum_pg_trigger_tgtype - 1] = Int16GetDatum(tgtype);
-
-   values[Anum_pg_trigger_tgenabled - 1] = true;
-   values[Anum_pg_trigger_tgisconstraint - 1] = stmt->isconstraint;
-   values[Anum_pg_trigger_tgconstrname - 1] = PointerGetDatum(constrname);;
-   values[Anum_pg_trigger_tgconstrrelid - 1] = constrrelid;
-   values[Anum_pg_trigger_tgdeferrable - 1] = stmt->deferrable;
-   values[Anum_pg_trigger_tginitdeferred - 1] = stmt->initdeferred;
+   values[Anum_pg_trigger_tgenabled - 1] = BoolGetDatum(true);
+   values[Anum_pg_trigger_tgisconstraint - 1] = BoolGetDatum(stmt->isconstraint);
+   values[Anum_pg_trigger_tgconstrname - 1] = PointerGetDatum(constrname);
+   values[Anum_pg_trigger_tgconstrrelid - 1] = ObjectIdGetDatum(constrrelid);
+   values[Anum_pg_trigger_tgdeferrable - 1] = BoolGetDatum(stmt->deferrable);
+   values[Anum_pg_trigger_tginitdeferred - 1] = BoolGetDatum(stmt->initdeferred);
 
    if (stmt->args)
    {
@@ -204,7 +216,7 @@ CreateTrigger(CreateTrigStmt *stmt)
        {
            char       *ar = (char *) lfirst(le);
 
-           len += strlen(ar) + VARHDRSZ;
+           len += strlen(ar) + 4;
            for (; *ar; ar++)
            {
                if (*ar == '\\')
@@ -239,6 +251,10 @@ CreateTrigger(CreateTrigStmt *stmt)
    values[Anum_pg_trigger_tgattr - 1] = PointerGetDatum(tgattr);
 
    tuple = heap_formtuple(tgrel->rd_att, values, nulls);
+
+   /*
+    * Insert tuple into pg_trigger.
+    */
    heap_insert(tgrel, tuple);
    CatalogOpenIndices(Num_pg_trigger_indices, Name_pg_trigger_indices, idescs);
    CatalogIndexInsert(idescs, Num_pg_trigger_indices, tgrel, tuple);
@@ -249,16 +265,20 @@ CreateTrigger(CreateTrigStmt *stmt)
    pfree(DatumGetPointer(values[Anum_pg_trigger_tgname - 1]));
    pfree(DatumGetPointer(values[Anum_pg_trigger_tgargs - 1]));
 
-   /* update pg_class */
+   /*
+    * Update relation's pg_class entry.  Crucial side-effect: other
+    * backends (and this one too!) are sent SI message to make them
+    * rebuild relcache entries.
+    */
    pgrel = heap_openr(RelationRelationName, RowExclusiveLock);
    tuple = SearchSysCacheTupleCopy(RELNAME,
                                    PointerGetDatum(stmt->relname),
                                    0, 0, 0);
    if (!HeapTupleIsValid(tuple))
-       elog(ERROR, "CreateTrigger: relation %s not found in pg_class", stmt->relname);
+       elog(ERROR, "CreateTrigger: relation %s not found in pg_class",
+            stmt->relname);
 
    ((Form_pg_class) GETSTRUCT(tuple))->reltriggers = found + 1;
-   RelationInvalidateHeapTuple(pgrel, tuple);
    heap_update(pgrel, &tuple->t_self, tuple, NULL);
    CatalogOpenIndices(Num_pg_class_indices, Name_pg_class_indices, ridescs);
    CatalogIndexInsert(ridescs, Num_pg_class_indices, pgrel, tuple);
@@ -271,6 +291,7 @@ CreateTrigger(CreateTrigStmt *stmt)
     * fairly pointless since it will happen as a byproduct of the
     * upcoming CommandCounterIncrement...
     */
+
    /* Keep lock on target rel until end of xact */
    heap_close(rel, NoLock);
 }
@@ -295,6 +316,12 @@ DropTrigger(DropTrigStmt *stmt)
 
    rel = heap_openr(stmt->relname, AccessExclusiveLock);
 
+   /*
+    * Search pg_trigger, delete target trigger, count remaining triggers
+    * for relation.  Note this is OK only because we have
+    * AccessExclusiveLock on the rel, so no one else is creating/deleting
+    * triggers on this rel at the same time.
+    */
    tgrel = heap_openr(TriggerRelationName, RowExclusiveLock);
    ScanKeyEntryInitialize(&key, 0, Anum_pg_trigger_tgrelid,
                           F_OIDEQ, RelationGetRelid(rel));
@@ -312,7 +339,6 @@ DropTrigger(DropTrigStmt *stmt)
 
            heap_delete(tgrel, &tuple->t_self, NULL);
            tgfound++;
-
        }
        else
            found++;
@@ -326,16 +352,20 @@ DropTrigger(DropTrigStmt *stmt)
    heap_endscan(tgscan);
    heap_close(tgrel, RowExclusiveLock);
 
-   /* update pg_class */
+   /*
+    * Update relation's pg_class entry.  Crucial side-effect: other
+    * backends (and this one too!) are sent SI message to make them
+    * rebuild relcache entries.
+    */
    pgrel = heap_openr(RelationRelationName, RowExclusiveLock);
    tuple = SearchSysCacheTupleCopy(RELNAME,
                                    PointerGetDatum(stmt->relname),
                                    0, 0, 0);
    if (!HeapTupleIsValid(tuple))
-       elog(ERROR, "DropTrigger: relation %s not found in pg_class", stmt->relname);
+       elog(ERROR, "DropTrigger: relation %s not found in pg_class",
+            stmt->relname);
 
    ((Form_pg_class) GETSTRUCT(tuple))->reltriggers = found;
-   RelationInvalidateHeapTuple(pgrel, tuple);
    heap_update(pgrel, &tuple->t_self, tuple, NULL);
    CatalogOpenIndices(Num_pg_class_indices, Name_pg_class_indices, ridescs);
    CatalogIndexInsert(ridescs, Num_pg_class_indices, pgrel, tuple);
@@ -348,10 +378,14 @@ DropTrigger(DropTrigStmt *stmt)
     * fairly pointless since it will happen as a byproduct of the
     * upcoming CommandCounterIncrement...
     */
+
    /* Keep lock on target rel until end of xact */
    heap_close(rel, NoLock);
 }
 
+/*
+ * Remove all triggers for a relation that's being deleted.
+ */
 void
 RelationRemoveTriggers(Relation rel)
 {
@@ -374,7 +408,6 @@ RelationRemoveTriggers(Relation rel)
        DeleteComments(tup->t_data->t_oid);
 
        heap_delete(tgrel, &tup->t_self, NULL);
-
    }
 
    heap_endscan(tgscan);
@@ -1141,6 +1174,7 @@ deferredTriggerCheckState(Oid tgoid, int32 itemstate)
  * deferredTriggerAddEvent()
  *
  * Add a new trigger event to the queue.
+ * Caller must have switched into appropriate memory context!
  * ----------
  */
 static void
@@ -1148,8 +1182,6 @@ deferredTriggerAddEvent(DeferredTriggerEvent event)
 {
    deftrig_events = lappend(deftrig_events, event);
    deftrig_n_events++;
-
-   return;
 }
 
 
@@ -1288,8 +1320,6 @@ deferredTriggerExecute(DeferredTriggerEvent event, int itemno)
        ReleaseBuffer(newbuffer);
 
    heap_close(rel, NoLock);
-
-   return;
 }
 
 
@@ -1410,8 +1440,8 @@ DeferredTriggerBeginXact(void)
    DeferredTriggerStatus stat;
 
    if (deftrig_cxt != NULL)
-       elog(FATAL,
-          "DeferredTriggerBeginXact() called while inside transaction");
+       elog(ERROR,
+            "DeferredTriggerBeginXact() called while inside transaction");
 
    /* ----------
     * Create the per transaction memory context and copy all states
@@ -1529,7 +1559,6 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
    Relation    irel = (Relation) NULL;
    List       *l;
    List       *ls;
-   List       *lnext;
    List       *loid = NIL;
    MemoryContext oldcxt;
    bool        found;
@@ -1546,11 +1575,7 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
        {
            /* ----------
             * ... outside of a transaction block
-            * ----------
-            */
-           oldcxt = MemoryContextSwitchTo(deftrig_gcxt);
-
-           /* ----------
+            *
             * Drop all information about individual trigger states per
             * session.
             * ----------
@@ -1558,10 +1583,11 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
            l = deftrig_dfl_trigstates;
            while (l != NIL)
            {
-               lnext = lnext(l);
+               List   *next = lnext(l);
+
                pfree(lfirst(l));
                pfree(l);
-               l = lnext;
+               l = next;
            }
            deftrig_dfl_trigstates = NIL;
 
@@ -1572,19 +1598,13 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
            deftrig_dfl_all_isset = true;
            deftrig_dfl_all_isdeferred = stmt->deferred;
 
-           MemoryContextSwitchTo(oldcxt);
-
            return;
        }
        else
        {
            /* ----------
             * ... inside of a transaction block
-            * ----------
-            */
-           oldcxt = MemoryContextSwitchTo(deftrig_cxt);
-
-           /* ----------
+            *
             * Drop all information about individual trigger states per
             * transaction.
             * ----------
@@ -1592,10 +1612,11 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
            l = deftrig_trigstates;
            while (l != NIL)
            {
-               lnext = lnext(l);
+               List   *next = lnext(l);
+
                pfree(lfirst(l));
                pfree(l);
-               l = lnext;
+               l = next;
            }
            deftrig_trigstates = NIL;
 
@@ -1606,8 +1627,6 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
            deftrig_all_isset = true;
            deftrig_all_isdeferred = stmt->deferred;
 
-           MemoryContextSwitchTo(oldcxt);
-
            return;
        }
    }
@@ -1697,7 +1716,7 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
                     (char *) lfirst(l));
 
            constr_oid = htup->t_data->t_oid;
-           loid = lappend(loid, (Node *) constr_oid);
+           loid = lappendi(loid, constr_oid);
            found = true;
 
            if (hasindex)
@@ -1720,7 +1739,6 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
        index_close(irel);
    heap_close(tgrel, AccessShareLock);
 
-
    if (!IsTransactionBlock())
    {
        /* ----------
@@ -1736,7 +1754,7 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
            foreach(ls, deftrig_dfl_trigstates)
            {
                state = (DeferredTriggerStatus) lfirst(ls);
-               if (state->dts_tgoid == (Oid) lfirst(l))
+               if (state->dts_tgoid == (Oid) lfirsti(l))
                {
                    state->dts_tgisdeferred = stmt->deferred;
                    found = true;
@@ -1747,7 +1765,7 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
            {
                state = (DeferredTriggerStatus)
                    palloc(sizeof(DeferredTriggerStatusData));
-               state->dts_tgoid = (Oid) lfirst(l);
+               state->dts_tgoid = (Oid) lfirsti(l);
                state->dts_tgisdeferred = stmt->deferred;
 
                deftrig_dfl_trigstates =
@@ -1774,7 +1792,7 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
            foreach(ls, deftrig_trigstates)
            {
                state = (DeferredTriggerStatus) lfirst(ls);
-               if (state->dts_tgoid == (Oid) lfirst(l))
+               if (state->dts_tgoid == (Oid) lfirsti(l))
                {
                    state->dts_tgisdeferred = stmt->deferred;
                    found = true;
@@ -1785,7 +1803,7 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
            {
                state = (DeferredTriggerStatus)
                    palloc(sizeof(DeferredTriggerStatusData));
-               state->dts_tgoid = (Oid) lfirst(l);
+               state->dts_tgoid = (Oid) lfirsti(l);
                state->dts_tgisdeferred = stmt->deferred;
 
                deftrig_trigstates =
@@ -2052,6 +2070,4 @@ DeferredTriggerSaveEvent(Relation rel, int event,
    oldcxt = MemoryContextSwitchTo(deftrig_cxt);
    deferredTriggerAddEvent(new_event);
    MemoryContextSwitchTo(oldcxt);
-
-   return;
 }