From 807dde3d299a2d4ae89d96ea7422d0feb9c58e4b Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Fri, 9 Oct 2020 22:09:08 +0300
Subject: [PATCH 2/2] Merge ExecCleanUpTriggerState() and
 ExecCloseResultRelations().

And some other kibitzing.
---
 src/backend/commands/copy.c              |  9 ++-
 src/backend/commands/explain.c           |  2 +-
 src/backend/commands/trigger.c           |  2 +-
 src/backend/executor/execMain.c          | 83 ++++++++++--------------
 src/backend/executor/execUtils.c         |  7 +-
 src/backend/replication/logical/worker.c |  6 +-
 src/include/executor/executor.h          |  1 -
 src/include/nodes/execnodes.h            | 17 ++---
 8 files changed, 59 insertions(+), 68 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 6948214334..bfdd366139 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2727,6 +2727,7 @@ CopyFrom(CopyState cstate)
 	bool		leafpart_use_multi_insert = false;
 
 	Assert(cstate->rel);
+	Assert(list_length(cstate->range_table) == 1);
 
 	/*
 	 * The target must be a plain, foreign, or partitioned relation, or have
@@ -3352,15 +3353,13 @@ CopyFrom(CopyState cstate)
 	if (insertMethod != CIM_SINGLE)
 		CopyMultiInsertInfoCleanup(&multiInsertInfo);
 
-	ExecCloseResultRelations(estate);
-	ExecCloseRangeTableRelations(estate);
-
 	/* Close all the partitioned tables, leaf partitions, and their indices */
 	if (proute)
 		ExecCleanupTupleRouting(mtstate, proute);
 
-	/* Close any trigger target relations */
-	ExecCleanUpTriggerState(estate);
+	/* Close the result relations */
+	ExecCloseResultRelations(estate);
+	ExecCloseRangeTableRelations(estate);
 
 	FreeExecutorState(estate);
 
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 3210f90bd1..1cc47b0e77 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -786,7 +786,7 @@ ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc)
 					routerels != NIL || targrels != NIL);
 	foreach(l, resultrels)
 	{
-		rInfo = lfirst(l);
+		rInfo = (ResultRelInfo *) lfirst(l);
 		report_triggers(rInfo, show_relname, es);
 	}
 
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 672fccff5b..3b4fbdadf4 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -4227,7 +4227,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
 
 	if (local_estate)
 	{
-		ExecCleanUpTriggerState(estate);
+		ExecCloseResultRelations(estate);
 		ExecResetTupleTable(estate->es_tupleTable, false);
 		FreeExecutorState(estate);
 	}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index f62fd8f7aa..403d62f2b6 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1340,35 +1340,6 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
 	return rInfo;
 }
 
-/*
- * Close any relations that have been opened by ExecGetTriggerResultRel().
- */
-void
-ExecCleanUpTriggerState(EState *estate)
-{
-	ListCell   *l;
-
-	foreach(l, estate->es_trig_target_relations)
-	{
-		ResultRelInfo *resultRelInfo = (ResultRelInfo *) lfirst(l);
-
-		/*
-		 * Assert this is a "dummy" ResultRelInfo, see above.  Otherwise we
-		 * might be issuing a duplicate close against a Relation opened by
-		 * ExecGetRangeTableRelation.
-		 */
-		Assert(resultRelInfo->ri_RangeTableIndex == 0);
-
-		/*
-		 * Since ExecGetTriggerResultRel doesn't call ExecOpenIndices for
-		 * these rels, we needn't call ExecCloseIndices either.
-		 */
-		Assert(resultRelInfo->ri_NumIndices == 0);
-
-		table_close(resultRelInfo->ri_RelationDesc, NoLock);
-	}
-}
-
 /* ----------------------------------------------------------------
  *		ExecPostprocessPlan
  *
@@ -1449,7 +1420,7 @@ ExecEndPlan(PlanState *planstate, EState *estate)
 	 */
 	ExecResetTupleTable(estate->es_tupleTable, false);
 
-	/* Close indexes of result relation(s), if any. */
+	/* Close result relation(s), if any. */
 	ExecCloseResultRelations(estate);
 
 	/*
@@ -1457,22 +1428,19 @@ ExecEndPlan(PlanState *planstate, EState *estate)
 	 * release any locks we might hold on those rels.
 	 */
 	ExecCloseRangeTableRelations(estate);
-
-	/* likewise close any trigger target relations */
-	ExecCleanUpTriggerState(estate);
 }
 
 /*
- * ExecCloseResultRelations
+ * Close any relations that have been opened for ResultRelInfos.
  */
 void
 ExecCloseResultRelations(EState *estate)
 {
-	ListCell *l;
+	ListCell   *l;
 
 	/*
-	 * close indexes of result relation(s) if any.  (Rels themselves get
-	 * closed next.)
+	 * close indexes of result relation(s) if any.  (Rels themselves are
+	 * closed in ExecCloseRangeTableRelations())
 	 */
 	foreach(l, estate->es_opened_result_relations)
 	{
@@ -1480,15 +1448,36 @@ ExecCloseResultRelations(EState *estate)
 
 		ExecCloseIndices(resultRelInfo);
 	}
+
+	/* Close any relations that have been opened by ExecGetTriggerResultRel(). */
+	foreach(l, estate->es_trig_target_relations)
+	{
+		ResultRelInfo *resultRelInfo = (ResultRelInfo *) lfirst(l);
+
+		/*
+		 * Assert this is a "dummy" ResultRelInfo, see above.  Otherwise we
+		 * might be issuing a duplicate close against a Relation opened by
+		 * ExecGetRangeTableRelation.
+		 */
+		Assert(resultRelInfo->ri_RangeTableIndex == 0);
+
+		/*
+		 * Since ExecGetTriggerResultRel doesn't call ExecOpenIndices for
+		 * these rels, we needn't call ExecCloseIndices either.
+		 */
+		Assert(resultRelInfo->ri_NumIndices == 0);
+
+		table_close(resultRelInfo->ri_RelationDesc, NoLock);
+	}
 }
 
 /*
- * ExecCloseRangeTableRelations
+ * Close all relations opened by ExecGetRangeTableRelation()
  */
 void
 ExecCloseRangeTableRelations(EState *estate)
 {
-	int		i;
+	int			i;
 
 	for (i = 0; i < estate->es_range_table_size; i++)
 	{
@@ -2689,9 +2678,9 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
 
 	/*
 	 * Child EPQ EStates share the parent's copy of unchanging state such as
-	 * the snapshot, rangetable, and external Param info.
-	 * They need their own copies of local state, including a tuple table,
-	 * es_param_exec_vals, result-rel info, etc.
+	 * the snapshot, rangetable, and external Param info.  They need their own
+	 * copies of local state, including a tuple table, es_param_exec_vals,
+	 * result-rel info, etc.
 	 */
 	rcestate->es_direction = ForwardScanDirection;
 	rcestate->es_snapshot = parentestate->es_snapshot;
@@ -2704,9 +2693,10 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
 	rcestate->es_plannedstmt = parentestate->es_plannedstmt;
 	rcestate->es_junkFilter = parentestate->es_junkFilter;
 	rcestate->es_output_cid = parentestate->es_output_cid;
+
 	/*
-	 * ResultRelInfos needed by subplans are initialized from scratch when
-	 * the subplans themselves are initialized.
+	 * ResultRelInfos needed by subplans are initialized from scratch when the
+	 * subplans themselves are initialized.
 	 */
 	if (parentestate->es_result_relations)
 		ExecInitResultRelationsArray(rcestate);
@@ -2859,13 +2849,10 @@ EvalPlanQualEnd(EPQState *epqstate)
 		ExecEndNode(subplanstate);
 	}
 
-	ExecCloseResultRelations(estate);
-
 	/* throw away the per-estate tuple table, some node may have used it */
 	ExecResetTupleTable(estate->es_tupleTable, false);
 
-	/* close any trigger target relations attached to this EState */
-	ExecCleanUpTriggerState(estate);
+	ExecCloseResultRelations(estate);
 
 	MemoryContextSwitchTo(oldcontext);
 
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 169dd925ad..eda84bca45 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -770,6 +770,7 @@ ExecInitRangeTable(EState *estate, List *rangeTable)
 	 * es_result_relations and es_rowmarks are also parallel to es_range_table,
 	 * but are only allocated if needed.
 	 */
+	estate->es_result_relations = NULL;
 	estate->es_rowmarks = NULL;
 }
 
