Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0cbc5b1

Browse files
committed
Fix failure due to accessing an already-freed tuple descriptor in a plan
involving HashAggregate over SubqueryScan (this is the known case, theremay well be more). The bug is only latent in releases before 8.2 since theydidn't try to access tupletable slots' descriptors during ExecDropTupleTable.The least bogus fix seems to be to make subqueries share the parent query'smemory context, so that tupdescs they create will have the same lifespan asthose of the parent query. There are comments in the code envisioning goingeven further by not having a separate child EState at all, but that willrequire rethinking executor access to range tables, which I don't want totackle right now. Per bug report from Jean-Pierre Pelletier.
1 parent6899646 commit0cbc5b1

File tree

6 files changed

+63
-50
lines changed

6 files changed

+63
-50
lines changed

‎src/backend/executor/execMain.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
*
2828
* IDENTIFICATION
29-
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.281 2006/12/04 02:06:55 tgl Exp $
29+
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.282 2006/12/26 21:37:19 tgl Exp $
3030
*
3131
*-------------------------------------------------------------------------
3232
*/
@@ -2247,6 +2247,10 @@ EvalPlanQualStart(evalPlanQual *epq, EState *estate, evalPlanQual *priorepq)
22472247

22482248
rtsize=list_length(estate->es_range_table);
22492249

2250+
/*
2251+
* It's tempting to think about using CreateSubExecutorState here, but
2252+
* at present we can't because of memory leakage concerns ...
2253+
*/
22502254
epq->estate=epqstate=CreateExecutorState();
22512255

22522256
oldcontext=MemoryContextSwitchTo(epqstate->es_query_cxt);

‎src/backend/executor/execUtils.c

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.140 2006/10/04 00:29:52 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.141 2006/12/26 21:37:19 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515
/*
1616
* INTERFACE ROUTINES
1717
*CreateExecutorStateCreate/delete executor working state
18+
*CreateSubExecutorState
1819
*FreeExecutorState
1920
*CreateExprContext
2021
*CreateStandaloneExprContext
@@ -65,6 +66,8 @@ intNIndexTupleInserted;
6566
intNIndexTupleProcessed;
6667

6768

69+
staticEState*InternalCreateExecutorState(MemoryContextqcontext,
70+
boolis_subquery);
6871
staticvoidShutdownExprContext(ExprContext*econtext);
6972

7073

@@ -149,9 +152,7 @@ DisplayTupleCount(FILE *statfp)
149152
EState*
150153
CreateExecutorState(void)
151154
{
152-
EState*estate;
153155
MemoryContextqcontext;
154-
MemoryContextoldcontext;
155156

156157
/*
157158
* Create the per-query context for this Executor run.
@@ -162,6 +163,37 @@ CreateExecutorState(void)
162163
ALLOCSET_DEFAULT_INITSIZE,
163164
ALLOCSET_DEFAULT_MAXSIZE);
164165

166+
returnInternalCreateExecutorState(qcontext, false);
167+
}
168+
169+
/* ----------------
170+
*CreateSubExecutorState
171+
*
172+
*Create and initialize an EState node for a sub-query.
173+
*
174+
* Ideally, sub-queries probably shouldn't have their own EState at all,
175+
* but right now this is necessary because they have their own rangetables
176+
* and we access the rangetable via the EState. It is critical that a
177+
* sub-query share the parent's es_query_cxt, else structures allocated by
178+
* the sub-query (especially its result tuple descriptor) may disappear
179+
* too soon during executor shutdown.
180+
* ----------------
181+
*/
182+
EState*
183+
CreateSubExecutorState(EState*parent_estate)
184+
{
185+
returnInternalCreateExecutorState(parent_estate->es_query_cxt, true);
186+
}
187+
188+
/*
189+
* Guts of CreateExecutorState/CreateSubExecutorState
190+
*/
191+
staticEState*
192+
InternalCreateExecutorState(MemoryContextqcontext,boolis_subquery)
193+
{
194+
EState*estate;
195+
MemoryContextoldcontext;
196+
165197
/*
166198
* Make the EState node within the per-query context. This way, we don't
167199
* need a separate pfree() operation for it at shutdown.
@@ -200,6 +232,8 @@ CreateExecutorState(void)
200232
estate->es_lastoid=InvalidOid;
201233
estate->es_rowMarks=NIL;
202234

235+
estate->es_is_subquery=is_subquery;
236+
203237
estate->es_instrument= false;
204238
estate->es_select_into= false;
205239
estate->es_into_oids= false;
@@ -258,9 +292,12 @@ FreeExecutorState(EState *estate)
258292

259293
/*
260294
* Free the per-query memory context, thereby releasing all working
261-
* memory, including the EState node itself.
295+
* memory, including the EState node itself. In a subquery, we don't
296+
* do this, leaving the memory cleanup to happen when the topmost query
297+
* is closed down.
262298
*/
263-
MemoryContextDelete(estate->es_query_cxt);
299+
if (!estate->es_is_subquery)
300+
MemoryContextDelete(estate->es_query_cxt);
264301
}
265302

266303
/* ----------------

‎src/backend/executor/nodeSubplan.c

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.80 2006/10/04 00:29:53 momjian Exp $
10+
* $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.81 2006/12/26 21:37:19 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -635,7 +635,6 @@ ExecInitSubPlan(SubPlanState *node, EState *estate, int eflags)
635635
{
636636
SubPlan*subplan= (SubPlan*)node->xprstate.expr;
637637
EState*sp_estate;
638-
MemoryContextoldcontext;
639638

640639
/*
641640
* Do access checking on the rangetable entries in the subquery.
@@ -661,15 +660,13 @@ ExecInitSubPlan(SubPlanState *node, EState *estate, int eflags)
661660
* create an EState for the subplan
662661
*
663662
* The subquery needs its own EState because it has its own rangetable. It
664-
* shares our Param ID space, however.XXX if rangetable access were done
665-
* differently, the subquery could share our EState, which would eliminate
666-
* some thrashing about in this module...
663+
* shares our Param ID space and es_query_cxt, however.XXX if rangetable
664+
*access were donedifferently, the subquery could share our EState,
665+
*which would eliminatesome thrashing about in this module...
667666
*/
668-
sp_estate=CreateExecutorState();
667+
sp_estate=CreateSubExecutorState(estate);
669668
node->sub_estate=sp_estate;
670669

671-
oldcontext=MemoryContextSwitchTo(sp_estate->es_query_cxt);
672-
673670
sp_estate->es_range_table=subplan->rtable;
674671
sp_estate->es_param_list_info=estate->es_param_list_info;
675672
sp_estate->es_param_exec_vals=estate->es_param_exec_vals;
@@ -694,8 +691,6 @@ ExecInitSubPlan(SubPlanState *node, EState *estate, int eflags)
694691

695692
node->needShutdown= true;/* now we need to shutdown the subplan */
696693

697-
MemoryContextSwitchTo(oldcontext);
698-
699694
/*
700695
* If this plan is un-correlated or undirect correlated one and want to
701696
* set params for parent plan then mark parameters as needing evaluation.
@@ -1036,11 +1031,7 @@ ExecEndSubPlan(SubPlanState *node)
10361031
{
10371032
if (node->needShutdown)
10381033
{
1039-
MemoryContextoldcontext;
1040-
1041-
oldcontext=MemoryContextSwitchTo(node->sub_estate->es_query_cxt);
10421034
ExecEndPlan(node->planstate,node->sub_estate);
1043-
MemoryContextSwitchTo(oldcontext);
10441035
FreeExecutorState(node->sub_estate);
10451036
node->sub_estate=NULL;
10461037
node->planstate=NULL;

‎src/backend/executor/nodeSubqueryscan.c

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
*
1313
*
1414
* IDENTIFICATION
15-
* $PostgreSQL: pgsql/src/backend/executor/nodeSubqueryscan.c,v 1.33 2006/12/2619:26:46 tgl Exp $
15+
* $PostgreSQL: pgsql/src/backend/executor/nodeSubqueryscan.c,v 1.34 2006/12/2621:37:19 tgl Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -49,7 +49,6 @@ SubqueryNext(SubqueryScanState *node)
4949
EState*estate;
5050
ScanDirectiondirection;
5151
TupleTableSlot*slot;
52-
MemoryContextoldcontext;
5352

5453
/*
5554
* get information from the estate and scan state
@@ -63,17 +62,12 @@ SubqueryNext(SubqueryScanState *node)
6362
*/
6463

6564
/*
66-
* Get the next tuple from the sub-query. We have to be careful to run it
67-
* in its appropriate memory context.
65+
* Get the next tuple from the sub-query.
6866
*/
6967
node->sss_SubEState->es_direction=direction;
7068

71-
oldcontext=MemoryContextSwitchTo(node->sss_SubEState->es_query_cxt);
72-
7369
slot=ExecProcNode(node->subplan);
7470

75-
MemoryContextSwitchTo(oldcontext);
76-
7771
/*
7872
* We just overwrite our ScanTupleSlot with the subplan's result slot,
7973
* rather than expending the cycles for ExecCopySlot().
@@ -112,7 +106,6 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags)
112106
SubqueryScanState*subquerystate;
113107
RangeTblEntry*rte;
114108
EState*sp_estate;
115-
MemoryContextoldcontext;
116109

117110
/* check for unsupported flags */
118111
Assert(!(eflags&EXEC_FLAG_MARK));
@@ -170,15 +163,13 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags)
170163

171164
/*
172165
* The subquery needs its own EState because it has its own rangetable. It
173-
* shares our Param ID space, however.XXX if rangetable access were done
174-
* differently, the subquery could share our EState, which would eliminate
175-
* some thrashing about in this module...
166+
* shares our Param ID space and es_query_cxt, however.XXX if rangetable
167+
*access were donedifferently, the subquery could share our EState,
168+
*which would eliminatesome thrashing about in this module...
176169
*/
177-
sp_estate=CreateExecutorState();
170+
sp_estate=CreateSubExecutorState(estate);
178171
subquerystate->sss_SubEState=sp_estate;
179172

180-
oldcontext=MemoryContextSwitchTo(sp_estate->es_query_cxt);
181-
182173
sp_estate->es_range_table=rte->subquery->rtable;
183174
sp_estate->es_param_list_info=estate->es_param_list_info;
184175
sp_estate->es_param_exec_vals=estate->es_param_exec_vals;
@@ -193,8 +184,6 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags)
193184
*/
194185
subquerystate->subplan=ExecInitNode(node->subplan,sp_estate,eflags);
195186

196-
MemoryContextSwitchTo(oldcontext);
197-
198187
subquerystate->ss.ps.ps_TupFromTlist= false;
199188

200189
/*
@@ -235,8 +224,6 @@ ExecCountSlotsSubqueryScan(SubqueryScan *node)
235224
void
236225
ExecEndSubqueryScan(SubqueryScanState*node)
237226
{
238-
MemoryContextoldcontext;
239-
240227
/*
241228
* Free the exprcontext
242229
*/
@@ -251,12 +238,8 @@ ExecEndSubqueryScan(SubqueryScanState *node)
251238
/*
252239
* close down subquery
253240
*/
254-
oldcontext=MemoryContextSwitchTo(node->sss_SubEState->es_query_cxt);
255-
256241
ExecEndPlan(node->subplan,node->sss_SubEState);
257242

258-
MemoryContextSwitchTo(oldcontext);
259-
260243
FreeExecutorState(node->sss_SubEState);
261244
}
262245

@@ -270,12 +253,9 @@ void
270253
ExecSubqueryReScan(SubqueryScanState*node,ExprContext*exprCtxt)
271254
{
272255
EState*estate;
273-
MemoryContextoldcontext;
274256

275257
estate=node->ss.ps.state;
276258

277-
oldcontext=MemoryContextSwitchTo(node->sss_SubEState->es_query_cxt);
278-
279259
/*
280260
* ExecReScan doesn't know about my subplan, so I have to do
281261
* changed-parameter signaling myself.This is just as well, because the
@@ -291,8 +271,6 @@ ExecSubqueryReScan(SubqueryScanState *node, ExprContext *exprCtxt)
291271
if (node->subplan->chgParam==NULL)
292272
ExecReScan(node->subplan,NULL);
293273

294-
MemoryContextSwitchTo(oldcontext);
295-
296274
node->ss.ss_ScanTupleSlot=NULL;
297275
node->ss.ps.ps_TupFromTlist= false;
298276
}

‎src/include/executor/executor.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.131 2006/12/04 02:06:55 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.132 2006/12/26 21:37:20 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -225,6 +225,7 @@ extern void end_tup_output(TupOutputState *tstate);
225225
* prototypes from functions in execUtils.c
226226
*/
227227
externEState*CreateExecutorState(void);
228+
externEState*CreateSubExecutorState(EState*parent_estate);
228229
externvoidFreeExecutorState(EState*estate);
229230
externExprContext*CreateExprContext(EState*estate);
230231
externExprContext*CreateStandaloneExprContext(void);

‎src/include/nodes/execnodes.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.164 2006/12/24 00:29:20 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.165 2006/12/26 21:37:20 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -328,6 +328,8 @@ typedef struct EState
328328
Oides_lastoid;/* last oid processed (by INSERT) */
329329
List*es_rowMarks;/* not good place, but there is no other */
330330

331+
booles_is_subquery;/* true if subquery (es_query_cxt not mine) */
332+
331333
booles_instrument;/* true requests runtime instrumentation */
332334
booles_select_into;/* true if doing SELECT INTO */
333335
booles_into_oids;/* true to generate OIDs in SELECT INTO */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp