Arrange to cache a ResultRelInfo in the executor's EState for relations that

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.

Author: tglsfdc

src/backend/commands/explain.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994-5, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/explain.c,v 1.164 2007/05/25 17:54:24 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/explain.c,v 1.165 2007/08/15 21:39:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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)

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.217 2007/08/15 19:15:46 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.218 2007/08/15 21:39:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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);
 }