v46-0002-Check-pointer-NULLness-before-cleanup-in-ExecEnd.patch

From 3dfe81f48a58a92e8c81469600d3502f18a8b137 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 1 Sep 2023 22:05:35 +0900
Subject: [PATCH v46 2/8] Check pointer NULLness before cleanup in ExecEnd*
 routines

Many routines already perform this check, but a few instances remain.

Currently, these NULLness checks might seem redundant since ExecEnd*
routines operate under the assumption that their matching ExecInit*
routine would have fully executed, ensuring pointers are set. However,
a forthcoming patch will modify ExecInit* routines to sometimes exit
early, potentially leaving some pointers in an undetermined state.
---
 src/backend/executor/nodeAgg.c            |  3 ++-
 src/backend/executor/nodeBitmapHeapscan.c |  3 ++-
 src/backend/executor/nodeForeignscan.c    | 21 ++++++++------------
 src/backend/executor/nodeMemoize.c        |  1 +
 src/backend/executor/nodeRecursiveunion.c |  6 ++++--
 src/backend/executor/nodeWindowAgg.c      | 24 +++++++++++++++--------
 6 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index f154f28902..aac9e9fc80 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -3150,7 +3150,8 @@ hashagg_reset_spill_state(AggState *aggstate)
 	}
 
 	/* free batches */
-	list_free_deep(aggstate->hash_batches);
+	if (aggstate->hash_batches)
+		list_free_deep(aggstate->hash_batches);
 	aggstate->hash_batches = NIL;
 
 	/* close tape set */
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 2db0acfc76..ffa51c06b4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -681,7 +681,8 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
 }
 
 /* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index c2139acca0..d5aaa983f7 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -301,25 +301,20 @@ ExecEndForeignScan(ForeignScanState *node)
 	EState	   *estate = node->ss.ps.state;
 
 	/* Let the FDW shut down */
-	if (plan->operation != CMD_SELECT)
+	if (node->fdwroutine)
 	{
-		if (estate->es_epq_active == NULL)
-			node->fdwroutine->EndDirectModify(node);
+		if (plan->operation != CMD_SELECT)
+		{
+			if (estate->es_epq_active == NULL)
+				node->fdwroutine->EndDirectModify(node);
+		}
+		else
+			node->fdwroutine->EndForeignScan(node);
 	}
-	else
-		node->fdwroutine->EndForeignScan(node);
 
 	/* Shut down any outer plan. */
 	if (outerPlanState(node))
 		ExecEndNode(outerPlanState(node));
-
-	/* Free the exprcontext */
-	ExecFreeExprContext(&node->ss.ps);
-
-	/* clean out the tuple table */
-	if (node->ss.ps.ps_ResultTupleSlot)
-		ExecClearTuple(node->ss.ps.ps_ResultTupleSlot);
-	ExecClearTuple(node->ss.ss_ScanTupleSlot);
 }
 
 /* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c
index 94bf479287..5352ca10c8 100644
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -1043,6 +1043,7 @@ ExecEndMemoize(MemoizeState *node)
 {
 #ifdef USE_ASSERT_CHECKING
 	/* Validate the memory accounting code is correct in assert builds. */
+	if (node->hashtable)
 	{
 		int			count;
 		uint64		mem = 0;
diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c
index e781003934..3dfcb4cafb 100644
--- a/src/backend/executor/nodeRecursiveunion.c
+++ b/src/backend/executor/nodeRecursiveunion.c
@@ -272,8 +272,10 @@ void
 ExecEndRecursiveUnion(RecursiveUnionState *node)
 {
 	/* Release tuplestores */
-	tuplestore_end(node->working_table);
-	tuplestore_end(node->intermediate_table);
+	if (node->working_table)
+		tuplestore_end(node->working_table);
+	if (node->intermediate_table)
+		tuplestore_end(node->intermediate_table);
 
 	/* free subsidiary stuff including hashtable */
 	if (node->tempContext)
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 77724a6daa..3849d2f847 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -1351,11 +1351,14 @@ release_partition(WindowAggState *winstate)
 	 * any aggregate temp data).  We don't rely on retail pfree because some
 	 * aggregates might have allocated data we don't have direct pointers to.
 	 */
-	MemoryContextResetAndDeleteChildren(winstate->partcontext);
-	MemoryContextResetAndDeleteChildren(winstate->aggcontext);
+	if (winstate->partcontext)
+		MemoryContextResetAndDeleteChildren(winstate->partcontext);
+	if (winstate->aggcontext)
+		MemoryContextResetAndDeleteChildren(winstate->aggcontext);
 	for (i = 0; i < winstate->numaggs; i++)
 	{
-		if (winstate->peragg[i].aggcontext != winstate->aggcontext)
+		if (winstate->peragg[i].aggcontext &&
+			winstate->peragg[i].aggcontext != winstate->aggcontext)
 			MemoryContextResetAndDeleteChildren(winstate->peragg[i].aggcontext);
 	}
 
@@ -2688,14 +2691,19 @@ ExecEndWindowAgg(WindowAggState *node)
 
 	for (i = 0; i < node->numaggs; i++)
 	{
-		if (node->peragg[i].aggcontext != node->aggcontext)
+		if (node->peragg[i].aggcontext &&
+			node->peragg[i].aggcontext != node->aggcontext)
 			MemoryContextDelete(node->peragg[i].aggcontext);
 	}
-	MemoryContextDelete(node->partcontext);
-	MemoryContextDelete(node->aggcontext);
+	if (node->partcontext)
+		MemoryContextDelete(node->partcontext);
+	if (node->aggcontext)
+		MemoryContextDelete(node->aggcontext);
 
-	pfree(node->perfunc);
-	pfree(node->peragg);
+	if (node->perfunc)
+		pfree(node->perfunc);
+	if (node->peragg)
+		pfree(node->peragg);
 
 	outerPlan = outerPlanState(node);
 	ExecEndNode(outerPlan);
-- 
2.35.3