8000 Fix incorrect logic in plpgsql for cleanup after evaluation of non-si… · danielcode/postgres@d0844a8 · GitHub
[go: up one dir, main page]
Skip to content

Commit d0844a8

Browse files
tglsfdctglsfdc
committed
Fix incorrect logic in plpgsql for cleanup after evaluation of non-simple
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
1 parent 2f7cd43 commit d0844a8

File tree

3 files changed

+123
-3
lines changed

3 files changed

+123
-3
lines changed

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* procedural language
44
*
55
* IDENTIFICATION
6-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.154.2.11 2010/07/05 09:27:42 heikki Exp $
6+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.154.2.12 2010/08/09 18:50:53 tgl Exp $
77
*
88
* This software is copyrighted by Jan Wieck - Hamburg.
99
*
@@ -920,6 +920,9 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
920920
*/
921921
SPI_restore_connection();
922922

923+
/* Must clean up the econtext too */
924+
exec_eval_cleanup(estate);
925+
923926
/* Look for a matching exception handler */
924927
foreach(e, block->exceptions->exc_list)
925928
{
@@ -2210,6 +2213,9 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
22102213
*
22112214
* NB: the result of the evaluation is no longer valid after this is done,
22122215
* unless it is a pass-by-value datatype.
2216+
*
2217+
* NB: if you change this code, see also the hacks in exec_assign_value's
2218+
* PLPGSQL_DTYPE_ARRAYELEM case.
22132219
* ----------
22142220
*/
22152221
static void
@@ -3049,6 +3055,10 @@ exec_assign_expr(PLpgSQL_execstate *estate, PLpgSQL_datum *target,
30493055

30503056
/* ----------
30513057
* exec_assign_value Put a value into a target field
3058+
*
3059+
* Note: in some code paths, this may leak memory in the eval_econtext;
3060+
* we assume that will be cleaned up later by exec_eval_cleanup. We cannot
3061+
* call exec_eval_cleanup here for fear of destroying the input Datum value.
30523062
* ----------
30533063
*/
30543064
static void
@@ -3310,6 +3320,9 @@ exec_assign_value(PLpgSQL_execstate *estate,
33103320

33113321
case PLPGSQL_DTYPE_ARRAYELEM:
33123322
{
3323+
/*
3324+
* Target is an element of an array
3325+
*/
33133326
int nsubscripts;
33143327
int i;
33153328
PLpgSQL_expr *subscripts[MAXDIM];
@@ -3326,10 +3339,19 @@ exec_assign_value(PLpgSQL_execstate *estate,
33263339
coerced_value;
33273340
ArrayType *oldarrayval;
33283341
ArrayType *newarrayval;
3342+
SPITupleTable *save_eval_tuptable;
3343+
3344+
/*
3345+
* We need to do subscript evaluation, which might require
3346+
* evaluating general expressions; and the caller might have
3347+
* done that too in order to prepare the input Datum. We
3348+
* have to save and restore the caller's SPI_execute result,
3349+
* if any.
3350+
*/
3351+
save_eval_tuptable = estate->eval_tuptable;
3352+
estate->eval_tuptable = NULL;
33293353

33303354
/*
3331-
* Target is an element of an array
3332-
*
33333355
* To handle constructs like x[1][2] := something, we have to
33343356
* be prepared to deal with a chain of arrayelem datums. Chase
33353357
* back to find the base array datum, and save the subscript
@@ -3380,8 +3402,23 @@ exec_assign_value(PLpgSQL_execstate *estate,
33803402
subscripts[nsubscripts - 1 - i],
33813403
&subisnull);
33823404
havenullsubscript |= subisnull;
3405+
3406+
/*
3407+
* Clean up in case the subscript expression wasn't simple.
3408+
* We can't do exec_eval_cleanup, but we can do this much
3409+
* (which is safe because the integer subscript value is
3410+
* surely pass-by-value), and we must do it in case the
3411+
* next subscript expression isn't simple either.
3412+
*/
3413+
if (estate->eval_tuptable != NULL)
3414+
SPI_freetuptable(estate->eval_tuptable);
3415+
estate->eval_tuptable = NULL;
33833416
}
33843417

3418+
/* Now we can restore caller's SPI_execute result if any. */
3419+
Assert(estate->eval_tuptable == NULL);
3420+
estate->eval_tuptable = save_eval_tuptable;
3421+
33853422
/*
33863423
* Skip the assignment if we have any nulls in the subscripts
33873424
* or the righthand side. This is pretty bogus but it

‎src/test/regress/expected/plpgsql.out

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2739,3 +2739,46 @@ select multi_datum_use(42);
27392739
t
27402740
(1 row)
27412741

2742+
-- Test for appropriate cleanup of non-simple expression evaluations
2743+
-- (bug in all versions prior to August 2010)
2744+
CREATE FUNCTION nonsimple_expr_test() RETURNS text[] AS $$
2745+
DECLARE
2746+
arr text[];
2747+
lr text;
2748+
i integer;
2749+
BEGIN
2750+
arr := array[array['foo','bar'], array['baz', 'quux']];
2751+
lr := 'fool';
2752+
i := 1;
2753+
-- use sub-SELECTs to make expressions non-simple
2754+
arr[(SELECT i)][(SELECT i+1)] := (SELECT lr);
2755+
RETURN arr;
2756+
END;
2757+
$$ LANGUAGE plpgsql;
2758+
SELECT nonsimple_expr_test();
2759+
nonsimple_expr_test
2760+
-------------------------
2761+
{{foo,fool},{baz,quux}}
2762+
(1 row)
2763+
2764+
DROP FUNCTION nonsimple_expr_test();
2765+
CREATE FUNCTION nonsimple_expr_test() RETURNS integer AS $$
2766+
declare
2767+
i integer NOT NULL := 0;
2768+
begin
2769+
begin
2770+
i := (SELECT NULL::integer); -- should throw error
2771+
exception
2772+
WHEN OTHERS THEN
2773+
i := (SELECT 1::integer);
2774+
end;
2775+
return i;
2776+
end;
2777+
$$ LANGUAGE plpgsql;
2778+
SELECT nonsimple_expr_test();
2779+
nonsimple_expr_test
2780+
---------------------
2781+
1
2782+
(1 row)
2783+
2784+
DROP FUNCTION nonsimple_expr_test();

‎src/test/regress/sql/plpgsql.sql

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2295,3 +2295,43 @@ begin
22952295
end$$ language plpgsql;
22962296

22972297
select multi_datum_use(42);
2298+
2299+
-- Test for appropriate cleanup of non-simple expression evaluations
2300+
-- (bug in all versions prior to August 2010)
2301+
2302+
CREATE FUNCTION nonsimple_expr_test() RETURNS text[] AS $$
2303+
DECLARE
2304+
arr text[];
2305+
lr text;
2306+
i integer;
2307+
BEGIN
2308+
arr := array[array['foo','bar'], array['baz', 'quux']];
2309+
lr := 'fool';
2310+
i := 1;
2311+
-- use sub-SELECTs to make expressions non-simple
2312+
arr[(SELECT i)][(SELECT i+1)] := (SELECT lr);
2313+
RETURN arr;
2314+
END;
2315+
$$ LANGUAGE plpgsql;
2316+
2317+
SELECT nonsimple_expr_test();
2318+
2319+
DROP FUNCTION nonsimple_expr_test();
2320+
2321+
CREATE FUNCTION nonsimple_expr_test() RETURNS integer AS $$
2322+
declare
2323+
i integer NOT NULL := 0;
2324+
begin
2325+
begin
2326+
i := (SELECT NULL::integer); -- should throw error
2327+
exception
2328+
WHEN OTHERS THEN
2329+
i := (SELECT 1::integer);
2330+
end;
2331+
return i;
2332+
end;
2333+
$$ LANGUAGE plpgsql;
2334+
2335+
SELECT nonsimple_expr_test();
2336+
2337+
DROP FUNCTION nonsimple_expr_test();

0 commit comments

Comments
 (0)
0