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

"Bill Rugolsky Jr." <brugolsky(at)telemetry-investments(dot)com> 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 ----