SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs) - Mailing list pgsql-patches

Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>>Is it OK to use fcinfo->flinfo->fn_mcxt as the long term memory
>>context or is there a better choice?
>
> That is the correct choice.
>
>>Is funcctx->multi_call_memory_ctx a
>>suitable name in place of funcctx->fmctx?
>
> No objection here.

Here's a patch to address Tom's SRF API memory management concerns, as
discussed earlier today on HACKERS.

Please note that, although this should apply cleanly on cvs tip, it will
have (two) failed hunks (nodeFunctionscan.c) *if* applied after Neil's
plpgsql SRF patch. Or it will cause a failure in Neil's patch if it is
applied first (I think). The fix in either case is to wrap the loop that
collects rows from the function and stashes them in the tuplestore as
follows:

do until no more tuples
+  ExprContext *econtext = scanstate->csstate.cstate.cs_ExprContext;

   get one tuple
   put it in the tuplestore

+  ResetExprContext(econtext);
loop

Also note that contrib/dblink is intentionally missing, because I'm
still working on other aspects of it. I'll have an updated dblink in a
day or so.

If there are no objections, please apply.

Thanks,

Joe
Index: contrib/pgstattuple/pgstattuple.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/contrib/pgstattuple/pgstattuple.c,v
retrieving revision 1.7
diff -c -r1.7 pgstattuple.c
*** contrib/pgstattuple/pgstattuple.c    23 Aug 2002 08:19:49 -0000    1.7
--- contrib/pgstattuple/pgstattuple.c    29 Aug 2002 05:31:26 -0000
***************
*** 78,94 ****
      TupleDesc            tupdesc;
      TupleTableSlot       *slot;
      AttInMetadata       *attinmeta;
!
!     char **values;
!     int i;
!     Datum        result;

      /* stuff done only on the first call of the function */
      if(SRF_IS_FIRSTCALL())
      {
          /* create a function context for cross-call persistence */
!         funcctx = SRF_FIRSTCALL_INIT();
!
          /* total number of tuples to be returned */
          funcctx->max_calls = 1;

--- 78,97 ----
      TupleDesc            tupdesc;
      TupleTableSlot       *slot;
      AttInMetadata       *attinmeta;
!     char              **values;
!     int                    i;
!     Datum                result;
!     MemoryContext        oldcontext;

      /* stuff done only on the first call of the function */
      if(SRF_IS_FIRSTCALL())
      {
          /* create a function context for cross-call persistence */
!          funcctx = SRF_FIRSTCALL_INIT();
!
!         /* switch to memory context appropriate for multiple function calls */
!         oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
!
          /* total number of tuples to be returned */
          funcctx->max_calls = 1;

***************
*** 109,114 ****
--- 112,119 ----
           */
          attinmeta = TupleDescGetAttInMetadata(tupdesc);
          funcctx->attinmeta = attinmeta;
+
+         MemoryContextSwitchTo(oldcontext);
      }

      /* stuff done on every call of the function */
Index: contrib/tablefunc/tablefunc.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/contrib/tablefunc/tablefunc.c,v
retrieving revision 1.2
diff -c -r1.2 tablefunc.c
*** contrib/tablefunc/tablefunc.c    15 Aug 2002 02:51:26 -0000    1.2
--- contrib/tablefunc/tablefunc.c    29 Aug 2002 05:37:30 -0000
***************
*** 87,92 ****
--- 87,93 ----
      float8                stddev;
      float8                carry_val;
      bool                use_carry;
+     MemoryContext        oldcontext;

      /* stuff done only on the first call of the function */
       if(SRF_IS_FIRSTCALL())
***************
*** 94,99 ****
--- 95,103 ----
          /* create a function context for cross-call persistence */
           funcctx = SRF_FIRSTCALL_INIT();

+         /* switch to memory context appropriate for multiple function calls */
+         oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
          /* total number of tuples to be returned */
          funcctx->max_calls = PG_GETARG_UINT32(0);

***************
*** 119,124 ****
--- 123,130 ----
           * purpose it doesn't matter, just cast it as an unsigned value
           */
          srandom(PG_GETARG_UINT32(3));
+
+         MemoryContextSwitchTo(oldcontext);
      }

      /* stuff done on every call of the function */
***************
*** 260,269 ****
      AttInMetadata       *attinmeta;
      SPITupleTable       *spi_tuptable = NULL;
      TupleDesc            spi_tupdesc;
!     char               *lastrowid;
      crosstab_fctx       *fctx;
      int                    i;
      int                    num_categories;

      /* stuff done only on the first call of the function */
       if(SRF_IS_FIRSTCALL())
