Re: plpgsql memory leak in 7.3.2? (repost)

"Mark Cave-Ayland" <m(dot)cave-ayland(at)webbased(dot)co(dot)uk> writes:
> Whilst continuing work on some of our large tables again, we've
> encountered what we think may be a memory leak in plpgsql in 7.3.2.

Yup, you're right. Looks like I introduced several memory leaks in
plpgsql when I modified spi.c to return a tuple descriptor even with
zero tuples returned: some plpgsql routines assumed they didn't need
to do SPI_freetuptable() after retrieving no tuples. Patch attached,
or you can grab the updated pl_exec.c from our CVS server.

regards, tom lane

*** src/pl/plpgsql/src/pl_exec.c~ Tue Jan 21 17:06:36 2003
--- src/pl/plpgsql/src/pl_exec.c Sun Mar 2 15:45:59 2003
***************
*** 1369,1379 ****
if (rc != PLPGSQL_RC_OK)
{
/*
! * We're aborting the loop, so cleanup and set FOUND
*/
- exec_set_found(estate, found);
SPI_freetuptable(tuptab);
SPI_cursor_close(portal);

if (rc == PLPGSQL_RC_EXIT)
{
--- 1369,1380 ----
if (rc != PLPGSQL_RC_OK)
{
/*
! * We're aborting the loop, so cleanup and set FOUND.
! * (This code should match the code after the loop.)
*/
SPI_freetuptable(tuptab);
SPI_cursor_close(portal);
+ exec_set_found(estate, found);

if (rc == PLPGSQL_RC_EXIT)
{
***************
*** 1411,1416 ****
--- 1412,1422 ----
}

/*
+ * Release last group of tuples
+ */
+ SPI_freetuptable(tuptab);
+
+ /*
* Close the implicit cursor
*/
SPI_cursor_close(portal);
***************
*** 2363,2369 ****
if (n == 0)
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
else
! found = true;

/*
* Now do the loop
--- 2369,2375 ----
if (n == 0)
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
else
! found = true; /* processed at least one tuple */

/*
* Now do the loop
***************
*** 2382,2395 ****
*/
rc = exec_stmts(estate, stmt->body);

- /*
- * We're aborting the loop, so cleanup and set FOUND
- */
if (rc != PLPGSQL_RC_OK)
{
! exec_set_found(estate, found);
SPI_freetuptable(tuptab);
SPI_cursor_close(portal);

if (rc == PLPGSQL_RC_EXIT)
{
--- 2388,2402 ----
*/
rc = exec_stmts(estate, stmt->body);

if (rc != PLPGSQL_RC_OK)
{
! /*
! * We're aborting the loop, so cleanup and set FOUND.
! * (This code should match the code after the loop.)
! */
SPI_freetuptable(tuptab);
SPI_cursor_close(portal);
+ exec_set_found(estate, found);

if (rc == PLPGSQL_RC_EXIT)
{
***************
*** 2427,2433 ****
}

/*
! * Close the cursor
*/
SPI_cursor_close(portal);

--- 2434,2445 ----
}

/*
! * Release last group of tuples
! */
! SPI_freetuptable(tuptab);
!
! /*
! * Close the implicit cursor
*/
SPI_cursor_close(portal);

***************
*** 2736,2759 ****
pfree(curname);

/* ----------
- * Initialize the global found variable to false
- * ----------
- */
- exec_set_found(estate, false);
-
- /* ----------
* Determine if we fetch into a record or a row
* ----------
*/
if (stmt->rec != NULL)
rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->recno]);
else
! {
! if (stmt->row != NULL)
! row = (PLpgSQL_row *) (estate->datums[stmt->row->rowno]);
! else
! elog(ERROR, "unsupported target in exec_stmt_select()");
! }

/* ----------
* Fetch 1 tuple from the cursor
--- 2748,2762 ----
pfree(curname);

/* ----------
* Determine if we fetch into a record or a row
* ----------
*/
if (stmt->rec != NULL)
rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->recno]);
+ else if (stmt->row != NULL)
+ row = (PLpgSQL_row *) (estate->datums[stmt->row->rowno]);
else
! elog(ERROR, "unsupported target in exec_stmt_fetch()");

/* ----------
* Fetch 1 tuple from the cursor
***************
*** 2764,2785 ****
n = SPI_processed;

/* ----------
! * If the FETCH didn't return a row, set the target
! * to NULL and return with FOUND = false.
* ----------
*/
if (n == 0)
{
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
! return PLPGSQL_RC_OK;
}
-
- /* ----------
- * Put the result into the target and set found to true
- * ----------
- */
- exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
- exec_set_found(estate, true);

SPI_freetuptable(tuptab);

--- 2767,2785 ----
n = SPI_processed;

/* ----------
! * Set the target and the global FOUND variable appropriately.
* ----------
*/
if (n == 0)
{
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
! exec_set_found(estate, false);
! }
! else
! {
! exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
! exec_set_found(estate, true);
}

SPI_freetuptable(tuptab);