From 13e4d0b8d381c1d0e507a09e2350150d4f1bc671 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Wed, 6 Sep 2023 17:53:16 +0900
Subject: [PATCH v47 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.

Reviewed-by: Robert Haas
---
 src/backend/executor/nodeBitmapHeapscan.c  |  3 ++-
 src/backend/executor/nodeForeignscan.c     | 13 +++++++-----
 src/backend/executor/nodeIncrementalSort.c |  6 ++++--
 src/backend/executor/nodeMemoize.c         |  1 +
 src/backend/executor/nodeRecursiveunion.c  |  6 ++++--
 src/backend/executor/nodeWindowAgg.c       | 24 ++++++++++++++--------
 6 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 2db0acfc76..c8c1c9d88e 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 != NULL)
+		table_endscan(scanDesc);
 }
 
 /* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 73913ebb18..298ea59a1e 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -301,13 +301,16 @@ ExecEndForeignScan(ForeignScanState *node)
 	EState	   *estate = node->ss.ps.state;
 
 	/* Let the FDW shut down */
-	if (plan->operation != CMD_SELECT)
+	if (node->fdwroutine != NULL)
 	{
-		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))
diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c
index cd094a190c..544e64dfab 100644
--- a/src/backend/executor/nodeIncrementalSort.c
+++ b/src/backend/executor/nodeIncrementalSort.c
@@ -1079,8 +1079,10 @@ ExecEndIncrementalSort(IncrementalSortState *node)
 {
 	SO_printf("ExecEndIncrementalSort: shutting down sort node\n");
 
-	ExecDropSingleTupleTableSlot(node->group_pivot);
-	ExecDropSingleTupleTableSlot(node->transfer_tuple);
+	if (node->group_pivot != NULL)
+		ExecDropSingleTupleTableSlot(node->group_pivot);
+	if (node->transfer_tuple != NULL)
+		ExecDropSingleTupleTableSlot(node->transfer_tuple);
 
 	/*
 	 * Release tuplesort resources.
diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c
index 94bf479287..81f2acde5e 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 != NULL)
 	{
 		int			count;
 		uint64		mem = 0;
diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c
index e781003934..54cd6f2347 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 != NULL)
+		tuplestore_end(node->working_table);
+	if (node->intermediate_table != NULL)
+		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..f5170799e4 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 != NULL)
+		MemoryContextResetAndDeleteChildren(winstate->partcontext);
+	if (winstate->aggcontext != NULL)
+		MemoryContextResetAndDeleteChildren(winstate->aggcontext);
 	for (i = 0; i < winstate->numaggs; i++)
 	{
-		if (winstate->peragg[i].aggcontext != winstate->aggcontext)
+		if (winstate->peragg[i].aggcontext != NULL &&
+			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 != NULL &&
+			node->peragg[i].aggcontext != node->aggcontext)
 			MemoryContextDelete(node->peragg[i].aggcontext);
 	}
-	MemoryContextDelete(node->partcontext);
-	MemoryContextDelete(node->aggcontext);
+	if (node->partcontext != NULL)
+		MemoryContextDelete(node->partcontext);
+	if (node->aggcontext != NULL)
+		MemoryContextDelete(node->aggcontext);
 
-	pfree(node->perfunc);
-	pfree(node->peragg);
+	if (node->perfunc != NULL)
+		pfree(node->perfunc);
+	if (node->peragg != NULL)
+		pfree(node->peragg);
 
 	outerPlan = outerPlanState(node);
 	ExecEndNode(outerPlan);
-- 
2.35.3