@@ -827,9 +828,9 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
  * ExecInitResultRelationsArray
  *		Allocate space to hold ResultRelInfo pointers of result relations
  *
- * Although not relations in the range table may be result relations, we
- * allocate that many pointers, because that allows to access individual
- * entries by RT index (minus 1 to be accurate), which is convenient.
+ * Usually, only some relations in the range table are result relations, but
+ * we allocate an array with the same size as the range table, so that we
+ * can index it by the RT index (minus 1 to be accurate).
  */
 void
 ExecInitResultRelationsArray(EState *estate)
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 77f71e52d4..07889f7a7b 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -357,9 +357,13 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel)
 	ExecInitRangeTable(estate, list_make1(rte));
 	ExecInitResultRelationsArray(estate);
 
+	/*
+	 * Initialize a ResultRelInfo for the target relation.  Note that we
+	 * intentionally don't add it to the es_opened_result_relations list,
+	 * because we do our own cleanup and don't use ExecCloseResultRelations().
+	 */
 	resultRelInfo = makeNode(ResultRelInfo);
 	InitResultRelInfo(resultRelInfo, rel->localrel, 1, NULL, 0);
-
 	estate->es_result_relations[0] = resultRelInfo;
 	estate->es_result_relation_info = resultRelInfo;
 
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 54455e1446..76ef5cd91c 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -191,7 +191,6 @@ extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
 							  Relation partition_root,
 							  int instrument_options);
 extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
-extern void ExecCleanUpTriggerState(EState *estate);
 extern void ExecConstraints(ResultRelInfo *resultRelInfo,
 							TupleTableSlot *slot, EState *estate);
 extern bool ExecPartitionCheck(ResultRelInfo *resultRelInfo,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 7eb5ca6a04..b6500abc24 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -508,14 +508,6 @@ typedef struct EState
 	Index		es_range_table_size;	/* size of the range table arrays */
 	Relation   *es_relations;	/* Array of per-range-table-entry Relation
 								 * pointers, or NULL if not yet opened */
-	ResultRelInfo **es_result_relations;	/* Array of Per-range-table-entry
-											 * ResultRelInfo pointers, or
-											 * NULL if a given range table
-											 * relation not a target
-											 * table */
-	List		*es_opened_result_relations;	/* List of non-NULL entries
-												 * in es_result_relations added
-												 * in no specific order */
 	struct ExecRowMark **es_rowmarks;	/* Array of per-range-table-entry
 										 * ExecRowMarks, or NULL if none */
 	PlannedStmt *es_plannedstmt;	/* link to top of plan tree */
@@ -526,6 +518,15 @@ typedef struct EState
 	/* If query can insert/delete tuples, the command ID to mark them with */
 	CommandId	es_output_cid;
 
+	/* Info about target table(s) for insert/update/delete queries: */
+	ResultRelInfo **es_result_relations;	/* Array of Per-range-table-entry
+											 * ResultRelInfo pointers, or
+											 * NULL if a given range table
+											 * relation not a target
+											 * table */
+	List		*es_opened_result_relations;	/* List of non-NULL entries
+												 * in es_result_relations added
+												 * in no specific order */
 	ResultRelInfo *es_result_relation_info; /* currently active array elt */
 
 	PartitionDirectory es_partition_directory;	/* for PartitionDesc lookup */
-- 
2.20.1

