Arrange to cache a ResultRelInfo in the executor's EState for relations that
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 15 Aug 2007 21:39:50 +0000 (21:39 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 15 Aug 2007 21:39:50 +0000 (21:39 +0000)
are not one of the query's defined result relations, but nonetheless have
triggers fired against them while the query is active.  This was formerly
impossible but can now occur because of my recent patch to fix the firing
order for RI triggers.  Caching a ResultRelInfo avoids duplicating work by
repeatedly opening and closing the same relation, and also allows EXPLAIN
ANALYZE to "see" and report on these extra triggers.  Use the same mechanism
to cache open relations when firing deferred triggers at transaction shutdown;
this replaces the former one-element-cache strategy used in that case, and
should improve performance a bit when there are deferred triggers on a number
of relations.

src/backend/commands/explain.c
src/backend/commands/trigger.c
src/backend/executor/execMain.c
src/backend/executor/execUtils.c
src/include/executor/executor.h
src/include/nodes/execnodes.h

index 078b1bbc9ee99a20cee11ea17afadcd46bfb5f18..c124aa497b005d4b43a86124ad4c615af8ce799f 100644 (file)
@@ -52,6 +52,8 @@ typedef struct ExplainState
 static void ExplainOneQuery(Query *query, ExplainStmt *stmt,
                                                        const char *queryString,
                                                        ParamListInfo params, TupOutputState *tstate);
+static void report_triggers(ResultRelInfo *rInfo, bool show_relname,
+                                                       StringInfo buf);
 static double elapsed_time(instr_time *starttime);
 static void explain_outNode(StringInfo str,
                                Plan *plan, PlanState *planstate,
@@ -310,50 +312,21 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ParamListInfo params,
        if (es->printAnalyze)
        {
                ResultRelInfo *rInfo;
+               bool            show_relname;
                int                     numrels = queryDesc->estate->es_num_result_relations;
+               List       *targrels = queryDesc->estate->es_trig_target_relations;
                int                     nr;
+               ListCell   *l;
 
+               show_relname = (numrels > 1 || targrels != NIL);
                rInfo = queryDesc->estate->es_result_relations;
                for (nr = 0; nr < numrels; rInfo++, nr++)
-               {
-                       int                     nt;
-
-                       if (!rInfo->ri_TrigDesc || !rInfo->ri_TrigInstrument)
-                               continue;
-                       for (nt = 0; nt < rInfo->ri_TrigDesc->numtriggers; nt++)
-                       {
-                               Trigger    *trig = rInfo->ri_TrigDesc->triggers + nt;
-                               Instrumentation *instr = rInfo->ri_TrigInstrument + nt;
-                               char       *conname;
+                       report_triggers(rInfo, show_relname, &buf);
 
-                               /* Must clean up instrumentation state */
-                               InstrEndLoop(instr);
-
-                               /*
-                                * We ignore triggers that were never invoked; they likely
-                                * aren't relevant to the current query type.
-                                */
-                               if (instr->ntuples == 0)
-                                       continue;
-
-                               if (OidIsValid(trig->tgconstraint) &&
-                                       (conname = get_constraint_name(trig->tgconstraint)) != NULL)
-                               {
-                                       appendStringInfo(&buf, "Trigger for constraint %s",
-                                                                        conname);
-                                       pfree(conname);
-                               }
-                               else
-                                       appendStringInfo(&buf, "Trigger %s", trig->tgname);
-
-                               if (numrels > 1)
-                                       appendStringInfo(&buf, " on %s",
-                                                       RelationGetRelationName(rInfo->ri_RelationDesc));
-
-                               appendStringInfo(&buf, ": time=%.3f calls=%.0f\n",
-                                                                1000.0 * instr->total,
-                                                                instr->ntuples);
-                       }
+               foreach(l, targrels)
+               {
+                       rInfo = (ResultRelInfo *) lfirst(l);
+                       report_triggers(rInfo, show_relname, &buf);
                }
        }
 
@@ -382,6 +355,51 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ParamListInfo params,
        pfree(es);
 }
 
+/*
+ * report_triggers -
+ *             report execution stats for a single relation's triggers
+ */
+static void
+report_triggers(ResultRelInfo *rInfo, bool show_relname, StringInfo buf)
+{
+       int                     nt;
+
+       if (!rInfo->ri_TrigDesc || !rInfo->ri_TrigInstrument)
+               return;
+       for (nt = 0; nt < rInfo->ri_TrigDesc->numtriggers; nt++)
+       {
+               Trigger    *trig = rInfo->ri_TrigDesc->triggers + nt;
+               Instrumentation *instr = rInfo->ri_TrigInstrument + nt;
+               char       *conname;
+
+               /* Must clean up instrumentation state */
+               InstrEndLoop(instr);
+
+               /*
+                * We ignore triggers that were never invoked; they likely
+                * aren't relevant to the current query type.
+                */
+               if (instr->ntuples == 0)
+                       continue;
+
+               if (OidIsValid(trig->tgconstraint) &&
+                       (conname = get_constraint_name(trig->tgconstraint)) != NULL)
+               {
+                       appendStringInfo(buf, "Trigger for constraint %s", conname);
+                       pfree(conname);
+               }
+               else
+                       appendStringInfo(buf, "Trigger %s", trig->tgname);
+
+               if (show_relname)
+                       appendStringInfo(buf, " on %s",
+                                                        RelationGetRelationName(rInfo->ri_RelationDesc));
+
+               appendStringInfo(buf, ": time=%.3f calls=%.0f\n",
+                                                1000.0 * instr->total, instr->ntuples);
+       }
+}
+
 /* Compute elapsed time in seconds since given timestamp */
 static double
 elapsed_time(instr_time *starttime)
index 07ef248926cca7b583e7a9c3e9dc7adee78c4589..1551ee2eb4b8568c8014e954f213000e7258b00a 100644 (file)
@@ -2313,11 +2313,10 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
  *     Scan the given event list for events that are marked as to be fired
  *     in the current firing cycle, and fire them.
  *
- *     If estate isn't NULL, then we expect that all the firable events are
- *     for triggers of the relations included in the estate's result relation
- *     array.  This allows us to re-use the estate's open relations and
- *     trigger cache info.  When estate is NULL, we have to find the relations
- *     the hard way.
+ *     If estate isn't NULL, we use its result relation info to avoid repeated
+ *     openings and closing of trigger target relations.  If it is NULL, we
+ *     make one locally to cache the info in case there are multiple trigger
+ *     events per rel.
  *
  *     When delete_ok is TRUE, it's okay to delete fully-processed events.
  *     The events list pointers are updated.
@@ -2332,12 +2331,19 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
        AfterTriggerEvent event,
                                prev_event;
        MemoryContext per_tuple_context;
-       bool            locally_opened = false;
+       bool            local_estate = false;
        Relation        rel = NULL;
        TriggerDesc *trigdesc = NULL;
        FmgrInfo   *finfo = NULL;
        Instrumentation *instr = NULL;
 
+       /* Make a local EState if need be */
+       if (estate == NULL)
+       {
+               estate = CreateExecutorState();
+               local_estate = true;
+       }
+
        /* Make a per-tuple memory context for trigger function calls */
        per_tuple_context =
                AllocSetContextCreate(CurrentMemoryContext,
@@ -2360,77 +2366,21 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
                        event->ate_firing_id == firing_id)
                {
                        /*
-                        * So let's fire it... but first, open the correct relation if
+                        * So let's fire it... but first, find the correct relation if
                         * this is not the same relation as before.
                         */
-                       if (rel == NULL || rel->rd_id != event->ate_relid)
+                       if (rel == NULL || RelationGetRelid(rel) != event->ate_relid)
                        {
-                               if (locally_opened)
-                               {
-                                       /* close prior rel if any */
-                                       if (rel)
-                                               heap_close(rel, NoLock);
-                                       if (trigdesc)
-                                               FreeTriggerDesc(trigdesc);
-                                       if (finfo)
-                                               pfree(finfo);
-                                       Assert(instr == NULL);          /* never used in this case */
-                               }
-                               locally_opened = true;
-
-                               if (estate)
-                               {
-                                       /* Find target relation among estate's result rels */
-                                       ResultRelInfo *rInfo;
-                                       int                     nr;
-
-                                       rInfo = estate->es_result_relations;
-                                       nr = estate->es_num_result_relations;
-                                       while (nr > 0)
-                                       {
-                                               if (rInfo->ri_RelationDesc->rd_id == event->ate_relid)
-                                               {
-                                                       rel = rInfo->ri_RelationDesc;
-                                                       trigdesc = rInfo->ri_TrigDesc;
-                                                       finfo = rInfo->ri_TrigFunctions;
-                                                       instr = rInfo->ri_TrigInstrument;
-                                                       locally_opened = false;
-                                                       break;
-                                               }
-                                               rInfo++;
-                                               nr--;
-                                       }
-                               }
-
-                               if (locally_opened)
-                               {
-                                       /* Hard way: open target relation for ourselves */
-
-                                       /*
-                                        * We assume that an appropriate lock is still held by the
-                                        * executor, so grab no new lock here.
-                                        */
-                                       rel = heap_open(event->ate_relid, NoLock);
-
-                                       /*
-                                        * 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)           /* should not happen */
-                                               elog(ERROR, "relation %u has no triggers",
-                                                        event->ate_relid);
-
-                                       /*
-                                        * Allocate space to cache fmgr lookup info for triggers.
-                                        */
-                                       finfo = (FmgrInfo *)
-                                               palloc0(trigdesc->numtriggers * sizeof(FmgrInfo));
-
-                                       /* Never any EXPLAIN info in this case */
-                                       instr = NULL;
-                               }
+                               ResultRelInfo *rInfo;
+
+                               rInfo = ExecGetTriggerResultRel(estate, event->ate_relid);
+                               rel = rInfo->ri_RelationDesc;
+                               trigdesc = rInfo->ri_TrigDesc;
+                               finfo = rInfo->ri_TrigFunctions;
+                               instr = rInfo->ri_TrigInstrument;
+                               if (trigdesc == NULL)           /* should not happen */
+                                       elog(ERROR, "relation %u has no triggers",
+                                                event->ate_relid);
                        }
 
                        /*
@@ -2480,17 +2430,22 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
        events->tail = prev_event;
 
        /* Release working resources */
-       if (locally_opened)
+       MemoryContextDelete(per_tuple_context);
+
+       if (local_estate)
        {
-               if (rel)
-                       heap_close(rel, NoLock);
-               if (trigdesc)
-                       FreeTriggerDesc(trigdesc);
-               if (finfo)
-                       pfree(finfo);
-               Assert(instr == NULL);  /* never used in this case */
+               ListCell *l;
+
+               foreach(l, estate->es_trig_target_relations)
+               {
+                       ResultRelInfo *resultRelInfo = (ResultRelInfo *) lfirst(l);
+
+                       /* Close indices and then the relation itself */
+                       ExecCloseIndices(resultRelInfo);
+                       heap_close(resultRelInfo->ri_RelationDesc, NoLock);
+               }
+               FreeExecutorState(estate);
        }
-       MemoryContextDelete(per_tuple_context);
 }
 
 
index 621ef89d64b851cf61b84239b6a0d691f0ffbe02..6855a68d88b13df4ee1e391343d42d7447042c25 100644 (file)
@@ -66,8 +66,8 @@ typedef struct evalPlanQual
 /* decls for local routines only used within this module */
 static void InitPlan(QueryDesc *queryDesc, int eflags);
 static void initResultRelInfo(ResultRelInfo *resultRelInfo,
+                                 Relation resultRelationDesc,
                                  Index resultRelationIndex,
-                                 List *rangeTable,
                                  CmdType operation,
                                  bool doInstrument);
 static void ExecEndPlan(PlanState *planstate, EState *estate);
@@ -494,9 +494,15 @@ InitPlan(QueryDesc *queryDesc, int eflags)
                resultRelInfo = resultRelInfos;
                foreach(l, resultRelations)
                {
+                       Index           resultRelationIndex = lfirst_int(l);
+                       Oid                     resultRelationOid;
+                       Relation        resultRelation;
+
+                       resultRelationOid = getrelid(resultRelationIndex, rangeTable);
+                       resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
                        initResultRelInfo(resultRelInfo,
-                                                         lfirst_int(l),
-                                                         rangeTable,
+                                                         resultRelation,
+                                                         resultRelationIndex,
                                                          operation,
                                                          estate->es_instrument);
                        resultRelInfo++;
@@ -831,19 +837,20 @@ InitPlan(QueryDesc *queryDesc, int eflags)
  */
 static void
 initResultRelInfo(ResultRelInfo *resultRelInfo,
+                                 Relation resultRelationDesc,
                                  Index resultRelationIndex,
-                                 List *rangeTable,
                                  CmdType operation,
                                  bool doInstrument)
 {
-       Oid                     resultRelationOid;
-       Relation        resultRelationDesc;
-
-       resultRelationOid = getrelid(resultRelationIndex, rangeTable);
-       resultRelationDesc = heap_open(resultRelationOid, RowExclusiveLock);
-
+       /*
+        * Check valid relkind ... parser and/or planner should have noticed
+        * this already, but let's make sure.
+        */
        switch (resultRelationDesc->rd_rel->relkind)
        {
+               case RELKIND_RELATION:
+                       /* OK */
+                       break;
                case RELKIND_SEQUENCE:
                        ereport(ERROR,
                                        (errcode(ERRCODE_WRONG_OBJECT_TYPE),
@@ -862,8 +869,15 @@ initResultRelInfo(ResultRelInfo *resultRelInfo,
                                         errmsg("cannot change view \"%s\"",
                                                        RelationGetRelationName(resultRelationDesc))));
                        break;
+               default:
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                        errmsg("cannot change relation \"%s\"",
+                                                       RelationGetRelationName(resultRelationDesc))));
+                       break;
        }
 
+       /* OK, fill in the node */
        MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
        resultRelInfo->type = T_ResultRelInfo;
        resultRelInfo->ri_RangeTableIndex = resultRelationIndex;
@@ -904,6 +918,76 @@ initResultRelInfo(ResultRelInfo *resultRelInfo,
                ExecOpenIndices(resultRelInfo);
 }
 
+/*
+ *             ExecGetTriggerResultRel
+ *
+ * Get a ResultRelInfo for a trigger target relation.  Most of the time,
+ * triggers are fired on one of the result relations of the query, and so
+ * we can just return a member of the es_result_relations array.  (Note: in
+ * self-join situations there might be multiple members with the same OID;
+ * if so it doesn't matter which one we pick.)  However, it is sometimes
+ * necessary to fire triggers on other relations; this happens mainly when an
+ * RI update trigger queues additional triggers on other relations, which will
+ * be processed in the context of the outer query.  For efficiency's sake,
+ * we want to have a ResultRelInfo for those triggers too; that can avoid
+ * repeated re-opening of the relation.  (It also provides a way for EXPLAIN
+ * ANALYZE to report the runtimes of such triggers.)  So we make additional
+ * ResultRelInfo's as needed, and save them in es_trig_target_relations.
+ */
+ResultRelInfo *
+ExecGetTriggerResultRel(EState *estate, Oid relid)
+{
+       ResultRelInfo *rInfo;
+       int                     nr;
+       ListCell   *l;
+       Relation        rel;
+       MemoryContext oldcontext;
+
+       /* First, search through the query result relations */
+       rInfo = estate->es_result_relations;
+       nr = estate->es_num_result_relations;
+       while (nr > 0)
+       {
+               if (RelationGetRelid(rInfo->ri_RelationDesc) == relid)
+                       return rInfo;
+               rInfo++;
+               nr--;
+       }
+       /* Nope, but maybe we already made an extra ResultRelInfo for it */
+       foreach(l, estate->es_trig_target_relations)
+       {
+               rInfo = (ResultRelInfo *) lfirst(l);
+               if (RelationGetRelid(rInfo->ri_RelationDesc) == relid)
+                       return rInfo;
+       }
+       /* Nope, so we need a new one */
+
+       /*
+        * Open the target relation's relcache entry.  We assume that an
+        * appropriate lock is still held by the backend from whenever the
+        * trigger event got queued, so we need take no new lock here.
+        */
+       rel = heap_open(relid, NoLock);
+
+       /*
+        * Make the new entry in the right context.  Currently, we don't need
+        * any index information in ResultRelInfos used only for triggers,
+        * so tell initResultRelInfo it's a DELETE.
+        */
+       oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
+       rInfo = makeNode(ResultRelInfo);
+       initResultRelInfo(rInfo,
+                                         rel,
+                                         0,            /* dummy rangetable index */
+                                         CMD_DELETE,
+                                         estate->es_instrument);
+       estate->es_trig_target_relations =
+               lappend(estate->es_trig_target_relations, rInfo);
+       MemoryContextSwitchTo(oldcontext);
+
+       return rInfo;
+}
+
 /*
  *             ExecContextForcesOids
  *
@@ -1019,6 +1103,17 @@ ExecEndPlan(PlanState *planstate, EState *estate)
                resultRelInfo++;
        }
 
+       /*
+        * likewise close any trigger target relations
+        */
+       foreach(l, estate->es_trig_target_relations)
+       {
+               resultRelInfo = (ResultRelInfo *) lfirst(l);
+               /* Close indices and then the relation itself */
+               ExecCloseIndices(resultRelInfo);
+               heap_close(resultRelInfo->ri_RelationDesc, NoLock);
+       }
+
        /*
         * close any relations selected FOR UPDATE/FOR SHARE, again keeping locks
         */
@@ -2267,6 +2362,7 @@ EvalPlanQualStart(evalPlanQual *epq, EState *estate, evalPlanQual *priorepq)
        epqstate->es_num_result_relations = estate->es_num_result_relations;
        epqstate->es_result_relation_info = estate->es_result_relation_info;
        epqstate->es_junkFilter = estate->es_junkFilter;
+       /* es_trig_target_relations must NOT be copied */
        epqstate->es_into_relation_descriptor = estate->es_into_relation_descriptor;
        epqstate->es_into_relation_use_wal = estate->es_into_relation_use_wal;
        epqstate->es_param_list_info = estate->es_param_list_info;
@@ -2331,7 +2427,8 @@ EvalPlanQualStart(evalPlanQual *epq, EState *estate, evalPlanQual *priorepq)
  *
  * This is a cut-down version of ExecutorEnd(); basically we want to do most
  * of the normal cleanup, but *not* close result relations (which we are
- * just sharing from the outer query).
+ * just sharing from the outer query).  We do, however, have to close any
+ * trigger target relations that got opened, since those are not shared.
  */
 static void
 EvalPlanQualStop(evalPlanQual *epq)
@@ -2360,6 +2457,15 @@ EvalPlanQualStop(evalPlanQual *epq)
                epqstate->es_evTuple[epq->rti - 1] = NULL;
        }
 
+       foreach(l, epqstate->es_trig_target_relations)
+       {
+               ResultRelInfo *resultRelInfo = (ResultRelInfo *) lfirst(l);
+
+               /* Close indices and then the relation itself */
+               ExecCloseIndices(resultRelInfo);
+               heap_close(resultRelInfo->ri_RelationDesc, NoLock);
+       }
+
        MemoryContextSwitchTo(oldcontext);
 
        FreeExecutorState(epqstate);
index 232d9a118421df218dcdeda7c2194795fac84b3b..db0e70b67fb0c5598107eb937463e7ed4e7d4259 100644 (file)
@@ -184,6 +184,7 @@ CreateExecutorState(void)
 
        estate->es_junkFilter = NULL;
 
+       estate->es_trig_target_relations = NIL;
        estate->es_trig_tuple_slot = NULL;
 
        estate->es_into_relation_descriptor = NULL;
index 351787ebf6d58896876bc7fefcc16944fa613cae..ce7532d5e98b9261300d37d3d19f8c9e1f4534ca 100644 (file)
@@ -138,6 +138,7 @@ extern TupleTableSlot *ExecutorRun(QueryDesc *queryDesc,
                        ScanDirection direction, long count);
 extern void ExecutorEnd(QueryDesc *queryDesc);
 extern void ExecutorRewind(QueryDesc *queryDesc);
+extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
 extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
 extern void ExecConstraints(ResultRelInfo *resultRelInfo,
                                TupleTableSlot *slot, EState *estate);
index 49e75e4fd5f8cb048e768eaf2d89e9e07f3d09fa..5b07d1f15bd3f4f39021b0c971a506d61a7f8af7 100644 (file)
@@ -310,9 +310,12 @@ typedef struct EState
        ResultRelInfo *es_result_relation_info;         /* currently active array elt */
        JunkFilter *es_junkFilter;      /* currently active junk filter */
 
+       /* Stuff used for firing triggers: */
+       List       *es_trig_target_relations;   /* trigger-only ResultRelInfos */
        TupleTableSlot *es_trig_tuple_slot; /* for trigger output tuples */
 
-       Relation        es_into_relation_descriptor;    /* for SELECT INTO */
+       /* Stuff used for SELECT INTO: */
+       Relation        es_into_relation_descriptor;
        bool            es_into_relation_use_wal;
 
        /* Parameter info: */