From a55bd363690bc4c28047e4b874ce80384e37c49d Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Tue, 1 Aug 2023 11:36:24 +0900
Subject: [PATCH v43 1/5] Make PlanState tree cleanup non-recursive

With this change, node type specific subroutines of ExecEndNode()
are no longer required to also clean up the child nodes of a given
node, only its own stuff.  Instead, ExecEndPlan() calls
ExecInitNode() directly for each node in the PlanState tree by
iterating over a list (EState.es_planstate_nodes) of all those nodes
built during the ExecInitNode() traversal of the tree.

This changes the order in which the nodes get cleaned up, because
they are now cleaned up in the order in which they are added into
the list which is from leaf-level up to the root, whereas with the
current recursive approach cleanup occurs from the root to the
leaves.  The change seems harmless though, because there isn't
necessarily any coupling between of the cleanup actions of parent
and child nodes.

The main motivation behind this change is to allow the cases in
the future where ExecInitNode() traversal of the plan tree may
be aborted in the middle resulting in a partially initialized
PlanState tree.  Dealing with that case by making the cleanup
phase walk over a list of successfully initialized nodes seems
better / more robust than making the individual ExecEndNode()
subroutines deal with partially valid PlanState nodes.
---
 src/backend/executor/README                |  4 +-
 src/backend/executor/execMain.c            | 36 +++++++-------
 src/backend/executor/execProcnode.c        | 56 ++++++++++------------
 src/backend/executor/execUtils.c           |  2 +
 src/backend/executor/nodeAgg.c             |  4 +-
 src/backend/executor/nodeAppend.c          | 20 +-------
 src/backend/executor/nodeBitmapAnd.c       | 23 +--------
 src/backend/executor/nodeBitmapHeapscan.c  |  5 +-
 src/backend/executor/nodeBitmapOr.c        | 23 +--------
 src/backend/executor/nodeForeignscan.c     |  4 +-
 src/backend/executor/nodeGather.c          |  2 +-
 src/backend/executor/nodeGatherMerge.c     |  2 +-
 src/backend/executor/nodeGroup.c           |  5 +-
 src/backend/executor/nodeHash.c            |  8 +---
 src/backend/executor/nodeHashjoin.c        |  6 +--
 src/backend/executor/nodeIncrementalSort.c |  5 +-
 src/backend/executor/nodeLimit.c           |  2 +-
 src/backend/executor/nodeLockRows.c        |  2 +-
 src/backend/executor/nodeMaterial.c        |  5 +-
 src/backend/executor/nodeMemoize.c         |  5 +-
 src/backend/executor/nodeMergeAppend.c     | 20 +-------
 src/backend/executor/nodeMergejoin.c       |  6 +--
 src/backend/executor/nodeModifyTable.c     |  7 +--
 src/backend/executor/nodeNestloop.c        |  6 +--
 src/backend/executor/nodeProjectSet.c      |  5 +-
 src/backend/executor/nodeRecursiveunion.c  |  6 +--
 src/backend/executor/nodeResult.c          |  5 +-
 src/backend/executor/nodeSetOp.c           |  2 +-
 src/backend/executor/nodeSort.c            |  5 +-
 src/backend/executor/nodeSubqueryscan.c    |  5 +-
 src/backend/executor/nodeUnique.c          |  2 +-
 src/backend/executor/nodeWindowAgg.c       |  4 +-
 src/include/nodes/execnodes.h              |  2 +
 33 files changed, 80 insertions(+), 214 deletions(-)

diff --git a/src/backend/executor/README b/src/backend/executor/README
index 17775a49e2..67a5c1769b 100644
--- a/src/backend/executor/README
+++ b/src/backend/executor/README
@@ -310,13 +310,13 @@ This is a sketch of control flow for full query processing:
 		AfterTriggerEndQuery
 
 	ExecutorEnd
-		ExecEndNode --- recursively releases resources
+		ExecEndPlan --- releases plan resources
 		FreeExecutorState
 			frees per-query context and child contexts
 
 	FreeQueryDesc
 
