mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix incorrect logic in plpgsql for cleanup after evaluation of non-simple

Browse Source 
expressions.  We need to deal with this when handling subscripts in an array
assignment, and also when catching an exception.  In an Assert-enabled build
these omissions led to Assert failures, but I think in a normal build the
only consequence would be short-term memory leakage; which may explain why
this wasn't reported from the field long ago.

Back-patch to all supported versions.  7.4 doesn't have exceptions, but
otherwise these bugs go all the way back.

Heikki Linnakangas and Tom Lane
This commit is contained in:
Tom Lane 2010-08-09 18:50:29 +00:00
parent 4e49ea084e
commit 63c232b505
3 changed files with 123 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
src/pl/plpgsql/src/pl_exec.c
View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.244.2.6 2010/07/05 09:27:24 heikki Exp $
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.244.2.7 2010/08/09 18:50:29 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -1076,6 +1076,9 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
*/
SPI_restore_connection();
/* Must clean up the econtext too */
exec_eval_cleanup(estate);
/* Look for a matching exception handler */
foreach(e, block->exceptions->exc_list)
{
@ -2636,6 +2639,9 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
*
* NB: the result of the evaluation is no longer valid after this is done,
* unless it is a pass-by-value datatype.
*
* NB: if you change this code, see also the hacks in exec_assign_value's
* PLPGSQL_DTYPE_ARRAYELEM case.
* ----------
*/
static void
@ -3443,6 +3449,10 @@ exec_assign_expr(PLpgSQL_execstate *estate, PLpgSQL_datum *target,
/* ----------
* exec_assign_value Put a value into a target field
*
* Note: in some code paths, this may leak memory in the eval_econtext;
* we assume that will be cleaned up later by exec_eval_cleanup. We cannot
* call exec_eval_cleanup here for fear of destroying the input Datum value.
* ----------
*/
static void
@ -3693,6 +3703,9 @@ exec_assign_value(PLpgSQL_execstate *estate,
case PLPGSQL_DTYPE_ARRAYELEM:
{
/*
* Target is an element of an array
*/
int nsubscripts;
int i;
PLpgSQL_expr *subscripts[MAXDIM];
@ -3708,10 +3721,19 @@ exec_assign_value(PLpgSQL_execstate *estate,
coerced_value;
ArrayType *oldarrayval;
ArrayType *newarrayval;
SPITupleTable *save_eval_tuptable;
/*
* We need to do subscript evaluation, which might require
* evaluating general expressions; and the caller might have
* done that too in order to prepare the input Datum. We
* have to save and restore the caller's SPI_execute result,
* if any.
*/
save_eval_tuptable = estate->eval_tuptable;
estate->eval_tuptable = NULL;
/*
* Target is an element of an array
*
* To handle constructs like x[1][2] := something, we have to
* be prepared to deal with a chain of arrayelem datums. Chase
* back to find the base array datum, and save the subscript
@ -3765,8 +3787,23 @@ exec_assign_value(PLpgSQL_execstate *estate,
ereport(ERROR,
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
errmsg("array subscript in assignment must not be null")));
/*
* Clean up in case the subscript expression wasn't simple.
* We can't do exec_eval_cleanup, but we can do this much
* (which is safe because the integer subscript value is
* surely pass-by-value), and we must do it in case the
* next subscript expression isn't simple either.
*/
if (estate->eval_tuptable != NULL)
SPI_freetuptable(estate->eval_tuptable);
estate->eval_tuptable = NULL;
}
/* Now we can restore caller's SPI_execute result if any. */
Assert(estate->eval_tuptable == NULL);
estate->eval_tuptable = save_eval_tuptable;
/* Coerce source value to match array element type. */
coerced_value = exec_simple_cast_value(value,
valtype,

43
src/test/regress/expected/plpgsql.out
View File

@ -3737,6 +3737,49 @@ SELECT * FROM leaker_1(true);
DROP FUNCTION leaker_1(bool);
DROP FUNCTION leaker_2(bool);
-- Test for appropriate cleanup of non-simple expression evaluations
-- (bug in all versions prior to August 2010)
CREATE FUNCTION nonsimple_expr_test() RETURNS text[] AS $$
DECLARE
arr text[];
lr text;
i integer;
BEGIN
arr := array[array['foo','bar'], array['baz', 'quux']];
lr := 'fool';
i := 1;
-- use sub-SELECTs to make expressions non-simple
arr[(SELECT i)][(SELECT i+1)] := (SELECT lr);
RETURN arr;
END;
$$ LANGUAGE plpgsql;
SELECT nonsimple_expr_test();
nonsimple_expr_test
-------------------------
{{foo,fool},{baz,quux}}
(1 row)
DROP FUNCTION nonsimple_expr_test();
CREATE FUNCTION nonsimple_expr_test() RETURNS integer AS $$
declare
i integer NOT NULL := 0;
begin
begin
i := (SELECT NULL::integer); -- should throw error
exception
WHEN OTHERS THEN
i := (SELECT 1::integer);
end;
return i;
end;
$$ LANGUAGE plpgsql;
SELECT nonsimple_expr_test();
nonsimple_expr_test
---------------------
1
(1 row)
DROP FUNCTION nonsimple_expr_test();
-- Test handling of string literals.
set standard_conforming_strings = off;
create or replace function strtest() returns text as $$

40
src/test/regress/sql/plpgsql.sql
View File

@ -3006,6 +3006,46 @@ SELECT * FROM leaker_1(true);
DROP FUNCTION leaker_1(bool);
DROP FUNCTION leaker_2(bool);
-- Test for appropriate cleanup of non-simple expression evaluations
-- (bug in all versions prior to August 2010)
CREATE FUNCTION nonsimple_expr_test() RETURNS text[] AS $$
DECLARE
arr text[];
lr text;
i integer;
BEGIN
arr := array[array['foo','bar'], array['baz', 'quux']];
lr := 'fool';
i := 1;
-- use sub-SELECTs to make expressions non-simple
arr[(SELECT i)][(SELECT i+1)] := (SELECT lr);
RETURN arr;
END;
$$ LANGUAGE plpgsql;
SELECT nonsimple_expr_test();
DROP FUNCTION nonsimple_expr_test();
CREATE FUNCTION nonsimple_expr_test() RETURNS integer AS $$
declare
i integer NOT NULL := 0;
begin
begin
i := (SELECT NULL::integer); -- should throw error
exception
WHEN OTHERS THEN
i := (SELECT 1::integer);
end;
return i;
end;
$$ LANGUAGE plpgsql;
SELECT nonsimple_expr_test();
DROP FUNCTION nonsimple_expr_test();
-- Test handling of string literals.
set standard_conforming_strings = off;