Commit 313de22c authored by Tom Lane's avatar Tom Lane

SQL functions returning pass-by-reference types were copying the results

into the wrong memory context, resulting in a query-lifespan memory leak.
Bug is new in 8.0, I believe.  Per report from Rae Stiening.
parent 9427cceb
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.96 2005/04/06 16:34:04 tgl Exp $ * $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.97 2005/04/10 18:04:20 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
...@@ -86,12 +86,13 @@ static execution_state *init_execution_state(List *queryTree_list, ...@@ -86,12 +86,13 @@ static execution_state *init_execution_state(List *queryTree_list,
static void init_sql_fcache(FmgrInfo *finfo); static void init_sql_fcache(FmgrInfo *finfo);
static void postquel_start(execution_state *es, SQLFunctionCachePtr fcache); static void postquel_start(execution_state *es, SQLFunctionCachePtr fcache);
static TupleTableSlot *postquel_getnext(execution_state *es); static TupleTableSlot *postquel_getnext(execution_state *es);
static void postquel_end(execution_state *es, SQLFunctionCachePtr fcache); static void postquel_end(execution_state *es);
static void postquel_sub_params(SQLFunctionCachePtr fcache, static void postquel_sub_params(SQLFunctionCachePtr fcache,
FunctionCallInfo fcinfo); FunctionCallInfo fcinfo);
static Datum postquel_execute(execution_state *es, static Datum postquel_execute(execution_state *es,
FunctionCallInfo fcinfo, FunctionCallInfo fcinfo,
SQLFunctionCachePtr fcache); SQLFunctionCachePtr fcache,
MemoryContext resultcontext);
static void sql_exec_error_callback(void *arg); static void sql_exec_error_callback(void *arg);
static void ShutdownSQLFunction(Datum arg); static void ShutdownSQLFunction(Datum arg);
...@@ -384,7 +385,7 @@ postquel_getnext(execution_state *es) ...@@ -384,7 +385,7 @@ postquel_getnext(execution_state *es)
} }
static void static void
postquel_end(execution_state *es, SQLFunctionCachePtr fcache) postquel_end(execution_state *es)
{ {
Snapshot saveActiveSnapshot; Snapshot saveActiveSnapshot;
...@@ -454,10 +455,12 @@ postquel_sub_params(SQLFunctionCachePtr fcache, ...@@ -454,10 +455,12 @@ postquel_sub_params(SQLFunctionCachePtr fcache,
static Datum static Datum
postquel_execute(execution_state *es, postquel_execute(execution_state *es,
FunctionCallInfo fcinfo, FunctionCallInfo fcinfo,
SQLFunctionCachePtr fcache) SQLFunctionCachePtr fcache,
MemoryContext resultcontext)
{ {
TupleTableSlot *slot; TupleTableSlot *slot;
Datum value; Datum value;
MemoryContext oldcontext;
if (es->status == F_EXEC_START) if (es->status == F_EXEC_START)
postquel_start(es, fcache); postquel_start(es, fcache);
...@@ -470,7 +473,7 @@ postquel_execute(execution_state *es, ...@@ -470,7 +473,7 @@ postquel_execute(execution_state *es,
* We fall out here for all cases except where we have obtained * We fall out here for all cases except where we have obtained
* a row from a function's final SELECT. * a row from a function's final SELECT.
*/ */
postquel_end(es, fcache); postquel_end(es);
fcinfo->isnull = true; fcinfo->isnull = true;
return (Datum) NULL; return (Datum) NULL;
} }
...@@ -482,8 +485,12 @@ postquel_execute(execution_state *es, ...@@ -482,8 +485,12 @@ postquel_execute(execution_state *es,
Assert(LAST_POSTQUEL_COMMAND(es)); Assert(LAST_POSTQUEL_COMMAND(es));
/* /*
* Set up to return the function value. * Set up to return the function value. For pass-by-reference
* datatypes, be sure to allocate the result in resultcontext,
* not the current memory context (which has query lifespan).
*/ */
oldcontext = MemoryContextSwitchTo(resultcontext);
if (fcache->returnsTuple) if (fcache->returnsTuple)
{ {
/* /*
...@@ -492,7 +499,7 @@ postquel_execute(execution_state *es, ...@@ -492,7 +499,7 @@ postquel_execute(execution_state *es,
* reasons why we do this: * reasons why we do this:
* *
* 1. To copy the tuple out of the child execution context and * 1. To copy the tuple out of the child execution context and
* into our own context. * into the desired result context.
* *
* 2. To remove any junk attributes present in the raw subselect * 2. To remove any junk attributes present in the raw subselect
* result. (This is probably not absolutely necessary, but it * result. (This is probably not absolutely necessary, but it
...@@ -553,8 +560,8 @@ postquel_execute(execution_state *es, ...@@ -553,8 +560,8 @@ postquel_execute(execution_state *es,
{ {
/* /*
* Returning a scalar, which we have to extract from the first * Returning a scalar, which we have to extract from the first
* column of the SELECT result, and then copy into current * column of the SELECT result, and then copy into result
* execution context if needed. * context if needed.
*/ */
value = slot_getattr(slot, 1, &(fcinfo->isnull)); value = slot_getattr(slot, 1, &(fcinfo->isnull));
...@@ -562,12 +569,14 @@ postquel_execute(execution_state *es, ...@@ -562,12 +569,14 @@ postquel_execute(execution_state *es,
value = datumCopy(value, fcache->typbyval, fcache->typlen); value = datumCopy(value, fcache->typbyval, fcache->typlen);
} }
MemoryContextSwitchTo(oldcontext);
/* /*
* If this is a single valued function we have to end the function * If this is a single valued function we have to end the function
* execution now. * execution now.
*/ */
if (!fcinfo->flinfo->fn_retset) if (!fcinfo->flinfo->fn_retset)
postquel_end(es, fcache); postquel_end(es);
return value; return value;
} }
...@@ -627,7 +636,7 @@ fmgr_sql(PG_FUNCTION_ARGS) ...@@ -627,7 +636,7 @@ fmgr_sql(PG_FUNCTION_ARGS)
*/ */
while (es) while (es)
{ {
result = postquel_execute(es, fcinfo, fcache); result = postquel_execute(es, fcinfo, fcache, oldcontext);
if (es->status != F_EXEC_DONE) if (es->status != F_EXEC_DONE)
break; break;
es = es->next; es = es->next;
...@@ -824,7 +833,7 @@ ShutdownSQLFunction(Datum arg) ...@@ -824,7 +833,7 @@ ShutdownSQLFunction(Datum arg)
{ {
/* Shut down anything still running */ /* Shut down anything still running */
if (es->status == F_EXEC_RUN) if (es->status == F_EXEC_RUN)
postquel_end(es, fcache); postquel_end(es);
/* Reset states to START in case we're called again */ /* Reset states to START in case we're called again */
es->status = F_EXEC_START; es->status = F_EXEC_START;
es = es->next; es = es->next;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment