Fix failure due to accessing an already-freed tuple descriptor in a plan

involving HashAggregate over SubqueryScan (this is the known case, there
may well be more).  The bug is only latent in releases before 8.2 since they
didn't try to access tupletable slots' descriptors during ExecDropTupleTable.
The least bogus fix seems to be to make subqueries share the parent query's
memory context, so that tupdescs they create will have the same lifespan as
those of the parent query.  There are comments in the code envisioning going
even further by not having a separate child EState at all, but that will
require rethinking executor access to range tables, which I don't want to
tackle right now.  Per bug report from Jean-Pierre Pelletier.

1 files changed, 6 insertions, 28 deletions

diff --git a/src/backend/executor/nodeSubqueryscan.c b/src/backend/executor/nodeSubqueryscan.c
index 45dfdb838d5..f715676d6e2 100644
--- a/src/backend/executor/nodeSubqueryscan.c
+++ b/src/backend/executor/nodeSubqueryscan.c
@@ -12,7 +12,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeSubqueryscan.c,v 1.33 2006/12/26 19:26:46 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeSubqueryscan.c,v 1.34 2006/12/26 21:37:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -49,7 +49,6 @@ SubqueryNext(SubqueryScanState *node)
 	EState	   *estate;
 	ScanDirection direction;
 	TupleTableSlot *slot;
-	MemoryContext oldcontext;
 
 	/*
 	 * get information from the estate and scan state
@@ -63,17 +62,12 @@ SubqueryNext(SubqueryScanState *node)
 	 */
 
 	/*
-	 * Get the next tuple from the sub-query.  We have to be careful to run it
-	 * in its appropriate memory context.
+	 * Get the next tuple from the sub-query.
 	 */
 	node->sss_SubEState->es_direction = direction;
 
-	oldcontext = MemoryContextSwitchTo(node->sss_SubEState->es_query_cxt);
-
 	slot = ExecProcNode(node->subplan);
 
-	MemoryContextSwitchTo(oldcontext);
-
 	/*
 	 * We just overwrite our ScanTupleSlot with the subplan's result slot,
 	 * rather than expending the cycles for ExecCopySlot().
@@ -112,7 +106,6 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags)
 	SubqueryScanState *subquerystate;
 	RangeTblEntry *rte;
 	EState	   *sp_estate;
-	MemoryContext oldcontext;
 
 	/* check for unsupported flags */
 	Assert(!(eflags & EXEC_FLAG_MARK));
@@ -170,15 +163,13 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags)
 
 	/*
 	 * The subquery needs its own EState because it has its own rangetable. It
-	 * shares our Param ID space, however.	XXX if rangetable access were done
-	 * differently, the subquery could share our EState, which would eliminate
-	 * some thrashing about in this module...
+	 * shares our Param ID space and es_query_cxt, however.  XXX if rangetable
+	 * access were done differently, the subquery could share our EState,
+	 * which would eliminate some thrashing about in this module...
 	 */
-	sp_estate = CreateExecutorState();
+	sp_estate = CreateSubExecutorState(estate);
 	subquerystate->sss_SubEState = sp_estate;
 
-	oldcontext = MemoryContextSwitchTo(sp_estate->es_query_cxt);
-
 	sp_estate->es_range_table = rte->subquery->rtable;
 	sp_estate->es_param_list_info = estate->es_param_list_info;
 	sp_estate->es_param_exec_vals = estate->es_param_exec_vals;
@@ -193,8 +184,6 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags)
 	 */
 	subquerystate->subplan = ExecInitNode(node->subplan, sp_estate, eflags);
 
-	MemoryContextSwitchTo(oldcontext);
-
 	subquerystate->ss.ps.ps_TupFromTlist = false;
 
 	/*
@@ -235,8 +224,6 @@ ExecCountSlotsSubqueryScan(SubqueryScan *node)
 void
 ExecEndSubqueryScan(SubqueryScanState *node)
 {
-	MemoryContext oldcontext;
-
 	/*
 	 * Free the exprcontext
 	 */
@@ -251,12 +238,8 @@ ExecEndSubqueryScan(SubqueryScanState *node)
 	/*
 	 * close down subquery
 	 */
-	oldcontext = MemoryContextSwitchTo(node->sss_SubEState->es_query_cxt);
-
 	ExecEndPlan(node->subplan, node->sss_SubEState);
 
-	MemoryContextSwitchTo(oldcontext);
-
 	FreeExecutorState(node->sss_SubEState);
 }
 
@@ -270,12 +253,9 @@ void
 ExecSubqueryReScan(SubqueryScanState *node, ExprContext *exprCtxt)
 {
 	EState	   *estate;
-	MemoryContext oldcontext;
 
 	estate = node->ss.ps.state;
 
-	oldcontext = MemoryContextSwitchTo(node->sss_SubEState->es_query_cxt);
-
 	/*
 	 * ExecReScan doesn't know about my subplan, so I have to do
 	 * changed-parameter signaling myself.	This is just as well, because the
@@ -291,8 +271,6 @@ ExecSubqueryReScan(SubqueryScanState *node, ExprContext *exprCtxt)
 	if (node->subplan->chgParam == NULL)
 		ExecReScan(node->subplan, NULL);
 
-	MemoryContextSwitchTo(oldcontext);
-
 	node->ss.ss_ScanTupleSlot = NULL;
 	node->ss.ps.ps_TupFromTlist = false;
 }