-Per above comments, it's not really critical for ExecEndNode to free any
+Per above comments, it's not really critical for ExecEndPlan to free any
 memory; it'll all go away in FreeExecutorState anyway.  However, we do need to
 be careful to close relations, drop buffer pins, etc, so we do need to scan
 the plan state tree to find these sorts of resources.
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 4c5a7bbf62..235bb52ccc 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -82,7 +82,7 @@ ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL;
 static void InitPlan(QueryDesc *queryDesc, int eflags);
 static void CheckValidRowMarkRel(Relation rel, RowMarkType markType);
 static void ExecPostprocessPlan(EState *estate);
-static void ExecEndPlan(PlanState *planstate, EState *estate);
+static void ExecEndPlan(EState *estate);
 static void ExecutePlan(EState *estate, PlanState *planstate,
 						bool use_parallel_mode,
 						CmdType operation,
@@ -500,7 +500,7 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
 	 */
 	oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
 
-	ExecEndPlan(queryDesc->planstate, estate);
+	ExecEndPlan(estate);
 
 	/* do away with our snapshots */
 	UnregisterSnapshot(estate->es_snapshot);
@@ -1499,23 +1499,21 @@ ExecPostprocessPlan(EState *estate)
  * ----------------------------------------------------------------
  */
 static void
-ExecEndPlan(PlanState *planstate, EState *estate)
+ExecEndPlan(EState *estate)
 {
 	ListCell   *l;
 
 	/*
-	 * shut down the node-type-specific query processing
+	 * Shut down the node-type-specific query processing for all nodes that
+	 * were initialized in InitPlan().  That includes the nodes in both the
+	 * main plan tree (es_plannedstmt->planTree) and those in subplans
+	 * (es_plannedstmt->subplans).
 	 */
-	ExecEndNode(planstate);
-
-	/*
-	 * for subplans too
-	 */
-	foreach(l, estate->es_subplanstates)
+	foreach(l, estate->es_planstate_nodes)
 	{
-		PlanState  *subplanstate = (PlanState *) lfirst(l);
+		PlanState  *pstate = (PlanState *) lfirst(l);
 
-		ExecEndNode(subplanstate);
+		ExecEndNode(pstate);
 	}
 
 	/*
@@ -3030,13 +3028,17 @@ EvalPlanQualEnd(EPQState *epqstate)
 
 	oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
 
-	ExecEndNode(epqstate->recheckplanstate);
-
-	foreach(l, estate->es_subplanstates)
+	/*
+	 * Shut down the node-type-specific query processing for all nodes that
+	 * were initialized in InitPlan().  That includes the nodes in both the
+	 * main plan tree (epqstate->plan) and those in subplans
+	 * (es_plannedstmt->subplans).
+	 */
+	foreach(l, estate->es_planstate_nodes)
 	{
-		PlanState  *subplanstate = (PlanState *) lfirst(l);
+		PlanState  *planstate = (PlanState *) lfirst(l);
 
-		ExecEndNode(subplanstate);
+		ExecEndNode(planstate);
 	}
 
 	/* throw away the per-estate tuple table, some node may have used it */
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index 4d288bc8d4..653f74cf58 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -1,11 +1,13 @@
 /*-------------------------------------------------------------------------
  *
  * execProcnode.c
- *	 contains dispatch functions which call the appropriate "initialize",
- *	 "get a tuple", and "cleanup" routines for the given node type.
- *	 If the node has children, then it will presumably call ExecInitNode,
- *	 ExecProcNode, or ExecEndNode on its subnodes and do the appropriate
- *	 processing.
+ *	 Contains dispatch functions ExecInitNode(), ExecProcNode(), and
+ *	 ExecEndNode(), which call the appropriate "initialize", "get a tuple",
+ *	 and "cleanup" routines, respectively, for the given node type.
+ *
+ *	 While the first two process the node's children recursively, ExecEndNode()
+ *	 is only concerned with the cleaning of the node itself while the children
+ *	 are processed by the caller.
  *
  * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
@@ -49,7 +51,9 @@
  *		Eventually this calls ExecInitNode() on the right and left subplans
  *		and so forth until the entire plan is initialized.  The result
  *		of ExecInitNode() is a plan state tree built with the same structure
- *		as the underlying plan tree.
+ *		as the underlying plan tree.  (The plan state nodes are also added to
+ *		a list in the same order in which they are created for the final
+ *		cleanup processing.)
  *
  *	  * Then when ExecutorRun() is called, it calls ExecutePlan() which calls
  *		ExecProcNode() repeatedly on the top node of the plan state tree.
@@ -61,14 +65,10 @@
  *		form the tuples it returns.
  *
  *	  * Eventually ExecSeqScan() stops returning tuples and the nest
- *		loop join ends.  Lastly, ExecutorEnd() calls ExecEndNode() which
- *		calls ExecEndNestLoop() which in turn calls ExecEndNode() on
- *		its subplans which result in ExecEndSeqScan().
+ *		loop join ends.  Lastly, ExecutorEnd() calls ExecEndPlan(), which
+ *		in turn calls ExecEndNode() on all the nodes that were initialized:
+ *		the two Seq Scans and the Nest Loop in this case.
  *
- *		This should show how the executor works by having
- *		ExecInitNode(), ExecProcNode() and ExecEndNode() dispatch
- *		their work to the appropriate node support routines which may
- *		in turn call these routines themselves on their subplans.
  */
 #include "postgres.h"
 
@@ -136,6 +136,9 @@ static bool ExecShutdownNode_walker(PlanState *node, void *context);
  *		  'eflags' is a bitwise OR of flag bits described in executor.h
  *
  *		Returns a PlanState node corresponding to the given Plan node.
+ *
+ *		As a side-effect, all PlanState nodes that are created are appended to
+ *		estate->es_planstate_nodes for the cleanup processing in ExecEndPlan().
  * ------------------------------------------------------------------------
  */
 PlanState *
@@ -411,6 +414,10 @@ ExecInitNode(Plan *node, EState *estate, int eflags)
 		result->instrument = InstrAlloc(1, estate->es_instrument,
 										result->async_capable);
 
+	/* And remember for the cleanup processing in ExecEndPlan(). */
+	estate->es_planstate_nodes = lappend(estate->es_planstate_nodes,
+										 result);
+
 	return result;
 }
 
@@ -545,29 +552,18 @@ MultiExecProcNode(PlanState *node)
 /* ----------------------------------------------------------------
  *		ExecEndNode
  *
- *		Recursively cleans up all the nodes in the plan rooted
- *		at 'node'.
+ *		Cleans up node
  *
- *		After this operation, the query plan will not be able to be
- *		processed any further.  This should be called only after
- *		the query plan has been fully executed.
+ *		Unlike ExecInitNode(), this does not recurse into child nodes, because
+ *		they are processed separately.  So the ExecEnd* routine for any given
+ *		node type is only responsible for cleaning up its own resources.
  * ----------------------------------------------------------------
  */
 void
 ExecEndNode(PlanState *node)
 {
-	/*
-	 * do nothing when we get to the end of a leaf on tree.
-	 */
-	if (node == NULL)
-		return;
-
-	/*
-	 * Make sure there's enough stack available. Need to check here, in
-	 * addition to ExecProcNode() (via ExecProcNodeFirst()), because it's not
-	 * guaranteed that ExecProcNode() is reached for all nodes.
-	 */
-	check_stack_depth();
+	/* We only ever get called on nodes that were actually initialized. */
+	Assert(node != NULL);
 
 	if (node->chgParam != NULL)
 	{
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index c06b228858..b567165003 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -154,6 +154,8 @@ CreateExecutorState(void)
 
 	estate->es_exprcontexts = NIL;
 
+	estate->es_planstate_nodes = NIL;
+
 	estate->es_subplanstates = NIL;
 
 	estate->es_auxmodifytables = NIL;
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 468db94fe5..e9d9ab6bdd 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -4304,7 +4304,6 @@ GetAggInitVal(Datum textInitVal, Oid transtype)
 void
 ExecEndAgg(AggState *node)
 {
-	PlanState  *outerPlan;
 	int			transno;
 	int			numGroupingSets = Max(node->maxsets, 1);
 	int			setno;
@@ -4367,8 +4366,7 @@ ExecEndAgg(AggState *node)
 	/* clean up tuple table */
 	ExecClearTuple(node->ss.ss_ScanTupleSlot);
 
-	outerPlan = outerPlanState(node);
-	ExecEndNode(outerPlan);
+	/* outerPlan is closely separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index 609df6b9e6..9148d7d3b1 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -376,30 +376,12 @@ ExecAppend(PlanState *pstate)
 
 /* ----------------------------------------------------------------
  *		ExecEndAppend
- *
- *		Shuts down the subscans of the append node.
- *
- *		Returns nothing of interest.
  * ----------------------------------------------------------------
  */
 void
 ExecEndAppend(AppendState *node)
 {
-	PlanState **appendplans;
-	int			nplans;
-	int			i;
-
-	/*
-	 * get information from the node
-	 */
-	appendplans = node->appendplans;
-	nplans = node->as_nplans;
-
-	/*
-	 * shut down each of the subscans
-	 */
-	for (i = 0; i < nplans; i++)
-		ExecEndNode(appendplans[i]);
+	/* Nothing to do as the nodes in appendplans are closed separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeBitmapAnd.c b/src/backend/executor/nodeBitmapAnd.c
index 4c5eb2b23b..147592f7e2 100644
--- a/src/backend/executor/nodeBitmapAnd.c
+++ b/src/backend/executor/nodeBitmapAnd.c
@@ -168,33 +168,12 @@ MultiExecBitmapAnd(BitmapAndState *node)
 
 /* ----------------------------------------------------------------
  *		ExecEndBitmapAnd
- *
- *		Shuts down the subscans of the BitmapAnd node.
- *
- *		Returns nothing of interest.
  * ----------------------------------------------------------------
  */
 void
 ExecEndBitmapAnd(BitmapAndState *node)
 {
-	PlanState **bitmapplans;
-	int			nplans;
-	int			i;
-
-	/*
-	 * get information from the node
-	 */
-	bitmapplans = node->bitmapplans;
-	nplans = node->nplans;
-
-	/*
-	 * shut down each of the subscans (that we've initialized)
-	 */
-	for (i = 0; i < nplans; i++)
-	{
-		if (bitmapplans[i])
-			ExecEndNode(bitmapplans[i]);
-	}
+	/* Nothing to do as the nodes in bitmapplans are closed separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index f35df0b8bf..d58ee4f4e1 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -667,10 +667,7 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 		ExecClearTuple(node->ss.ps.ps_ResultTupleSlot);
 	ExecClearTuple(node->ss.ss_ScanTupleSlot);
 
-	/*
-	 * close down subplans
-	 */
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 
 	/*
 	 * release bitmaps and buffers if any
diff --git a/src/backend/executor/nodeBitmapOr.c b/src/backend/executor/nodeBitmapOr.c
index 0bf8af9652..736852a0ae 100644
--- a/src/backend/executor/nodeBitmapOr.c
+++ b/src/backend/executor/nodeBitmapOr.c
@@ -186,33 +186,12 @@ MultiExecBitmapOr(BitmapOrState *node)
 
 /* ----------------------------------------------------------------
  *		ExecEndBitmapOr
- *
- *		Shuts down the subscans of the BitmapOr node.
- *
- *		Returns nothing of interest.
  * ----------------------------------------------------------------
  */
 void
 ExecEndBitmapOr(BitmapOrState *node)
 {
-	PlanState **bitmapplans;
-	int			nplans;
-	int			i;
-
-	/*
-	 * get information from the node
-	 */
-	bitmapplans = node->bitmapplans;
-	nplans = node->nplans;
-
-	/*
-	 * shut down each of the subscans (that we've initialized)
-	 */
-	for (i = 0; i < nplans; i++)
-	{
-		if (bitmapplans[i])
-			ExecEndNode(bitmapplans[i]);
-	}
+	/* Nothing to do as the nodes in bitmapplans are closed separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index c2139acca0..e6616dd718 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -309,9 +309,7 @@ ExecEndForeignScan(ForeignScanState *node)
 	else
 		node->fdwroutine->EndForeignScan(node);
 
-	/* Shut down any outer plan. */
-	if (outerPlanState(node))
-		ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 
 	/* Free the exprcontext */
 	ExecFreeExprContext(&node->ss.ps);
diff --git a/src/backend/executor/nodeGather.c b/src/backend/executor/nodeGather.c
index 307fc10eea..f7a69f185b 100644
--- a/src/backend/executor/nodeGather.c
+++ b/src/backend/executor/nodeGather.c
@@ -248,7 +248,7 @@ ExecGather(PlanState *pstate)
 void
 ExecEndGather(GatherState *node)
 {
-	ExecEndNode(outerPlanState(node));	/* let children clean up first */
+	/* outerPlan is closed separately. */
 	ExecShutdownGather(node);
 	ExecFreeExprContext(&node->ps);
 	if (node->ps.ps_ResultTupleSlot)
diff --git a/src/backend/executor/nodeGatherMerge.c b/src/backend/executor/nodeGatherMerge.c
index 9d5e1a46e9..d357ff0c47 100644
--- a/src/backend/executor/nodeGatherMerge.c
+++ b/src/backend/executor/nodeGatherMerge.c
@@ -288,7 +288,7 @@ ExecGatherMerge(PlanState *pstate)
 void
 ExecEndGatherMerge(GatherMergeState *node)
 {
-	ExecEndNode(outerPlanState(node));	/* let children clean up first */
+	/* outerPlan is closed separately. */
 	ExecShutdownGatherMerge(node);
 	ExecFreeExprContext(&node->ps);
 	if (node->ps.ps_ResultTupleSlot)
diff --git a/src/backend/executor/nodeGroup.c b/src/backend/executor/nodeGroup.c
index 25a1618952..2badcc7e60 100644
--- a/src/backend/executor/nodeGroup.c
+++ b/src/backend/executor/nodeGroup.c
@@ -226,15 +226,12 @@ ExecInitGroup(Group *node, EState *estate, int eflags)
 void
 ExecEndGroup(GroupState *node)
 {
-	PlanState  *outerPlan;
-
 	ExecFreeExprContext(&node->ss.ps);
 
 	/* clean up tuple table */
 	ExecClearTuple(node->ss.ss_ScanTupleSlot);
 
-	outerPlan = outerPlanState(node);
-	ExecEndNode(outerPlan);
+	/* outerPlan is closed separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 8b5c35b82b..edd2324384 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -413,18 +413,12 @@ ExecInitHash(Hash *node, EState *estate, int eflags)
 void
 ExecEndHash(HashState *node)
 {
-	PlanState  *outerPlan;
-
 	/*
 	 * free exprcontext
 	 */
 	ExecFreeExprContext(&node->ps);
 
-	/*
-	 * shut down the subplan
-	 */
-	outerPlan = outerPlanState(node);
-	ExecEndNode(outerPlan);
+	/* outerPlan is closed separately. */
 }
 
 
diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index 980746128b..8078d7f229 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -879,11 +879,7 @@ ExecEndHashJoin(HashJoinState *node)
 	ExecClearTuple(node->hj_OuterTupleSlot);
 	ExecClearTuple(node->hj_HashTupleSlot);
 
-	/*
-	 * clean up subtrees
-	 */
-	ExecEndNode(outerPlanState(node));
-	ExecEndNode(innerPlanState(node));
+	/* outerPlan and innerPlan are closed separately. */
 }
 
 /*
diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c
index 7683e3341c..52b146cfb8 100644
--- a/src/backend/executor/nodeIncrementalSort.c
+++ b/src/backend/executor/nodeIncrementalSort.c
@@ -1101,10 +1101,7 @@ ExecEndIncrementalSort(IncrementalSortState *node)
 		node->prefixsort_state = NULL;
 	}
 
-	/*
-	 * Shut down the subplan.
-	 */
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 
 	SO_printf("ExecEndIncrementalSort: sort node shutdown\n");
 }
diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c
index 425fbfc405..a75099dd73 100644
--- a/src/backend/executor/nodeLimit.c
+++ b/src/backend/executor/nodeLimit.c
@@ -535,7 +535,7 @@ void
 ExecEndLimit(LimitState *node)
 {
 	ExecFreeExprContext(&node->ps);
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 }
 
 
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index e459971d32..55de8d3d65 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -386,7 +386,7 @@ ExecEndLockRows(LockRowsState *node)
 {
 	/* We may have shut down EPQ already, but no harm in another call */
 	EvalPlanQualEnd(&node->lr_epqstate);
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 }
 
 
diff --git a/src/backend/executor/nodeMaterial.c b/src/backend/executor/nodeMaterial.c
index 09632678b0..ef04e9a8e7 100644
--- a/src/backend/executor/nodeMaterial.c
+++ b/src/backend/executor/nodeMaterial.c
@@ -251,10 +251,7 @@ ExecEndMaterial(MaterialState *node)
 		tuplestore_end(node->tuplestorestate);
 	node->tuplestorestate = NULL;
 
-	/*
-	 * shut down the subplan
-	 */
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 }
 
 /* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c
index 4f04269e26..61578d4b5c 100644
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -1100,10 +1100,7 @@ ExecEndMemoize(MemoizeState *node)
 	 */
 	ExecFreeExprContext(&node->ss.ps);
 
-	/*
-	 * shut down the subplan
-	 */
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c
index 21b5726e6e..8aa64944c9 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -310,30 +310,12 @@ heap_compare_slots(Datum a, Datum b, void *arg)
 
 /* ----------------------------------------------------------------
  *		ExecEndMergeAppend
- *
- *		Shuts down the subscans of the MergeAppend node.
- *
- *		Returns nothing of interest.
  * ----------------------------------------------------------------
  */
 void
 ExecEndMergeAppend(MergeAppendState *node)
 {
-	PlanState **mergeplans;
-	int			nplans;
-	int			i;
-
-	/*
-	 * get information from the node
-	 */
-	mergeplans = node->mergeplans;
-	nplans = node->ms_nplans;
-
-	/*
-	 * shut down each of the subscans
-	 */
-	for (i = 0; i < nplans; i++)
-		ExecEndNode(mergeplans[i]);
+	/* Nothing to do as the nodes in mergeplans are closed separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c
index 00f96d045e..7b530d9088 100644
--- a/src/backend/executor/nodeMergejoin.c
+++ b/src/backend/executor/nodeMergejoin.c
@@ -1654,11 +1654,7 @@ ExecEndMergeJoin(MergeJoinState *node)
 	ExecClearTuple(node->js.ps.ps_ResultTupleSlot);
 	ExecClearTuple(node->mj_MarkedTupleSlot);
 
-	/*
-	 * shut down the subplans
-	 */
-	ExecEndNode(innerPlanState(node));
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan and innerPlan are closed separately. */
 
 	MJ1_printf("ExecEndMergeJoin: %s\n",
 			   "node processing ended");
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 2a5fec8d01..bdbaa4753b 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4397,7 +4397,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 /* ----------------------------------------------------------------
  *		ExecEndModifyTable
  *
- *		Shuts down the plan.
+ *		Releases ModifyTable resources.
  *
  *		Returns nothing of interest.
  * ----------------------------------------------------------------
@@ -4461,10 +4461,7 @@ ExecEndModifyTable(ModifyTableState *node)
 	 */
 	EvalPlanQualEnd(&node->mt_epqstate);
 
-	/*
-	 * shut down subplan
-	 */
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeNestloop.c b/src/backend/executor/nodeNestloop.c
index b3d52e69ec..5cfb50a366 100644
--- a/src/backend/executor/nodeNestloop.c
+++ b/src/backend/executor/nodeNestloop.c
@@ -374,11 +374,7 @@ ExecEndNestLoop(NestLoopState *node)
 	 */
 	ExecClearTuple(node->js.ps.ps_ResultTupleSlot);
 
-	/*
-	 * close down subplans
-	 */
-	ExecEndNode(outerPlanState(node));
-	ExecEndNode(innerPlanState(node));
+	/* outerPlan and innerPlan are closed separately. */
 
 	NL1_printf("ExecEndNestLoop: %s\n",
 			   "node processing ended");
diff --git a/src/backend/executor/nodeProjectSet.c b/src/backend/executor/nodeProjectSet.c
index f6ff3dc44c..4a388220ee 100644
--- a/src/backend/executor/nodeProjectSet.c
+++ b/src/backend/executor/nodeProjectSet.c
@@ -330,10 +330,7 @@ ExecEndProjectSet(ProjectSetState *node)
 	 */
 	ExecClearTuple(node->ps.ps_ResultTupleSlot);
 
-	/*
-	 * shut down subplans
-	 */
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c
index e781003934..aee31c7139 100644
--- a/src/backend/executor/nodeRecursiveunion.c
+++ b/src/backend/executor/nodeRecursiveunion.c
@@ -281,11 +281,7 @@ ExecEndRecursiveUnion(RecursiveUnionState *node)
 	if (node->tableContext)
 		MemoryContextDelete(node->tableContext);
 
-	/*
-	 * close down subplans
-	 */
-	ExecEndNode(outerPlanState(node));
-	ExecEndNode(innerPlanState(node));
+	/* outerPlan and innerPlan are closed separately. */
 }
 
 /* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeResult.c b/src/backend/executor/nodeResult.c
index 4219712d30..a100b144be 100644
--- a/src/backend/executor/nodeResult.c
+++ b/src/backend/executor/nodeResult.c
@@ -250,10 +250,7 @@ ExecEndResult(ResultState *node)
 	 */
 	ExecClearTuple(node->ps.ps_ResultTupleSlot);
 
-	/*
-	 * shut down subplans
-	 */
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 }
 
 void
diff --git a/src/backend/executor/nodeSetOp.c b/src/backend/executor/nodeSetOp.c
index 4bc2406b89..f7db9a3415 100644
--- a/src/backend/executor/nodeSetOp.c
+++ b/src/backend/executor/nodeSetOp.c
@@ -590,7 +590,7 @@ ExecEndSetOp(SetOpState *node)
 		MemoryContextDelete(node->tableContext);
 	ExecFreeExprContext(&node->ps);
 
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 }
 
 
diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c
index c6c72c6e67..078d041c40 100644
--- a/src/backend/executor/nodeSort.c
+++ b/src/backend/executor/nodeSort.c
@@ -317,10 +317,7 @@ ExecEndSort(SortState *node)
 		tuplesort_end((Tuplesortstate *) node->tuplesortstate);
 	node->tuplesortstate = NULL;
 
-	/*
-	 * shut down the subplan
-	 */
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 
 	SO1_printf("ExecEndSort: %s\n",
 			   "sort node shutdown");
diff --git a/src/backend/executor/nodeSubqueryscan.c b/src/backend/executor/nodeSubqueryscan.c
index 42471bfc04..bc55a82fc3 100644
--- a/src/backend/executor/nodeSubqueryscan.c
+++ b/src/backend/executor/nodeSubqueryscan.c
@@ -179,10 +179,7 @@ ExecEndSubqueryScan(SubqueryScanState *node)
 		ExecClearTuple(node->ss.ps.ps_ResultTupleSlot);
 	ExecClearTuple(node->ss.ss_ScanTupleSlot);
 
-	/*
-	 * close down subquery
-	 */
-	ExecEndNode(node->subplan);
+	/* subplan is closed separately. */
 }
 
 /* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeUnique.c b/src/backend/executor/nodeUnique.c
index 45035d74fa..50babacdc8 100644
--- a/src/backend/executor/nodeUnique.c
+++ b/src/backend/executor/nodeUnique.c
@@ -173,7 +173,7 @@ ExecEndUnique(UniqueState *node)
 
 	ExecFreeExprContext(&node->ps);
 
-	ExecEndNode(outerPlanState(node));
+	/* outerPlan is closed separately. */
 }
 
 
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 310ac23e3a..648cdadc32 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -2681,7 +2681,6 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 void
 ExecEndWindowAgg(WindowAggState *node)
 {
-	PlanState  *outerPlan;
 	int			i;
 
 	release_partition(node);
@@ -2714,8 +2713,7 @@ ExecEndWindowAgg(WindowAggState *node)
 	pfree(node->perfunc);
 	pfree(node->peragg);
 
-	outerPlan = outerPlanState(node);
-	ExecEndNode(outerPlan);
+	/* outerPlan is closed separately. */
 }
 
 /* -----------------
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index cb714f4a19..233fb6b4f9 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -671,6 +671,8 @@ typedef struct EState
 
 	List	   *es_exprcontexts;	/* List of ExprContexts within EState */
 
+	List	   *es_planstate_nodes; /* "flat" list of PlanState nodes */
+
 	List	   *es_subplanstates;	/* List of PlanState for SubPlans */
 
 	List	   *es_auxmodifytables; /* List of secondary ModifyTableStates */
-- 
2.35.3