mirror of
https://git.postgresql.org/git/postgresql.git
synced 2025-01-12 18:34:36 +08:00
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.
This commit is contained in:
parent
85a373b1f0
commit
4a1bffa066
@ -26,7 +26,7 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.280 2006/10/04 00:29:52 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.280.2.1 2006/12/26 21:37:28 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -2223,6 +2223,10 @@ EvalPlanQualStart(evalPlanQual *epq, EState *estate, evalPlanQual *priorepq)
|
||||
|
||||
rtsize = list_length(estate->es_range_table);
|
||||
|
||||
/*
|
||||
* It's tempting to think about using CreateSubExecutorState here, but
|
||||
* at present we can't because of memory leakage concerns ...
|
||||
*/
|
||||
epq->estate = epqstate = CreateExecutorState();
|
||||
|
||||
oldcontext = MemoryContextSwitchTo(epqstate->es_query_cxt);
|
||||
|
@ -8,13 +8,14 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.140 2006/10/04 00:29:52 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.140.2.1 2006/12/26 21:37:28 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
/*
|
||||
* INTERFACE ROUTINES
|
||||
* CreateExecutorState Create/delete executor working state
|
||||
* CreateSubExecutorState
|
||||
* FreeExecutorState
|
||||
* CreateExprContext
|
||||
* CreateStandaloneExprContext
|
||||
@ -65,6 +66,8 @@ int NIndexTupleInserted;
|
||||
int NIndexTupleProcessed;
|
||||
|
||||
|
||||
static EState *InternalCreateExecutorState(MemoryContext qcontext,
|
||||
bool is_subquery);
|
||||
static void ShutdownExprContext(ExprContext *econtext);
|
||||
|
||||
|
||||
@ -149,9 +152,7 @@ DisplayTupleCount(FILE *statfp)
|
||||
EState *
|
||||
CreateExecutorState(void)
|
||||
{
|
||||
EState *estate;
|
||||
MemoryContext qcontext;
|
||||
MemoryContext oldcontext;
|
||||
|
||||
/*
|
||||
* Create the per-query context for this Executor run.
|
||||
@ -162,6 +163,37 @@ CreateExecutorState(void)
|
||||
ALLOCSET_DEFAULT_INITSIZE,
|
||||
ALLOCSET_DEFAULT_MAXSIZE);
|
||||
|
||||
return InternalCreateExecutorState(qcontext, false);
|
||||
}
|
||||
|
||||
/* ----------------
|
||||
* CreateSubExecutorState
|
||||
*
|
||||
* Create and initialize an EState node for a sub-query.
|
||||
*
|
||||
* Ideally, sub-queries probably shouldn't have their own EState at all,
|
||||
* but right now this is necessary because they have their own rangetables
|
||||
* and we access the rangetable via the EState. It is critical that a
|
||||
* sub-query share the parent's es_query_cxt, else structures allocated by
|
||||
* the sub-query (especially its result tuple descriptor) may disappear
|
||||
* too soon during executor shutdown.
|
||||
* ----------------
|
||||
*/
|
||||
EState *
|
||||
CreateSubExecutorState(EState *parent_estate)
|
||||
{
|
||||
return InternalCreateExecutorState(parent_estate->es_query_cxt, true);
|
||||
}
|
||||
|
||||
/*
|
||||
* Guts of CreateExecutorState/CreateSubExecutorState
|
||||
*/
|
||||
static EState *
|
||||
InternalCreateExecutorState(MemoryContext qcontext, bool is_subquery)
|
||||
{
|
||||
EState *estate;
|
||||
MemoryContext oldcontext;
|
||||
|
||||
/*
|
||||
* Make the EState node within the per-query context. This way, we don't
|
||||
* need a separate pfree() operation for it at shutdown.
|
||||
@ -200,6 +232,8 @@ CreateExecutorState(void)
|
||||
estate->es_lastoid = InvalidOid;
|
||||
estate->es_rowMarks = NIL;
|
||||
|
||||
estate->es_is_subquery = is_subquery;
|
||||
|
||||
estate->es_instrument = false;
|
||||
estate->es_select_into = false;
|
||||
estate->es_into_oids = false;
|
||||
@ -258,9 +292,12 @@ FreeExecutorState(EState *estate)
|
||||
|
||||
/*
|
||||
* Free the per-query memory context, thereby releasing all working
|
||||
* memory, including the EState node itself.
|
||||
* memory, including the EState node itself. In a subquery, we don't
|
||||
* do this, leaving the memory cleanup to happen when the topmost query
|
||||
* is closed down.
|
||||
*/
|
||||
MemoryContextDelete(estate->es_query_cxt);
|
||||
if (!estate->es_is_subquery)
|
||||
MemoryContextDelete(estate->es_query_cxt);
|
||||
}
|
||||
|
||||
/* ----------------
|
||||
|
@ -7,7 +7,7 @@
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.80 2006/10/04 00:29:53 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.80.2.1 2006/12/26 21:37:28 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -635,7 +635,6 @@ ExecInitSubPlan(SubPlanState *node, EState *estate, int eflags)
|
||||
{
|
||||
SubPlan *subplan = (SubPlan *) node->xprstate.expr;
|
||||
EState *sp_estate;
|
||||
MemoryContext oldcontext;
|
||||
|
||||
/*
|
||||
* Do access checking on the rangetable entries in the subquery.
|
||||
@ -661,15 +660,13 @@ ExecInitSubPlan(SubPlanState *node, EState *estate, int eflags)
|
||||
* create an EState for the subplan
|
||||
*
|
||||
* 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);
|
||||
node->sub_estate = sp_estate;
|
||||
|
||||
oldcontext = MemoryContextSwitchTo(sp_estate->es_query_cxt);
|
||||
|
||||
sp_estate->es_range_table = subplan->rtable;
|
||||
sp_estate->es_param_list_info = estate->es_param_list_info;
|
||||
sp_estate->es_param_exec_vals = estate->es_param_exec_vals;
|
||||
@ -694,8 +691,6 @@ ExecInitSubPlan(SubPlanState *node, EState *estate, int eflags)
|
||||
|
||||
node->needShutdown = true; /* now we need to shutdown the subplan */
|
||||
|
||||
MemoryContextSwitchTo(oldcontext);
|
||||
|
||||
/*
|
||||
* If this plan is un-correlated or undirect correlated one and want to
|
||||
* set params for parent plan then mark parameters as needing evaluation.
|
||||
@ -1036,11 +1031,7 @@ ExecEndSubPlan(SubPlanState *node)
|
||||
{
|
||||
if (node->needShutdown)
|
||||
{
|
||||
MemoryContext oldcontext;
|
||||
|
||||
oldcontext = MemoryContextSwitchTo(node->sub_estate->es_query_cxt);
|
||||
ExecEndPlan(node->planstate, node->sub_estate);
|
||||
MemoryContextSwitchTo(oldcontext);
|
||||
FreeExecutorState(node->sub_estate);
|
||||
node->sub_estate = NULL;
|
||||
node->planstate = NULL;
|
||||
|
@ -12,7 +12,7 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/executor/nodeSubqueryscan.c,v 1.32.2.1 2006/12/26 19:26:56 tgl Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/executor/nodeSubqueryscan.c,v 1.32.2.2 2006/12/26 21:37:28 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;
|
||||
}
|
||||
|
@ -7,7 +7,7 @@
|
||||
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.130 2006/10/04 00:30:08 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.130.2.1 2006/12/26 21:37:28 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -223,6 +223,7 @@ extern void end_tup_output(TupOutputState *tstate);
|
||||
* prototypes from functions in execUtils.c
|
||||
*/
|
||||
extern EState *CreateExecutorState(void);
|
||||
extern EState *CreateSubExecutorState(EState *parent_estate);
|
||||
extern void FreeExecutorState(EState *estate);
|
||||
extern ExprContext *CreateExprContext(EState *estate);
|
||||
extern ExprContext *CreateStandaloneExprContext(void);
|
||||
|
@ -7,7 +7,7 @@
|
||||
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.161 2006/09/28 20:51:42 tgl Exp $
|
||||
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.161.2.1 2006/12/26 21:37:28 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -323,6 +323,8 @@ typedef struct EState
|
||||
Oid es_lastoid; /* last oid processed (by INSERT) */
|
||||
List *es_rowMarks; /* not good place, but there is no other */
|
||||
|
||||
bool es_is_subquery; /* true if subquery (es_query_cxt not mine) */
|
||||
|
||||
bool es_instrument; /* true requests runtime instrumentation */
|
||||
bool es_select_into; /* true if doing SELECT INTO */
|
||||
bool es_into_oids; /* true to generate OIDs in SELECT INTO */
|
||||
|
Loading…
Reference in New Issue
Block a user