diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index b4de718eae993228d8c436c590572dc3fa527313..2bf0c0958094adf5bad0a056e964df51cab39952 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.175 2002/09/20 16:56:02 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.176 2002/10/14 16:51:28 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -780,7 +780,7 @@ CopyFrom(Relation rel, List *attnumlist, bool binary, bool oids, resultRelInfo = makeNode(ResultRelInfo); resultRelInfo->ri_RangeTableIndex = 1; /* dummy */ resultRelInfo->ri_RelationDesc = rel; - resultRelInfo->ri_TrigDesc = rel->trigdesc; + resultRelInfo->ri_TrigDesc = CopyTriggerDesc(rel->trigdesc); ExecOpenIndices(resultRelInfo); diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 4af8a9f9cdc3b6ea3d0549d449f6a7d6e9b8457b..b404bc3dc5157300b63ee8535a3ff717f6daa102 100644 --- 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 - * $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.134 2002/10/03 21:06:23 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.135 2002/10/14 16:51:29 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -49,7 +49,7 @@ static HeapTuple ExecCallTriggerFunc(TriggerData *trigdata, static void DeferredTriggerSaveEvent(ResultRelInfo *relinfo, int event, HeapTuple oldtup, HeapTuple newtup); static void DeferredTriggerExecute(DeferredTriggerEvent event, int itemno, - Relation rel, FmgrInfo *finfo, + Relation rel, TriggerDesc *trigdesc, FmgrInfo *finfo, MemoryContext per_tuple_context); @@ -680,9 +680,12 @@ renametrig(Oid relid, /* * Build trigger data to attach to the given relcache entry. * - * Note that trigger data must be allocated in CacheMemoryContext - * to ensure it survives as long as the relcache entry. But we - * are probably running in a less long-lived working context. + * Note that trigger data attached to a relcache entry must be stored in + * CacheMemoryContext to ensure it survives as long as the relcache entry. + * But we should be running in a less long-lived working context. To avoid + * leaking cache memory if this routine fails partway through, we build a + * temporary TriggerDesc in working memory and then copy the completed + * structure into cache memory. */ void RelationBuildTriggers(Relation relation) @@ -695,9 +698,11 @@ RelationBuildTriggers(Relation relation) ScanKeyData skey; SysScanDesc tgscan; HeapTuple htup; + MemoryContext oldContext; + + Assert(ntrigs > 0); /* else I should not have been called */ - triggers = (Trigger *) MemoryContextAlloc(CacheMemoryContext, - ntrigs * sizeof(Trigger)); + triggers = (Trigger *) palloc(ntrigs * sizeof(Trigger)); /* * Note: since we scan the triggers using TriggerRelidNameIndex, we @@ -726,9 +731,8 @@ RelationBuildTriggers(Relation relation) build = &(triggers[found]); build->tgoid = HeapTupleGetOid(htup); - build->tgname = MemoryContextStrdup(CacheMemoryContext, - DatumGetCString(DirectFunctionCall1(nameout, - NameGetDatum(&pg_trigger->tgname)))); + build->tgname = DatumGetCString(DirectFunctionCall1(nameout, + NameGetDatum(&pg_trigger->tgname))); build->tgfoid = pg_trigger->tgfoid; build->tgtype = pg_trigger->tgtype; build->tgenabled = pg_trigger->tgenabled; @@ -753,13 +757,10 @@ RelationBuildTriggers(Relation relation) elog(ERROR, "RelationBuildTriggers: tgargs IS NULL for rel %s", RelationGetRelationName(relation)); p = (char *) VARDATA(val); - build->tgargs = (char **) - MemoryContextAlloc(CacheMemoryContext, - build->tgnargs * sizeof(char *)); + build->tgargs = (char **) palloc(build->tgnargs * sizeof(char *)); for (i = 0; i < build->tgnargs; i++) { - build->tgargs[i] = MemoryContextStrdup(CacheMemoryContext, - p); + build->tgargs[i] = pstrdup(p); p += strlen(p) + 1; } } @@ -778,18 +779,30 @@ RelationBuildTriggers(Relation relation) RelationGetRelationName(relation)); /* Build trigdesc */ - trigdesc = (TriggerDesc *) MemoryContextAlloc(CacheMemoryContext, - sizeof(TriggerDesc)); + trigdesc = (TriggerDesc *) palloc(sizeof(TriggerDesc)); MemSet(trigdesc, 0, sizeof(TriggerDesc)); trigdesc->triggers = triggers; trigdesc->numtriggers = ntrigs; for (found = 0; found < ntrigs; found++) InsertTrigger(trigdesc, &(triggers[found]), found); - relation->trigdesc = trigdesc; + /* Copy completed trigdesc into cache storage */ + oldContext = MemoryContextSwitchTo(CacheMemoryContext); + relation->trigdesc = CopyTriggerDesc(trigdesc); + MemoryContextSwitchTo(oldContext); + + /* Release working memory */ + FreeTriggerDesc(trigdesc); } -/* Insert the given trigger into the appropriate index list(s) for it */ +/* + * Insert the given trigger into the appropriate index list(s) for it + * + * To simplify storage management, we allocate each index list at the max + * possible size (trigdesc->numtriggers) if it's used at all. This does + * not waste space permanently since we're only building a temporary + * trigdesc at this point. + */ static void InsertTrigger(TriggerDesc *trigdesc, Trigger *trigger, int indx) { @@ -830,11 +843,7 @@ InsertTrigger(TriggerDesc *trigdesc, Trigger *trigger, int indx) { tp = &(t[TRIGGER_EVENT_INSERT]); if (*tp == NULL) - *tp = (int *) MemoryContextAlloc(CacheMemoryContext, - sizeof(int)); - else - *tp = (int *) repalloc(*tp, (n[TRIGGER_EVENT_INSERT] + 1) * - sizeof(int)); + *tp = (int *) palloc(trigdesc->numtriggers * sizeof(int)); (*tp)[n[TRIGGER_EVENT_INSERT]] = indx; (n[TRIGGER_EVENT_INSERT])++; } @@ -843,11 +852,7 @@ InsertTrigger(TriggerDesc *trigdesc, Trigger *trigger, int indx) { tp = &(t[TRIGGER_EVENT_DELETE]); if (*tp == NULL) - *tp = (int *) MemoryContextAlloc(CacheMemoryContext, - sizeof(int)); - else - *tp = (int *) repalloc(*tp, (n[TRIGGER_EVENT_DELETE] + 1) * - sizeof(int)); + *tp = (int *) palloc(trigdesc->numtriggers * sizeof(int)); (*tp)[n[TRIGGER_EVENT_DELETE]] = indx; (n[TRIGGER_EVENT_DELETE])++; } @@ -856,16 +861,113 @@ InsertTrigger(TriggerDesc *trigdesc, Trigger *trigger, int indx) { tp = &(t[TRIGGER_EVENT_UPDATE]); if (*tp == NULL) - *tp = (int *) MemoryContextAlloc(CacheMemoryContext, - sizeof(int)); - else - *tp = (int *) repalloc(*tp, (n[TRIGGER_EVENT_UPDATE] + 1) * - sizeof(int)); + *tp = (int *) palloc(trigdesc->numtriggers * sizeof(int)); (*tp)[n[TRIGGER_EVENT_UPDATE]] = indx; (n[TRIGGER_EVENT_UPDATE])++; } } +/* + * Copy a TriggerDesc data structure. + * + * The copy is allocated in the current memory context. + */ +TriggerDesc * +CopyTriggerDesc(TriggerDesc *trigdesc) +{ + TriggerDesc *newdesc; + uint16 *n; + int **t, + *tnew; + Trigger *trigger; + int i; + + if (trigdesc == NULL || trigdesc->numtriggers <= 0) + return NULL; + + newdesc = (TriggerDesc *) palloc(sizeof(TriggerDesc)); + memcpy(newdesc, trigdesc, sizeof(TriggerDesc)); + + trigger = (Trigger *) palloc(trigdesc->numtriggers * sizeof(Trigger)); + memcpy(trigger, trigdesc->triggers, + trigdesc->numtriggers * sizeof(Trigger)); + newdesc->triggers = trigger; + + for (i = 0; i < trigdesc->numtriggers; i++) + { + trigger->tgname = pstrdup(trigger->tgname); + if (trigger->tgnargs > 0) + { + char **newargs; + int16 j; + + newargs = (char **) palloc(trigger->tgnargs * sizeof(char *)); + for (j = 0; j < trigger->tgnargs; j++) + newargs[j] = pstrdup(trigger->tgargs[j]); + trigger->tgargs = newargs; + } + trigger++; + } + + n = newdesc->n_before_statement; + t = newdesc->tg_before_statement; + for (i = 0; i < TRIGGER_NUM_EVENT_CLASSES; i++) + { + if (n[i] > 0) + { + tnew = (int *) palloc(n[i] * sizeof(int)); + memcpy(tnew, t[i], n[i] * sizeof(int)); + t[i] = tnew; + } + else + t[i] = NULL; + } + n = newdesc->n_before_row; + t = newdesc->tg_before_row; + for (i = 0; i < TRIGGER_NUM_EVENT_CLASSES; i++) + { + if (n[i] > 0) + { + tnew = (int *) palloc(n[i] * sizeof(int)); + memcpy(tnew, t[i], n[i] * sizeof(int)); + t[i] = tnew; + } + else + t[i] = NULL; + } + n = newdesc->n_after_row; + t = newdesc->tg_after_row; + for (i = 0; i < TRIGGER_NUM_EVENT_CLASSES; i++) + { + if (n[i] > 0) + { + tnew = (int *) palloc(n[i] * sizeof(int)); + memcpy(tnew, t[i], n[i] * sizeof(int)); + t[i] = tnew; + } + else + t[i] = NULL; + } + n = newdesc->n_after_statement; + t = newdesc->tg_after_statement; + for (i = 0; i < TRIGGER_NUM_EVENT_CLASSES; i++) + { + if (n[i] > 0) + { + tnew = (int *) palloc(n[i] * sizeof(int)); + memcpy(tnew, t[i], n[i] * sizeof(int)); + t[i] = tnew; + } + else + t[i] = NULL; + } + + return newdesc; +} + +/* + * Free a TriggerDesc data structure. + */ void FreeTriggerDesc(TriggerDesc *trigdesc) { @@ -909,6 +1011,10 @@ FreeTriggerDesc(TriggerDesc *trigdesc) pfree(trigdesc); } +/* + * Compare two TriggerDesc structures for logical equality. + */ +#ifdef NOT_USED bool equalTriggerDescs(TriggerDesc *trigdesc1, TriggerDesc *trigdesc2) { @@ -966,6 +1072,7 @@ equalTriggerDescs(TriggerDesc *trigdesc1, TriggerDesc *trigdesc2) return false; return true; } +#endif /* NOT_USED */ /* * Call a trigger function. @@ -1455,17 +1562,17 @@ deferredTriggerAddEvent(DeferredTriggerEvent event) * event: event currently being fired. * itemno: item within event currently being fired. * rel: open relation for event. - * finfo: array of fmgr lookup cache entries (one per trigger of relation). + * trigdesc: working copy of rel's trigger info. + * finfo: array of fmgr lookup cache entries (one per trigger in trigdesc). * per_tuple_context: memory context to call trigger function in. * ---------- */ static void DeferredTriggerExecute(DeferredTriggerEvent event, int itemno, - Relation rel, FmgrInfo *finfo, + Relation rel, TriggerDesc *trigdesc, FmgrInfo *finfo, MemoryContext per_tuple_context) { Oid tgoid = event->dte_item[itemno].dti_tgoid; - TriggerDesc *trigdesc = rel->trigdesc; TriggerData LocTriggerData; HeapTupleData oldtuple; HeapTupleData newtuple; @@ -1530,7 +1637,7 @@ DeferredTriggerExecute(DeferredTriggerEvent event, int itemno, } /* - * Call the trigger and throw away an eventually returned updated + * Call the trigger and throw away any eventually returned updated * tuple. */ rettuple = ExecCallTriggerFunc(&LocTriggerData, @@ -1569,6 +1676,7 @@ deferredTriggerInvokeEvents(bool immediate_only) prev_event = NULL; MemoryContext per_tuple_context; Relation rel = NULL; + TriggerDesc *trigdesc = NULL; FmgrInfo *finfo = NULL; /* @@ -1637,6 +1745,7 @@ deferredTriggerInvokeEvents(bool immediate_only) { if (rel) heap_close(rel, NoLock); + FreeTriggerDesc(trigdesc); if (finfo) pfree(finfo); @@ -1647,16 +1756,25 @@ deferredTriggerInvokeEvents(bool immediate_only) rel = heap_open(event->dte_relid, NoLock); /* - * Allocate space to cache fmgr lookup info for - * triggers of this relation. + * Copy relation's trigger info so that we have a stable + * copy no matter what the called triggers do. + */ + trigdesc = CopyTriggerDesc(rel->trigdesc); + + if (trigdesc == NULL) + elog(ERROR, "deferredTriggerInvokeEvents: relation %u has no triggers", + event->dte_relid); + + /* + * Allocate space to cache fmgr lookup info for triggers. */ finfo = (FmgrInfo *) - palloc(rel->trigdesc->numtriggers * sizeof(FmgrInfo)); + palloc(trigdesc->numtriggers * sizeof(FmgrInfo)); MemSet(finfo, 0, - rel->trigdesc->numtriggers * sizeof(FmgrInfo)); + trigdesc->numtriggers * sizeof(FmgrInfo)); } - DeferredTriggerExecute(event, i, rel, finfo, + DeferredTriggerExecute(event, i, rel, trigdesc, finfo, per_tuple_context); event->dte_item[i].dti_state |= TRIGGER_DEFERRED_DONE; @@ -1708,6 +1826,7 @@ deferredTriggerInvokeEvents(bool immediate_only) /* Release working resources */ if (rel) heap_close(rel, NoLock); + FreeTriggerDesc(trigdesc); if (finfo) pfree(finfo); MemoryContextDelete(per_tuple_context); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 370d7e52ec36b434348300cc9f4f2530c80f3206..c94767926d23977cde93cdae8b69a34c258a2a96 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -27,7 +27,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.179 2002/09/23 22:57:44 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.180 2002/10/14 16:51:30 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -799,7 +799,8 @@ initResultRelInfo(ResultRelInfo *resultRelInfo, resultRelInfo->ri_NumIndices = 0; resultRelInfo->ri_IndexRelationDescs = NULL; resultRelInfo->ri_IndexRelationInfo = NULL; - resultRelInfo->ri_TrigDesc = resultRelationDesc->trigdesc; + /* make a copy so as not to depend on relcache info not changing... */ + resultRelInfo->ri_TrigDesc = CopyTriggerDesc(resultRelationDesc->trigdesc); resultRelInfo->ri_TrigFunctions = NULL; resultRelInfo->ri_ConstraintExprs = NULL; resultRelInfo->ri_junkFilter = NULL; diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index ac80fba519af32f713716c07062748f6aecc87be..29fbd5ec067d0f9bd21a84b100173f8e16df30d5 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.176 2002/09/22 20:56:28 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.177 2002/10/14 16:51:30 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1716,11 +1716,11 @@ RelationClearRelation(Relation relation, bool rebuild) /* * Free all the subsidiary data structures of the relcache entry. We * cannot free rd_att if we are trying to rebuild the entry, however, - * because pointers to it may be cached in various places. The trigger - * manager might also have pointers into the trigdesc, and the rule - * manager might have pointers into the rewrite rules. So to begin + * because pointers to it may be cached in various places. The rule + * manager might also have pointers into the rewrite rules. So to begin * with, we can only get rid of these fields: */ + FreeTriggerDesc(relation->trigdesc); if (relation->rd_index) pfree(relation->rd_index); if (relation->rd_am) @@ -1743,22 +1743,20 @@ RelationClearRelation(Relation relation, bool rebuild) FreeTupleDesc(relation->rd_att); if (relation->rd_rulescxt) MemoryContextDelete(relation->rd_rulescxt); - FreeTriggerDesc(relation->trigdesc); pfree(relation); } else { /* * When rebuilding an open relcache entry, must preserve ref count - * and rd_isnew flag. Also attempt to preserve the tupledesc, - * rewrite rules, and trigger substructures in place. + * and rd_isnew flag. Also attempt to preserve the tupledesc and + * rewrite-rule substructures in place. */ int old_refcnt = relation->rd_refcnt; bool old_isnew = relation->rd_isnew; TupleDesc old_att = relation->rd_att; RuleLock *old_rules = relation->rd_rules; MemoryContext old_rulescxt = relation->rd_rulescxt; - TriggerDesc *old_trigdesc = relation->trigdesc; RelationBuildDescInfo buildinfo; buildinfo.infotype = INFO_RELID; @@ -1770,7 +1768,6 @@ RelationClearRelation(Relation relation, bool rebuild) FreeTupleDesc(old_att); if (old_rulescxt) MemoryContextDelete(old_rulescxt); - FreeTriggerDesc(old_trigdesc); pfree(relation); elog(ERROR, "RelationClearRelation: relation %u deleted while still in use", buildinfo.i.info_id); @@ -1796,13 +1793,6 @@ RelationClearRelation(Relation relation, bool rebuild) if (old_rulescxt) MemoryContextDelete(old_rulescxt); } - if (equalTriggerDescs(old_trigdesc, relation->trigdesc)) - { - FreeTriggerDesc(relation->trigdesc); - relation->trigdesc = old_trigdesc; - } - else - FreeTriggerDesc(old_trigdesc); /* * Update rd_nblocks. This is kind of expensive, but I think we diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index cf1a6b61ea574407f45a8c23ab079e1ced0861c3..219b2251f5ef278cca77b0c2ab706ba8c3310a80 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: trigger.h,v 1.38 2002/09/04 20:31:42 momjian Exp $ + * $Id: trigger.h,v 1.39 2002/10/14 16:51:30 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -112,9 +112,9 @@ extern void renametrig(Oid relid, const char *oldname, const char *newname); extern void RelationBuildTriggers(Relation relation); -extern void FreeTriggerDesc(TriggerDesc *trigdesc); +extern TriggerDesc *CopyTriggerDesc(TriggerDesc *trigdesc); -extern bool equalTriggerDescs(TriggerDesc *trigdesc1, TriggerDesc *trigdesc2); +extern void FreeTriggerDesc(TriggerDesc *trigdesc); extern HeapTuple ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,