--- 266,276 ----
      AttInMetadata       *attinmeta;
      SPITupleTable       *spi_tuptable = NULL;
      TupleDesc            spi_tupdesc;
!     char               *lastrowid = NULL;
      crosstab_fctx       *fctx;
      int                    i;
      int                    num_categories;
+     MemoryContext        oldcontext;

      /* stuff done only on the first call of the function */
       if(SRF_IS_FIRSTCALL())
***************
*** 275,287 ****
          TupleDesc        tupdesc = NULL;
          int                ret;
          int                proc;
-         MemoryContext    oldcontext;

          /* create a function context for cross-call persistence */
           funcctx = SRF_FIRSTCALL_INIT();

!         /* SPI switches context on us, so save it first */
!         oldcontext = CurrentMemoryContext;

          /* Connect to SPI manager */
          if ((ret = SPI_connect()) < 0)
--- 282,293 ----
          TupleDesc        tupdesc = NULL;
          int                ret;
          int                proc;

          /* create a function context for cross-call persistence */
           funcctx = SRF_FIRSTCALL_INIT();

!         /* switch to memory context appropriate for multiple function calls */
!         oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);

          /* Connect to SPI manager */
          if ((ret = SPI_connect()) < 0)
***************
*** 317,324 ****
               SRF_RETURN_DONE(funcctx);
          }

!         /* back to the original memory context */
!         MemoryContextSwitchTo(oldcontext);

          /* get the typeid that represents our return type */
          functypeid = get_func_rettype(funcid);
--- 323,330 ----
               SRF_RETURN_DONE(funcctx);
          }

!         /* SPI switches context on us, so reset it */
!         MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);

          /* get the typeid that represents our return type */
          functypeid = get_func_rettype(funcid);
***************
*** 381,386 ****
--- 387,394 ----

          /* total number of tuples to be returned */
          funcctx->max_calls = proc;
+
+         MemoryContextSwitchTo(oldcontext);
      }

      /* stuff done on every call of the function */
