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.

diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 521d656ff16d6db0569e57093958a0d9a4e1beee..893ef64f03f9edd6aee5260a3a9f66205a02a251 100644 (file)
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -8,7 +8,7 @@
  *
  *
  * 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,
 static void init_sql_fcache(FmgrInfo *finfo);
 static void postquel_start(execution_state *es, SQLFunctionCachePtr fcache);
 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,
                    FunctionCallInfo fcinfo);
 static Datum postquel_execute(execution_state *es,
                 FunctionCallInfo fcinfo,
-                SQLFunctionCachePtr fcache);
+                SQLFunctionCachePtr fcache,
+                MemoryContext resultcontext);
 static void sql_exec_error_callback(void *arg);
 static void ShutdownSQLFunction(Datum arg);
 
@@ -384,7 +385,7 @@ postquel_getnext(execution_state *es)
 }
 
 static void
-postquel_end(execution_state *es, SQLFunctionCachePtr fcache)
+postquel_end(execution_state *es)
 {
    Snapshot    saveActiveSnapshot;
 
@@ -454,10 +455,12 @@ postquel_sub_params(SQLFunctionCachePtr fcache,
 static Datum
 postquel_execute(execution_state *es,
                 FunctionCallInfo fcinfo,
-                SQLFunctionCachePtr fcache)
+                SQLFunctionCachePtr fcache,
+                MemoryContext resultcontext)
 {
    TupleTableSlot *slot;
    Datum       value;
+   MemoryContext oldcontext;
 
    if (es->status == F_EXEC_START)
        postquel_start(es, fcache);
@@ -470,7 +473,7 @@ postquel_execute(execution_state *es,
         * We fall out here for all cases except where we have obtained
         * a row from a function's final SELECT.
         */
-       postquel_end(es, fcache);
+       postquel_end(es);
        fcinfo->isnull = true;
        return (Datum) NULL;
    }
@@ -482,8 +485,12 @@ postquel_execute(execution_state *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)
    {
        /*
@@ -492,7 +499,7 @@ postquel_execute(execution_state *es,
         * reasons why we do this:
         *
         * 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
         * result.  (This is probably not absolutely necessary, but it
@@ -553,8 +560,8 @@ postquel_execute(execution_state *es,
    {
        /*
         * Returning a scalar, which we have to extract from the first
-        * column of the SELECT result, and then copy into current
-        * execution context if needed.
+        * column of the SELECT result, and then copy into result
+        * context if needed.
         */
        value = slot_getattr(slot, 1, &(fcinfo->isnull));
 
@@ -562,12 +569,14 @@ postquel_execute(execution_state *es,
            value = datumCopy(value, fcache->typbyval, fcache->typlen);
    }
 
+   MemoryContextSwitchTo(oldcontext);
+
    /*
     * If this is a single valued function we have to end the function
     * execution now.
     */
    if (!fcinfo->flinfo->fn_retset)
-       postquel_end(es, fcache);
+       postquel_end(es);
 
    return value;
 }
@@ -627,7 +636,7 @@ fmgr_sql(PG_FUNCTION_ARGS)
     */
    while (es)
    {
-       result = postquel_execute(es, fcinfo, fcache);
+       result = postquel_execute(es, fcinfo, fcache, oldcontext);
        if (es->status != F_EXEC_DONE)
            break;
        es = es->next;
@@ -824,7 +833,7 @@ ShutdownSQLFunction(Datum arg)
    {
        /* Shut down anything still running */
        if (es->status == F_EXEC_RUN)
-           postquel_end(es, fcache);
+           postquel_end(es);
        /* Reset states to START in case we're called again */
        es->status = F_EXEC_START;
        es = es->next;