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

Commit25bf7f8

Browse files
committed
Fix possible failures when a tuplestore switches from in-memory to on-disk
mode while callers hold pointers to in-memory tuples. I reported this forthe case of nodeWindowAgg's primary scan tuple, but inspection of the codeshows that all of the calls in nodeWindowAgg and nodeCtescan are at risk.For the moment, fix it with a rather brute-force approach of copyingwhenever one of the at-risk callers requests a tuple. Later we mightthink of some sort of reference-count approach to reduce tuple copying.
1 parenta95307b commit25bf7f8

File tree

10 files changed

+53
-25
lines changed

10 files changed

+53
-25
lines changed

‎src/backend/executor/execQual.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.242 2009/03/26 22:26:06 petere Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.243 2009/03/27 18:30:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1385,7 +1385,7 @@ ExecMakeFunctionResult(FuncExprState *fcache,
13851385
if (fcache->funcResultStore)
13861386
{
13871387
Assert(isDone);/* it was provided before ... */
1388-
if (tuplestore_gettupleslot(fcache->funcResultStore, true,
1388+
if (tuplestore_gettupleslot(fcache->funcResultStore, true, false,
13891389
fcache->funcResultSlot))
13901390
{
13911391
*isDone=ExprMultipleResult;

‎src/backend/executor/functions.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.132 2009/01/02 20:42:00 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.133 2009/03/27 18:30:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -736,7 +736,7 @@ fmgr_sql(PG_FUNCTION_ARGS)
736736
/* Re-use the junkfilter's output slot to fetch back the tuple */
737737
Assert(fcache->junkFilter);
738738
slot=fcache->junkFilter->jf_resultSlot;
739-
if (!tuplestore_gettupleslot(fcache->tstore, true,slot))
739+
if (!tuplestore_gettupleslot(fcache->tstore, true,false,slot))
740740
elog(ERROR,"failed to fetch lazy-eval tuple");
741741
/* Extract the result as a datum, and copy out from the slot */
742742
result=postquel_get_single_result(slot,fcinfo,
@@ -822,7 +822,7 @@ fmgr_sql(PG_FUNCTION_ARGS)
822822
{
823823
/* Re-use the junkfilter's output slot to fetch back the tuple */
824824
slot=fcache->junkFilter->jf_resultSlot;
825-
if (tuplestore_gettupleslot(fcache->tstore, true,slot))
825+
if (tuplestore_gettupleslot(fcache->tstore, true,false,slot))
826826
result=postquel_get_single_result(slot,fcinfo,
827827
fcache,oldcontext);
828828
else

‎src/backend/executor/nodeCtescan.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeCtescan.c,v 1.3 2009/01/01 17:23:41 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeCtescan.c,v 1.4 2009/03/27 18:30:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -71,10 +71,14 @@ CteScanNext(CteScanState *node)
7171

7272
/*
7373
* If we can fetch another tuple from the tuplestore, return it.
74+
*
75+
* Note: we have to use copy=true in the tuplestore_gettupleslot call,
76+
* because we are sharing the tuplestore with other nodes that might
77+
* write into the tuplestore before we get called again.
7478
*/
7579
if (!eof_tuplestore)
7680
{
77-
if (tuplestore_gettupleslot(tuplestorestate,forward,slot))
81+
if (tuplestore_gettupleslot(tuplestorestate,forward,true,slot))
7882
returnslot;
7983
if (forward)
8084
eof_tuplestore= true;

‎src/backend/executor/nodeFunctionscan.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeFunctionscan.c,v 1.50 2009/01/01 17:23:41 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeFunctionscan.c,v 1.51 2009/03/27 18:30:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -74,6 +74,7 @@ FunctionNext(FunctionScanState *node)
7474
slot=node->ss.ss_ScanTupleSlot;
7575
(void)tuplestore_gettupleslot(tuplestorestate,
7676
ScanDirectionIsForward(direction),
77+
false,
7778
slot);
7879
returnslot;
7980
}

‎src/backend/executor/nodeMaterial.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeMaterial.c,v 1.65 2009/01/01 17:23:42 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeMaterial.c,v 1.66 2009/03/27 18:30:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -104,7 +104,7 @@ ExecMaterial(MaterialState *node)
104104
slot=node->ss.ps.ps_ResultTupleSlot;
105105
if (!eof_tuplestore)
106106
{
107-
if (tuplestore_gettupleslot(tuplestorestate,forward,slot))
107+
if (tuplestore_gettupleslot(tuplestorestate,forward,false,slot))
108108
returnslot;
109109
if (forward)
110110
eof_tuplestore= true;

‎src/backend/executor/nodeWindowAgg.c‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
* Portions Copyright (c) 1994, Regents of the University of California
2828
*
2929
* IDENTIFICATION
30-
* $PostgreSQL: pgsql/src/backend/executor/nodeWindowAgg.c,v 1.3 2009/01/01 17:23:42 momjian Exp $
30+
* $PostgreSQL: pgsql/src/backend/executor/nodeWindowAgg.c,v 1.4 2009/03/27 18:30:21 tgl Exp $
3131
*
3232
*-------------------------------------------------------------------------
3333
*/
@@ -480,7 +480,8 @@ eval_windowaggregates(WindowAggState *winstate)
480480
spool_tuples(winstate,winstate->aggregatedupto);
481481
tuplestore_select_read_pointer(winstate->buffer,
482482
winstate->agg_ptr);
483-
if (!tuplestore_gettupleslot(winstate->buffer, true,agg_row_slot))
483+
if (!tuplestore_gettupleslot(winstate->buffer, true, true,
484+
agg_row_slot))
484485
break;/* must be end of partition */
485486
}
486487

@@ -1001,12 +1002,14 @@ ExecWindowAgg(WindowAggState *winstate)
10011002
/*
10021003
* Read the current row from the tuplestore, and save in ScanTupleSlot.
10031004
* (We can't rely on the outerplan's output slot because we may have to
1004-
* read beyond the current row.)
1005+
* read beyond the current row. Also, we have to actually copy the row
1006+
* out of the tuplestore, since window function evaluation might cause
1007+
* the tuplestore to dump its state to disk.)
10051008
*
10061009
* Current row must be in the tuplestore, since we spooled it above.
10071010
*/
10081011
tuplestore_select_read_pointer(winstate->buffer,winstate->current_ptr);
1009-
if (!tuplestore_gettupleslot(winstate->buffer, true,
1012+
if (!tuplestore_gettupleslot(winstate->buffer, true, true,
10101013
winstate->ss.ss_ScanTupleSlot))
10111014
elog(ERROR,"unexpected end of tuplestore");
10121015

@@ -1589,14 +1592,14 @@ window_gettupleslot(WindowObject winobj, int64 pos, TupleTableSlot *slot)
15891592

15901593
while (winobj->seekpos>pos)
15911594
{
1592-
if (!tuplestore_gettupleslot(winstate->buffer, false,slot))
1595+
if (!tuplestore_gettupleslot(winstate->buffer, false,true,slot))
15931596
elog(ERROR,"unexpected end of tuplestore");
15941597
winobj->seekpos--;
15951598
}
15961599

15971600
while (winobj->seekpos<pos)
15981601
{
1599-
if (!tuplestore_gettupleslot(winstate->buffer, true,slot))
1602+
if (!tuplestore_gettupleslot(winstate->buffer, true,true,slot))
16001603
elog(ERROR,"unexpected end of tuplestore");
16011604
winobj->seekpos++;
16021605
}

‎src/backend/executor/nodeWorktablescan.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeWorktablescan.c,v 1.5 2009/01/01 17:23:42 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeWorktablescan.c,v 1.6 2009/03/27 18:30:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -43,6 +43,10 @@ WorkTableScanNext(WorkTableScanState *node)
4343
* worktable plan node, since it cannot appear high enough in the plan
4444
* tree of a scrollable cursor to be exposed to a backward-scan
4545
* requirement. So it's not worth expending effort to support it.
46+
*
47+
* Note: we are also assuming that this node is the only reader of the
48+
* worktable. Therefore, we don't need a private read pointer for the
49+
* tuplestore, nor do we need to tell tuplestore_gettupleslot to copy.
4650
*/
4751
estate=node->ss.ps.state;
4852
Assert(ScanDirectionIsForward(estate->es_direction));
@@ -53,7 +57,7 @@ WorkTableScanNext(WorkTableScanState *node)
5357
* Get the next tuple from tuplestore. Return NULL if no more tuples.
5458
*/
5559
slot=node->ss.ss_ScanTupleSlot;
56-
(void)tuplestore_gettupleslot(tuplestorestate, true,slot);
60+
(void)tuplestore_gettupleslot(tuplestorestate, true,false,slot);
5761
returnslot;
5862
}
5963

‎src/backend/tcop/pquery.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.129 2009/01/02 20:42:00 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.130 2009/03/27 18:30:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1118,7 +1118,8 @@ RunFromStore(Portal portal, ScanDirection direction, long count,
11181118

11191119
oldcontext=MemoryContextSwitchTo(portal->holdContext);
11201120

1121-
ok=tuplestore_gettupleslot(portal->holdStore,forward,slot);
1121+
ok=tuplestore_gettupleslot(portal->holdStore,forward, false,
1122+
slot);
11221123

11231124
MemoryContextSwitchTo(oldcontext);
11241125

‎src/backend/utils/sort/tuplestore.c‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
* Portions Copyright (c) 1994, Regents of the University of California
4848
*
4949
* IDENTIFICATION
50-
* $PostgreSQL: pgsql/src/backend/utils/sort/tuplestore.c,v 1.46 2009/01/01 17:23:53 momjian Exp $
50+
* $PostgreSQL: pgsql/src/backend/utils/sort/tuplestore.c,v 1.47 2009/03/27 18:30:21 tgl Exp $
5151
*
5252
*-------------------------------------------------------------------------
5353
*/
@@ -871,10 +871,17 @@ tuplestore_gettuple(Tuplestorestate *state, bool forward,
871871
*
872872
* If successful, put tuple in slot and return TRUE; else, clear the slot
873873
* and return FALSE.
874+
*
875+
* If copy is TRUE, the slot receives a copied tuple (allocated in current
876+
* memory context) that will stay valid regardless of future manipulations of
877+
* the tuplestore's state. If copy is FALSE, the slot may just receive a
878+
* pointer to a tuple held within the tuplestore. The latter is more
879+
* efficient but the slot contents may be corrupted if additional writes to
880+
* the tuplestore occur. (If using tuplestore_trim, see comments therein.)
874881
*/
875882
bool
876883
tuplestore_gettupleslot(Tuplestorestate*state,boolforward,
877-
TupleTableSlot*slot)
884+
boolcopy,TupleTableSlot*slot)
878885
{
879886
MinimalTupletuple;
880887
boolshould_free;
@@ -883,6 +890,11 @@ tuplestore_gettupleslot(Tuplestorestate *state, bool forward,
883890

884891
if (tuple)
885892
{
893+
if (copy&& !should_free)
894+
{
895+
tuple=heap_copy_minimal_tuple(tuple);
896+
should_free= true;
897+
}
886898
ExecStoreMinimalTuple(tuple,slot,should_free);
887899
return true;
888900
}
@@ -1107,7 +1119,10 @@ tuplestore_trim(Tuplestorestate *state)
11071119
* since tuplestore_gettuple returns a direct pointer to our
11081120
* internal copy of the tuple, it's likely that the caller has
11091121
* still got the tuple just before "current" referenced in a slot.
1110-
* So we keep one extra tuple before the oldest "current".
1122+
* So we keep one extra tuple before the oldest "current". (Strictly
1123+
* speaking, we could require such callers to use the "copy" flag to
1124+
* tuplestore_gettupleslot, but for efficiency we allow this one case
1125+
* to not use "copy".)
11111126
*/
11121127
nremove=oldest-1;
11131128
if (nremove <=0)

‎src/include/utils/tuplestore.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
2525
* Portions Copyright (c) 1994, Regents of the University of California
2626
*
27-
* $PostgreSQL: pgsql/src/include/utils/tuplestore.h,v 1.28 2009/01/01 17:24:02 momjian Exp $
27+
* $PostgreSQL: pgsql/src/include/utils/tuplestore.h,v 1.29 2009/03/27 18:30:21 tgl Exp $
2828
*
2929
*-------------------------------------------------------------------------
3030
*/
@@ -71,7 +71,7 @@ extern void tuplestore_trim(Tuplestorestate *state);
7171
externbooltuplestore_in_memory(Tuplestorestate*state);
7272

7373
externbooltuplestore_gettupleslot(Tuplestorestate*state,boolforward,
74-
TupleTableSlot*slot);
74+
boolcopy,TupleTableSlot*slot);
7575
externbooltuplestore_advance(Tuplestorestate*state,boolforward);
7676

7777
externbooltuplestore_ateof(Tuplestorestate*state);

0 commit comments

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