***************
*** 432,438 ****
              for (i = 0; i < num_categories; i++)
              {
                  HeapTuple    spi_tuple;
!                 char       *rowid;

                  /* see if we've gone too far already */
                  if (call_cntr >= max_calls)
--- 440,446 ----
              for (i = 0; i < num_categories; i++)
              {
                  HeapTuple    spi_tuple;
!                 char       *rowid = NULL;

                  /* see if we've gone too far already */
                  if (call_cntr >= max_calls)
***************
*** 496,502 ****
--- 504,516 ----
              xpfree(fctx->lastrowid);

              if (values[0] != NULL)
+             {
+                 /* switch to memory context appropriate for multiple function calls */
+                 oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
                  lastrowid = fctx->lastrowid = pstrdup(values[0]);
+                 MemoryContextSwitchTo(oldcontext);
+             }

              if (!allnulls)
              {
Index: doc/src/sgml/xfunc.sgml
===================================================================
RCS file: /opt/src/cvs/pgsql-server/doc/src/sgml/xfunc.sgml,v
retrieving revision 1.57
diff -c -r1.57 xfunc.sgml
*** doc/src/sgml/xfunc.sgml    29 Aug 2002 00:17:02 -0000    1.57
--- doc/src/sgml/xfunc.sgml    29 Aug 2002 05:30:04 -0000
***************
*** 1670,1682 ****
      AttInMetadata       *attinmeta;

      /*
!      * memory context used to initialize structure
       *
!      * fmctx is set by SRF_FIRSTCALL_INIT() for you, and used by
!      * SRF_RETURN_DONE() for cleanup. It is primarily for internal use
!      * by the API.
       */
!     MemoryContext    fmctx;

  }    FuncCallContext;
  </programlisting>
--- 1670,1682 ----
      AttInMetadata       *attinmeta;

      /*
!      * memory context used for structures which must live for multiple calls
       *
!      * multi_call_memory_ctx is set by SRF_FIRSTCALL_INIT() for you, and used
!      * by SRF_RETURN_DONE() for cleanup. It is the most appropriate memory
!      * context for any memory that is allocated for use in multiple calls.
       */
!     MemoryContext    multi_call_memory_ctx;

  }    FuncCallContext;
  </programlisting>
***************
*** 1720,1740 ****
  Datum
  my_Set_Returning_Function(PG_FUNCTION_ARGS)
  {
!     FuncCallContext     *funcctx;
!     Datum                result;
      [user defined declarations]

      if (SRF_IS_FIRSTCALL())
      {
          /* one-time setup code appears here: */
          [user defined code]
-         funcctx = SRF_FIRSTCALL_INIT();
          [if returning composite]
              [build TupleDesc, and perhaps AttInMetadata]
              [obtain slot]
              funcctx->slot = slot;
          [endif returning composite]
          [user defined code]
      }

      /* each-time setup code appears here: */
--- 1720,1743 ----
  Datum
  my_Set_Returning_Function(PG_FUNCTION_ARGS)
  {
!     FuncCallContext  *funcctx;
!     Datum             result;
!     MemoryContext     oldcontext;
      [user defined declarations]

      if (SRF_IS_FIRSTCALL())
      {
+            funcctx = SRF_FIRSTCALL_INIT();
+         oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
          /* one-time setup code appears here: */
          [user defined code]
          [if returning composite]
              [build TupleDesc, and perhaps AttInMetadata]
              [obtain slot]
              funcctx->slot = slot;
          [endif returning composite]
          [user defined code]
+         MemoryContextSwitchTo(oldcontext);
      }

      /* each-time setup code appears here: */
***************
*** 1747,1752 ****
--- 1750,1758 ----
      {
          /* here we want to return another item: */
          [user defined code]
+         oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+         [user allocated memory needed in later function calls]
+         MemoryContextSwitchTo(oldcontext);
          [obtain result Datum]
          SRF_RETURN_NEXT(funcctx, result);
      }
***************
*** 1777,1784 ****
       /* stuff done only on the first call of the function */
       if (SRF_IS_FIRSTCALL())
       {
!         /* create a function context for cross-call persistence */
!          funcctx = SRF_FIRSTCALL_INIT();

          /* total number of tuples to be returned */
          funcctx->max_calls = PG_GETARG_UINT32(0);
--- 1783,1795 ----
       /* stuff done only on the first call of the function */
       if (SRF_IS_FIRSTCALL())
       {
!         MemoryContext    oldcontext;
!
!         /* create a function context for cross-call persistence */
!         funcctx = SRF_FIRSTCALL_INIT();
!
!         /* switch to memory context appropriate for multiple function calls */
!         oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);

          /* total number of tuples to be returned */
          funcctx->max_calls = PG_GETARG_UINT32(0);
***************
*** 1800,1805 ****
--- 1811,1818 ----
           */
          attinmeta = TupleDescGetAttInMetadata(tupdesc);
          funcctx->attinmeta = attinmeta;
+
+         MemoryContextSwitchTo(oldcontext);
      }

      /* stuff done on every call of the function */
Index: src/backend/executor/nodeFunctionscan.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/executor/nodeFunctionscan.c,v
retrieving revision 1.6
diff -c -r1.6 nodeFunctionscan.c
*** src/backend/executor/nodeFunctionscan.c    29 Aug 2002 00:17:04 -0000    1.6
--- src/backend/executor/nodeFunctionscan.c    29 Aug 2002 04:38:01 -0000
***************
*** 96,101 ****
--- 96,102 ----
          {
              bool                isNull;
              ExprDoneCond        isDone;
+             ExprContext           *econtext = scanstate->csstate.cstate.cs_ExprContext;

              isNull = false;
              isDone = ExprSingleResult;
***************
*** 108,113 ****
--- 109,116 ----

              if (isDone != ExprMultipleResult)
                  break;
+
+             ResetExprContext(econtext);
          }

          /*
Index: src/backend/utils/adt/lockfuncs.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/utils/adt/lockfuncs.c,v
retrieving revision 1.3
diff -c -r1.3 lockfuncs.c
*** src/backend/utils/adt/lockfuncs.c    29 Aug 2002 00:17:05 -0000    1.3
--- src/backend/utils/adt/lockfuncs.c    29 Aug 2002 05:37:32 -0000
***************
*** 24,38 ****
  Datum
  pg_lock_status(PG_FUNCTION_ARGS)
  {
!     FuncCallContext        *funccxt;
!     LockData            *lockData;

      if (SRF_IS_FIRSTCALL())
      {
-         MemoryContext    oldcxt;
          TupleDesc        tupdesc;

!         funccxt = SRF_FIRSTCALL_INIT();
          tupdesc = CreateTemplateTupleDesc(5, WITHOUTOID);
          TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relation",
                             OIDOID, -1, 0, false);
--- 24,43 ----
  Datum
  pg_lock_status(PG_FUNCTION_ARGS)
  {
!     FuncCallContext       *funcctx;
!     LockData           *lockData;
!     MemoryContext        oldcontext;

      if (SRF_IS_FIRSTCALL())
      {
          TupleDesc        tupdesc;

!         /* create a function context for cross-call persistence */
!         funcctx = SRF_FIRSTCALL_INIT();
!
!         /* switch to memory context appropriate for multiple function calls */
!         oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
!
          tupdesc = CreateTemplateTupleDesc(5, WITHOUTOID);
          TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relation",
                             OIDOID, -1, 0, false);
***************
*** 45,54 ****
          TupleDescInitEntry(tupdesc, (AttrNumber) 5, "isgranted",
                             BOOLOID, -1, 0, false);

!         funccxt->slot = TupleDescGetSlot(tupdesc);
!         funccxt->attinmeta = TupleDescGetAttInMetadata(tupdesc);
!
!         oldcxt = MemoryContextSwitchTo(funccxt->fmctx);

          /*
           * Preload all the locking information that we will eventually format
--- 50,57 ----
          TupleDescInitEntry(tupdesc, (AttrNumber) 5, "isgranted",
                             BOOLOID, -1, 0, false);

!         funcctx->slot = TupleDescGetSlot(tupdesc);
!         funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);

          /*
           * Preload all the locking information that we will eventually format
***************
*** 56,70 ****
           * MemoryContext is reset when the SRF finishes, we don't need to
           * free it ourselves.
           */
!         funccxt->user_fctx = (LockData *) palloc(sizeof(LockData));

!         GetLockStatusData(funccxt->user_fctx);

!         MemoryContextSwitchTo(oldcxt);
      }

!     funccxt    = SRF_PERCALL_SETUP();
!     lockData = (LockData *) funccxt->user_fctx;

      while (lockData->currIdx < lockData->nelements)
      {
--- 59,73 ----
           * MemoryContext is reset when the SRF finishes, we don't need to
           * free it ourselves.
           */
!         funcctx->user_fctx = (LockData *) palloc(sizeof(LockData));

!         GetLockStatusData(funcctx->user_fctx);

!         MemoryContextSwitchTo(oldcontext);
      }

!     funcctx    = SRF_PERCALL_SETUP();
!     lockData = (LockData *) funcctx->user_fctx;

      while (lockData->currIdx < lockData->nelements)
      {
***************
*** 82,88 ****
          holder        = &(lockData->holders[currIdx]);
          lock        = &(lockData->locks[currIdx]);
          proc        = &(lockData->procs[currIdx]);
!         num_attrs    = funccxt->attinmeta->tupdesc->natts;

          values = (char **) palloc(sizeof(*values) * num_attrs);

--- 85,91 ----
          holder        = &(lockData->holders[currIdx]);
          lock        = &(lockData->locks[currIdx]);
          proc        = &(lockData->procs[currIdx]);
!         num_attrs    = funcctx->attinmeta->tupdesc->natts;

          values = (char **) palloc(sizeof(*values) * num_attrs);

***************
*** 133,144 ****

          strncpy(values[3], GetLockmodeName(mode), 32);

!         tuple = BuildTupleFromCStrings(funccxt->attinmeta, values);
!         result = TupleGetDatum(funccxt->slot, tuple);
!         SRF_RETURN_NEXT(funccxt, result);
      }

!     SRF_RETURN_DONE(funccxt);
  }

  static LOCKMODE
--- 136,147 ----

          strncpy(values[3], GetLockmodeName(mode), 32);

!         tuple = BuildTupleFromCStrings(funcctx->attinmeta, values);
!         result = TupleGetDatum(funcctx->slot, tuple);
!         SRF_RETURN_NEXT(funcctx, result);
      }

!     SRF_RETURN_DONE(funcctx);
  }

  static LOCKMODE
Index: src/backend/utils/fmgr/funcapi.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/utils/fmgr/funcapi.c,v
retrieving revision 1.3
diff -c -r1.3 funcapi.c
*** src/backend/utils/fmgr/funcapi.c    29 Aug 2002 00:17:05 -0000    1.3
--- src/backend/utils/fmgr/funcapi.c    29 Aug 2002 04:32:33 -0000
***************
*** 59,72 ****
          retval->slot = NULL;
          retval->user_fctx = NULL;
          retval->attinmeta = NULL;
!         retval->fmctx = fcinfo->flinfo->fn_mcxt;

          /*
           * save the pointer for cross-call use
           */
          fcinfo->flinfo->fn_extra = retval;

-         /* back to the original memory context */
          MemoryContextSwitchTo(oldcontext);
      }
      else    /* second and subsequent calls */
--- 59,71 ----
          retval->slot = NULL;
          retval->user_fctx = NULL;
          retval->attinmeta = NULL;
!         retval->multi_call_memory_ctx = fcinfo->flinfo->fn_mcxt;

          /*
           * save the pointer for cross-call use
           */
          fcinfo->flinfo->fn_extra = retval;

          MemoryContextSwitchTo(oldcontext);
      }
      else    /* second and subsequent calls */
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.85
diff -c -r1.85 guc.c
*** src/backend/utils/misc/guc.c    29 Aug 2002 00:17:05 -0000    1.85
--- src/backend/utils/misc/guc.c    29 Aug 2002 05:37:32 -0000
***************
*** 2421,2426 ****
--- 2421,2427 ----
      int                    max_calls;
      TupleTableSlot       *slot;
      AttInMetadata       *attinmeta;
+     MemoryContext        oldcontext;

      /* stuff done only on the first call of the function */
       if(SRF_IS_FIRSTCALL())
***************
*** 2428,2433 ****
--- 2429,2437 ----
          /* create a function context for cross-call persistence */
           funcctx = SRF_FIRSTCALL_INIT();

+         /* switch to memory context appropriate for multiple function calls */
+         oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
          /* need a tuple descriptor representing two TEXT columns */
          tupdesc = CreateTemplateTupleDesc(2, WITHOUTOID);
          TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
***************
*** 2450,2455 ****
--- 2454,2461 ----

          /* total number of tuples to be returned */
          funcctx->max_calls = GetNumConfigOptions();
+
+         MemoryContextSwitchTo(oldcontext);
      }

      /* stuff done on every call of the function */
Index: src/include/funcapi.h
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/include/funcapi.h,v
retrieving revision 1.5
diff -c -r1.5 funcapi.h
*** src/include/funcapi.h    29 Aug 2002 00:17:06 -0000    1.5
--- src/include/funcapi.h    29 Aug 2002 05:37:32 -0000
***************
*** 101,113 ****
      AttInMetadata       *attinmeta;

      /*
!      * memory context used to initialize structure
       *
!      * fmctx is set by SRF_FIRSTCALL_INIT() for you, and used by
!      * SRF_RETURN_DONE() for cleanup. It is primarily for internal use
!      * by the API.
       */
!     MemoryContext    fmctx;

  }    FuncCallContext;

--- 101,113 ----
      AttInMetadata       *attinmeta;

      /*
!      * memory context used for structures which must live for multiple calls
       *
!      * multi_call_memory_ctx is set by SRF_FIRSTCALL_INIT() for you, and used
!      * by SRF_RETURN_DONE() for cleanup. It is the most appropriate memory
!      * context for any memory that is allocated for use in multiple calls.
       */
!     MemoryContext    multi_call_memory_ctx;

  }    FuncCallContext;

***************
*** 160,176 ****
   * {
   *     FuncCallContext       *funcctx;
   *     Datum                result;
   *     <user defined declarations>
   *
   *     if(SRF_IS_FIRSTCALL())
   *     {
-  *         <user defined code>
   *         funcctx = SRF_FIRSTCALL_INIT();
   *         <if returning composite>
   *             <obtain slot>
   *             funcctx->slot = slot;
   *         <endif returning composite>
   *         <user defined code>
   *  }
   *     <user defined code>
   *     funcctx = SRF_PERCALL_SETUP();
--- 160,179 ----
   * {
   *     FuncCallContext       *funcctx;
   *     Datum                result;
+  *  MemoryContext        oldcontext;
   *     <user defined declarations>
   *
   *     if(SRF_IS_FIRSTCALL())
   *     {
   *         funcctx = SRF_FIRSTCALL_INIT();
+  *        oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+  *         <user defined code>
   *         <if returning composite>
   *             <obtain slot>
   *             funcctx->slot = slot;
   *         <endif returning composite>
   *         <user defined code>
+  *        MemoryContextSwitchTo(oldcontext);
   *  }
   *     <user defined code>
   *     funcctx = SRF_PERCALL_SETUP();
***************
*** 179,184 ****
--- 182,190 ----
   *     if (funcctx->call_cntr < funcctx->max_calls)
   *     {
   *         <user defined code>
+  *        oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+  *      <user allocated memory needed in later function calls>
+  *        MemoryContextSwitchTo(oldcontext);
   *         <obtain result Datum>
   *         SRF_RETURN_NEXT(funcctx, result);
   *     }