Out-of-Memory with ROWTYPE assignment in PL/pgSQL FOR loop

Hello,

The PL/pgSQL FOR loop in the function consume_memory() defined below
will consume VM on each iteration until the process hits its ulimit.
The problem occurs with variables of ROWTYPE; there is no unbounded
allocation when using simple types such as integer or varchar.  Before I
delve into the SPI code, perhaps someone familiar with PostgreSQL internals
can spot the resource leak.

Tested with 8.0.1 and CVS head as of 2005-06-20:

-- Start of test code

-- create a table with ten million rows
CREATE TEMPORARY TABLE ten ( n integer DEFAULT 0 ) ;
INSERT INTO ten VALUES (0);
INSERT INTO ten VALUES (1);
INSERT INTO ten VALUES (2);
INSERT INTO ten VALUES (3);
INSERT INTO ten VALUES (4);
INSERT INTO ten VALUES (5);
INSERT INTO ten VALUES (6);
INSERT INTO ten VALUES (7);
INSERT INTO ten VALUES (8);
INSERT INTO ten VALUES (9);
CREATE TEMPORARY TABLE thousand AS
   SELECT 100*i.n + 10*j.n + k.n AS n FROM ten AS i, ten as j, ten as k ;
CREATE TEMPORARY TABLE tenmillion AS
   SELECT 10000*i.n + 10*j.n + k.n AS n FROM thousand AS i, thousand as j, ten as k ;

-- a function to consume memory
CREATE OR REPLACE FUNCTION consume_memory()
RETURNS void AS $PROC$
DECLARE
   rec tenmillion%ROWTYPE ;
   prev tenmillion%ROWTYPE ;
BEGIN
   FOR rec IN SELECT * FROM tenmillion LOOP
      prev := rec ;
   END LOOP ;
   RETURN ;
END
$PROC$ LANGUAGE plpgsql;

-- Until this point, memory usage is approximately constant.
-- Evaluating the above function will rapidly consume VM.

SELECT consume_memory() ;

-- End of test code

Here's a record of Committed_AS from /proc/meminfo on a Linux 2.6 system,
over the course of the test:

rugolsky@ti64: while : ; do grep Committed_AS /proc/meminfo ; sleep 1 ; done
Committed_AS:   225592 kB
Committed_AS:   225592 kB
Committed_AS:   233692 kB <- Started
Committed_AS:   258280 kB  
Committed_AS:   282868 kB
Committed_AS:   299260 kB  
Committed_AS:   323848 kB
Committed_AS:   340232 kB
Committed_AS:   348436 kB
Committed_AS:   356632 kB
Committed_AS:   381220 kB
Committed_AS:   397612 kB
Committed_AS:   414004 kB
Committed_AS:   422200 kB
Committed_AS:   438592 kB
Committed_AS:   463180 kB
Committed_AS:   487768 kB
Committed_AS:   504160 kB
Committed_AS:   504160 kB
Committed_AS:   520552 kB
Committed_AS:   545140 kB
Committed_AS:   569728 kB
Committed_AS:   586120 kB
Committed_AS:   586120 kB
Committed_AS:   602512 kB
Committed_AS:   225640 kB <- Cancelled

Regards,

        Bill Rugolsky

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
      joining column's datatypes do not match

"Bill Rugolsky Jr." <[hidden email]> writes:
> The PL/pgSQL FOR loop in the function consume_memory() defined below
> will consume VM on each iteration until the process hits its ulimit.
> The problem occurs with variables of ROWTYPE; there is no unbounded
> allocation when using simple types such as integer or varchar.

Yeah, looks like I introduced a memory leak with the 8.0 changes for
better support of rowtype variables :-(.  Here's the patch.

                        regards, tom lane


Index: pl_exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.127.4.2
diff -c -r1.127.4.2 pl_exec.c
*** pl_exec.c 20 Jun 2005 20:44:50 -0000 1.127.4.2
--- pl_exec.c 20 Jun 2005 22:46:14 -0000
***************
*** 2003,2013 ****
  estate->eval_tuptable = NULL;
  estate->eval_processed = 0;
  estate->eval_lastoid = InvalidOid;
- estate->eval_econtext = NULL;
 
  estate->err_func = func;
  estate->err_stmt = NULL;
  estate->err_text = NULL;
  }
 
  /* ----------
--- 2003,2032 ----
  estate->eval_tuptable = NULL;
  estate->eval_processed = 0;
  estate->eval_lastoid = InvalidOid;
 
  estate->err_func = func;
  estate->err_stmt = NULL;
  estate->err_text = NULL;
+
+ /*
+ * Create an EState for evaluation of simple expressions, if there's
+ * not one already in the current transaction. The EState is made a
+ * child of TopTransactionContext so it will have the right lifespan.
+ */
+ if (simple_eval_estate == NULL)
+ {
+ MemoryContext oldcontext;
+
+ oldcontext = MemoryContextSwitchTo(TopTransactionContext);
+ simple_eval_estate = CreateExecutorState();
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ /*
+ * Create an expression context for simple expressions.
+ * This must be a child of simple_eval_estate.
+ */
+ estate->eval_econtext = CreateExprContext(simple_eval_estate);
  }
 
  /* ----------
***************
*** 3238,3243 ****
--- 3257,3264 ----
  Datum *value,
  bool *isnull)
  {
+ MemoryContext oldcontext;
+
  switch (datum->dtype)
  {
  case PLPGSQL_DTYPE_VAR:
***************
*** 3264,3272 ****
--- 3285,3295 ----
  elog(ERROR, "row variable has no tupdesc");
  /* Make sure we have a valid type/typmod setting */
  BlessTupleDesc(row->rowtupdesc);
+ oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
  tup = make_tuple_from_row(estate, row, row->rowtupdesc);
  if (tup == NULL) /* should not happen */
  elog(ERROR, "row not compatible with its own tupdesc");
+ MemoryContextSwitchTo(oldcontext);
  *typeid = row->rowtupdesc->tdtypeid;
  *value = HeapTupleGetDatum(tup);
  *isnull = false;
***************
*** 3299,3308 ****
--- 3322,3333 ----
  * fields. Copy the tuple body and insert the right
  * values.
  */
+ oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
  heap_copytuple_with_tuple(rec->tup, &worktup);
  HeapTupleHeaderSetDatumLength(worktup.t_data, worktup.t_len);
  HeapTupleHeaderSetTypeId(worktup.t_data, rec->tupdesc->tdtypeid);
  HeapTupleHeaderSetTypMod(worktup.t_data, rec->tupdesc->tdtypmod);
+ MemoryContextSwitchTo(oldcontext);
  *typeid = rec->tupdesc->tdtypeid;
  *value = HeapTupleGetDatum(&worktup);
  *isnull = false;
***************
*** 3579,3585 ****
   Oid *rettype)
  {
  Datum retval;
! ExprContext * volatile econtext;
  ParamListInfo paramLI;
  int i;
  Snapshot saveActiveSnapshot;
--- 3604,3610 ----
   Oid *rettype)
  {
  Datum retval;
! ExprContext *econtext = estate->eval_econtext;
  ParamListInfo paramLI;
  int i;
  Snapshot saveActiveSnapshot;
***************
*** 3590,3609 ****
  *rettype = expr->expr_simple_type;
 
  /*
- * Create an EState for evaluation of simple expressions, if there's
- * not one already in the current transaction. The EState is made a
- * child of TopTransactionContext so it will have the right lifespan.
- */
- if (simple_eval_estate == NULL)
- {
- MemoryContext oldcontext;
-
- oldcontext = MemoryContextSwitchTo(TopTransactionContext);
- simple_eval_estate = CreateExecutorState();
- MemoryContextSwitchTo(oldcontext);
- }
-
- /*
  * Prepare the expression for execution, if it's not been done already
  * in the current transaction.
  */
--- 3615,3620 ----
***************
*** 3617,3634 ****
  }
 
  /*
- * Create an expression context for simple expressions, if there's not
- * one already in the current function call.  This must be a child of
- * simple_eval_estate.
- */
- econtext = estate->eval_econtext;
- if (econtext == NULL)
- {
- econtext = CreateExprContext(simple_eval_estate);
- estate->eval_econtext = econtext;
- }
-
- /*
  * Param list can live in econtext's temporary memory context.
  *
  * XXX think about avoiding repeated palloc's for param lists? Beware
--- 3628,3633 ----

---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend

On Mon, Jun 20, 2005 at 06:54:20PM -0400, Tom Lane wrote:
> "Bill Rugolsky Jr." <[hidden email]> writes:
> > The PL/pgSQL FOR loop in the function consume_memory() defined below
> > will consume VM on each iteration until the process hits its ulimit.
> > The problem occurs with variables of ROWTYPE; there is no unbounded
> > allocation when using simple types such as integer or varchar.
>
> Yeah, looks like I introduced a memory leak with the 8.0 changes for
> better support of rowtype variables :-(.  Here's the patch.

Thank you for the quick reply; much appreciated!
Applied and tested against CVS head; that plugged the leak.

        -Bill

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings