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

Commite83a8ae

Browse files
committed
Don't use a tuplestore if we don't have to for SQL-language functions.
We only need a tuplestore if we're actually going to accumulatemultiple result tuples. Obviously then we don't need one for non-set-returning functions; but even a SRF doesn't need one if we decide touse "lazyEval" (one row at a time) mode. In these cases, it'ssufficient to use the junkfilter's result slot to hold the single rowthat's due to be returned. We just need to "materialize" that slotto ensure it holds onto the data past shutdown of the sub-executor.The original intent of this patch was partially to save a few cycles(by not putting tuples into a tuplestore only to pull them back outimmediately), but mostly to ensure that we don't use a tuplestorein non-set-returning functions. That's because I had concernsabout whether a tuplestore is safe to keep across queries,which was possible for functions invoked via long-lived FmgrInfossuch as those kept in the typcache. There are no cases where SRFsare called that way, so getting rid of the tuplestore in non-SRFsshould make things safer.However, it emerges that running fmgr_sql in a short-lived context(as595d1ef made it do) makes the existing coding unsafe anyway:we can end up with a long-lived TupleTableSlot holding a freeablereference to a short-lived tuple, resulting in a double-free crash.Not trying to pull tuples out of the tuplestore using that slotdodges the problem, so I'm going to commit this now rather thaninvent a band-aid solution for v18.Reported-by: Alexander Lakhin <exclusion@gmail.com>Author: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/2443532.1744919968@sss.pgh.pa.usDiscussion:https://postgr.es/m/9f975803-1a1c-4f21-b987-f572e110e860@gmail.com
1 parentc83a387 commite83a8ae

File tree

1 file changed

+85
-50
lines changed

1 file changed

+85
-50
lines changed

‎src/backend/executor/functions.c‎

Lines changed: 85 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
typedefstruct
4545
{
4646
DestReceiverpub;/* publicly-known function pointers */
47-
Tuplestorestate*tstore;/* where to put result tuples */
47+
Tuplestorestate*tstore;/* where to put result tuples, or NULL */
4848
JunkFilter*filter;/* filter to convert tuple type */
4949
}DR_sqlfunction;
5050

@@ -145,11 +145,13 @@ typedef struct SQLFunctionCache
145145
boollazyEvalOK;/* true if lazyEval is safe */
146146
boolshutdown_reg;/* true if registered shutdown callback */
147147
boollazyEval;/* true if using lazyEval for result query */
148+
boolrandomAccess;/* true if tstore needs random access */
148149
boolownSubcontext;/* is subcontext really a separate context? */
149150

150151
ParamListInfoparamLI;/* Param list representing current args */
151152

152-
Tuplestorestate*tstore;/* where we accumulate result tuples */
153+
Tuplestorestate*tstore;/* where we accumulate result for a SRF */
154+
MemoryContexttscontext;/* memory context that tstore should be in */
153155

154156
JunkFilter*junkFilter;/* will be NULL if function returns VOID */
155157
intjf_generation;/* tracks whether junkFilter is up-to-date */
@@ -1250,7 +1252,7 @@ static void
12501252
postquel_start(execution_state*es,SQLFunctionCachePtrfcache)
12511253
{
12521254
DestReceiver*dest;
1253-
MemoryContextoldcontext;
1255+
MemoryContextoldcontext=CurrentMemoryContext;
12541256

12551257
Assert(es->qd==NULL);
12561258

@@ -1296,12 +1298,27 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache)
12961298
fcache->ownSubcontext= false;
12971299
}
12981300

1301+
/*
1302+
* Build a tuplestore if needed, that is if it's a set-returning function
1303+
* and we're producing the function result without using lazyEval mode.
1304+
*/
1305+
if (es->setsResult)
1306+
{
1307+
Assert(fcache->tstore==NULL);
1308+
if (fcache->func->returnsSet&& !es->lazyEval)
1309+
{
1310+
MemoryContextSwitchTo(fcache->tscontext);
1311+
fcache->tstore=tuplestore_begin_heap(fcache->randomAccess,
1312+
false,work_mem);
1313+
}
1314+
}
1315+
12991316
/* Switch into the selected subcontext (might be a no-op) */
1300-
oldcontext=MemoryContextSwitchTo(fcache->subcontext);
1317+
MemoryContextSwitchTo(fcache->subcontext);
13011318

13021319
/*
1303-
* If this query produces the function result,send its outputto the
1304-
*tuplestore; else discard any output.
1320+
* If this query produces the function result,collect its outputusing
1321+
*our custom DestReceiver; else discard any output.
13051322
*/
13061323
if (es->setsResult)
13071324
{
@@ -1311,8 +1328,11 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache)
13111328
/* pass down the needed info to the dest receiver routines */
13121329
myState= (DR_sqlfunction*)dest;
13131330
Assert(myState->pub.mydest==DestSQLFunction);
1314-
myState->tstore=fcache->tstore;
1331+
myState->tstore=fcache->tstore;/* might be NULL */
13151332
myState->filter=fcache->junkFilter;
1333+
1334+
/* Make very sure the junkfilter's result slot is empty */
1335+
ExecClearTuple(fcache->junkFilter->jf_resultSlot);
13161336
}
13171337
else
13181338
dest=None_Receiver;
@@ -1500,8 +1520,8 @@ postquel_get_single_result(TupleTableSlot *slot,
15001520
/*
15011521
* Set up to return the function value. For pass-by-reference datatypes,
15021522
* be sure to copy the result into the current context. We can't leave
1503-
* the data in the TupleTableSlot because weintend toclear the slot
1504-
*beforereturning.
1523+
* the data in the TupleTableSlot because wemustclear the slot before
1524+
* returning.
15051525
*/
15061526
if (fcache->func->returnsTuple)
15071527
{
@@ -1521,6 +1541,9 @@ postquel_get_single_result(TupleTableSlot *slot,
15211541
value=datumCopy(value,fcache->func->typbyval,fcache->func->typlen);
15221542
}
15231543

1544+
/* Clear the slot for next time */
1545+
ExecClearTuple(slot);
1546+
15241547
returnvalue;
15251548
}
15261549

@@ -1532,6 +1555,7 @@ fmgr_sql(PG_FUNCTION_ARGS)
15321555
{
15331556
SQLFunctionCachePtrfcache;
15341557
ErrorContextCallbacksqlerrcontext;
1558+
MemoryContexttscontext;
15351559
boolrandomAccess;
15361560
boollazyEvalOK;
15371561
boolpushed_snapshot;
@@ -1558,18 +1582,26 @@ fmgr_sql(PG_FUNCTION_ARGS)
15581582
errmsg("set-valued function called in context that cannot accept a set")));
15591583
randomAccess=rsi->allowedModes&SFRM_Materialize_Random;
15601584
lazyEvalOK= !(rsi->allowedModes&SFRM_Materialize_Preferred);
1585+
/* tuplestore, if used, must have query lifespan */
1586+
tscontext=rsi->econtext->ecxt_per_query_memory;
15611587
}
15621588
else
15631589
{
15641590
randomAccess= false;
15651591
lazyEvalOK= true;
1592+
/* we won't need a tuplestore */
1593+
tscontext=NULL;
15661594
}
15671595

15681596
/*
15691597
* Initialize fcache if starting a fresh execution.
15701598
*/
15711599
fcache=init_sql_fcache(fcinfo,lazyEvalOK);
15721600

1601+
/* Remember info that we might need later to construct tuplestore */
1602+
fcache->tscontext=tscontext;
1603+
fcache->randomAccess=randomAccess;
1604+
15731605
/*
15741606
* Now we can set up error traceback support for ereport()
15751607
*/
@@ -1578,20 +1610,6 @@ fmgr_sql(PG_FUNCTION_ARGS)
15781610
sqlerrcontext.previous=error_context_stack;
15791611
error_context_stack=&sqlerrcontext;
15801612

1581-
/*
1582-
* Build tuplestore to hold results, if we don't have one already. We
1583-
* want to re-use the tuplestore across calls, so it needs to live in
1584-
* fcontext.
1585-
*/
1586-
if (!fcache->tstore)
1587-
{
1588-
MemoryContextoldcontext;
1589-
1590-
oldcontext=MemoryContextSwitchTo(fcache->fcontext);
1591-
fcache->tstore=tuplestore_begin_heap(randomAccess, false,work_mem);
1592-
MemoryContextSwitchTo(oldcontext);
1593-
}
1594-
15951613
/*
15961614
* Find first unfinished execution_state. If none, advance to the next
15971615
* query in function.
@@ -1661,11 +1679,12 @@ fmgr_sql(PG_FUNCTION_ARGS)
16611679

16621680
/*
16631681
* If we ran the command to completion, we can shut it down now. Any
1664-
* row(s) we need to return are safely stashed in the tuplestore, and
1665-
* we want to be sure that, for example, AFTER triggers get fired
1666-
* before we return anything. Also, if the function doesn't return
1667-
* set, we can shut it down anyway because it must be a SELECT and we
1668-
* don't care about fetching any more result rows.
1682+
* row(s) we need to return are safely stashed in the result slot or
1683+
* tuplestore, and we want to be sure that, for example, AFTER
1684+
* triggers get fired before we return anything. Also, if the
1685+
* function doesn't return set, we can shut it down anyway because it
1686+
* must be a SELECT and we don't care about fetching any more result
1687+
* rows.
16691688
*/
16701689
if (completed|| !fcache->func->returnsSet)
16711690
postquel_end(es,fcache);
@@ -1708,7 +1727,8 @@ fmgr_sql(PG_FUNCTION_ARGS)
17081727
}
17091728

17101729
/*
1711-
* The tuplestore now contains whatever row(s) we are supposed to return.
1730+
* The result slot or tuplestore now contains whatever row(s) we are
1731+
* supposed to return.
17121732
*/
17131733
if (fcache->func->returnsSet)
17141734
{
@@ -1721,16 +1741,12 @@ fmgr_sql(PG_FUNCTION_ARGS)
17211741
* row.
17221742
*/
17231743
Assert(es->lazyEval);
1724-
/*Re-use thejunkfilter'soutput slotto fetch back the tuple */
1744+
/*Thejunkfilter'sresult slotcontains the query result tuple */
17251745
Assert(fcache->junkFilter);
17261746
slot=fcache->junkFilter->jf_resultSlot;
1727-
if (!tuplestore_gettupleslot(fcache->tstore, true, false,slot))
1728-
elog(ERROR,"failed to fetch lazy-eval tuple");
1747+
Assert(!TTS_EMPTY(slot));
17291748
/* Extract the result as a datum, and copy out from the slot */
17301749
result=postquel_get_single_result(slot,fcinfo,fcache);
1731-
/* Clear the tuplestore, but keep it for next time */
1732-
/* NB: this might delete the slot's content, but we don't care */
1733-
tuplestore_clear(fcache->tstore);
17341750

17351751
/*
17361752
* Let caller know we're not finished.
@@ -1752,12 +1768,8 @@ fmgr_sql(PG_FUNCTION_ARGS)
17521768
elseif (fcache->lazyEval)
17531769
{
17541770
/*
1755-
* We are done with a lazy evaluation. Clean up.
1756-
*/
1757-
tuplestore_clear(fcache->tstore);
1758-
1759-
/*
1760-
* Let caller know we're finished.
1771+
* We are done with a lazy evaluation. Let caller know we're
1772+
* finished.
17611773
*/
17621774
rsi->isDone=ExprEndResult;
17631775

@@ -1779,7 +1791,12 @@ fmgr_sql(PG_FUNCTION_ARGS)
17791791
* We are done with a non-lazy evaluation. Return whatever is in
17801792
* the tuplestore. (It is now caller's responsibility to free the
17811793
* tuplestore when done.)
1794+
*
1795+
* Note an edge case: we could get here without having made a
1796+
* tuplestore if the function is declared to return SETOF VOID.
1797+
* ExecMakeTableFunctionResult will cope with null setResult.
17821798
*/
1799+
Assert(fcache->tstore||fcache->func->rettype==VOIDOID);
17831800
rsi->returnMode=SFRM_Materialize;
17841801
rsi->setResult=fcache->tstore;
17851802
fcache->tstore=NULL;
@@ -1807,9 +1824,9 @@ fmgr_sql(PG_FUNCTION_ARGS)
18071824
*/
18081825
if (fcache->junkFilter)
18091826
{
1810-
/*Re-use thejunkfilter'soutput slotto fetch back the tuple */
1827+
/*Thejunkfilter'sresult slotcontains the query result tuple */
18111828
slot=fcache->junkFilter->jf_resultSlot;
1812-
if (tuplestore_gettupleslot(fcache->tstore, true, false,slot))
1829+
if (!TTS_EMPTY(slot))
18131830
result=postquel_get_single_result(slot,fcinfo,fcache);
18141831
else
18151832
{
@@ -1824,9 +1841,6 @@ fmgr_sql(PG_FUNCTION_ARGS)
18241841
fcinfo->isnull= true;
18251842
result= (Datum)0;
18261843
}
1827-
1828-
/* Clear the tuplestore, but keep it for next time */
1829-
tuplestore_clear(fcache->tstore);
18301844
}
18311845

18321846
/* Pop snapshot if we have pushed one */
@@ -2604,11 +2618,32 @@ sqlfunction_receive(TupleTableSlot *slot, DestReceiver *self)
26042618
{
26052619
DR_sqlfunction*myState= (DR_sqlfunction*)self;
26062620

2607-
/* Filter tuple as needed */
2608-
slot=ExecFilterJunk(myState->filter,slot);
2621+
if (myState->tstore)
2622+
{
2623+
/* We are collecting all of a set result into the tuplestore */
2624+
2625+
/* Filter tuple as needed */
2626+
slot=ExecFilterJunk(myState->filter,slot);
26092627

2610-
/* Store the filtered tuple into the tuplestore */
2611-
tuplestore_puttupleslot(myState->tstore,slot);
2628+
/* Store the filtered tuple into the tuplestore */
2629+
tuplestore_puttupleslot(myState->tstore,slot);
2630+
}
2631+
else
2632+
{
2633+
/*
2634+
* We only want the first tuple, which we'll save in the junkfilter's
2635+
* result slot. Ignore any additional tuples passed.
2636+
*/
2637+
if (TTS_EMPTY(myState->filter->jf_resultSlot))
2638+
{
2639+
/* Filter tuple as needed */
2640+
slot=ExecFilterJunk(myState->filter,slot);
2641+
Assert(slot==myState->filter->jf_resultSlot);
2642+
2643+
/* Materialize the slot so it preserves pass-by-ref values */
2644+
ExecMaterializeSlot(slot);
2645+
}
2646+
}
26122647

26132648
return true;
26142649
}

0 commit comments

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