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

Commit d3e41cb

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 1eef280 commit d3e41cb

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.127.4.9 2010/07/05 09:27:49 heikki Exp $
6+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.127.4.10 2010/08/09 18:51:02 tgl Exp $
77
*
88
* This software is copyrighted by Jan Wieck - Hamburg.
99
*
@@ -970,6 +970,9 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
970970
*/
971971
SPI_restore_connection();
972972

973+
/* Must clean up the econtext too */
974+
exec_eval_cleanup(estate);
975+
973976
/* Look for a matching exception handler */
974977
exceptions = block->exceptions;
975978
for (j = 0; j < exceptions->exceptions_used; j++)
@@ -2067,6 +2070,9 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
20672070
*
20682071
* NB: the result of the evaluation is no longer valid after this is done,
20692072
* unless it is a pass-by-value datatype.
2073+
*
2074+
* NB: if you change this code, see also the hacks in exec_assign_value's
2075+
* PLPGSQL_DTYPE_ARRAYELEM case.
20702076
* ----------
20712077
*/
20722078
static void
@@ -2866,6 +2872,10 @@ exec_assign_expr(PLpgSQL_execstate *estate, PLpgSQL_datum *target,
28662872

28672873
/* ----------
28682874
* exec_assign_value Put a value into a target field
2875+
*
2876+
* Note: in some code paths, this may leak memory in the eval_econtext;
2877+
* we assume that will be cleaned up later by exec_eval_cleanup. We cannot
2878+
* call exec_eval_cleanup here for fear of destroying the input Datum value.
28692879
* ----------
28702880
*/
28712881
static void
@@ -3131,6 +3141,9 @@ exec_assign_value(PLpgSQL_execstate *estate,
31313141

31323142
case PLPGSQL_DT 8000 YPE_ARRAYELEM:
31333143
{
3144+
/*
3145+
* Target is an element of an array
3146+
*/
31343147
int nsubscripts;
31353148
int i;
31363149
PLpgSQL_expr *subscripts[MAXDIM];
@@ -3147,10 +3160,19 @@ exec_assign_value(PLpgSQL_execstate *estate,
31473160
coerced_value;
31483161
ArrayType *oldarrayval;
31493162
ArrayType *newarrayval;
3163+
SPITupleTable *save_eval_tuptable;
3164+
3165+
/*
3166+
* We need to do subscript evaluation, which might require
3167+
* evaluating general expressions; and the caller might have
3168+
* done that too in order to prepare the input Datum. We
3169+
* have to save and restore the caller's SPI_execute result,
3170+
* if any.
3171+
*/
3172+
save_eval_tuptable = estate->eval_tuptable;
3173+
estate->eval_tuptable = NULL;
31503174

31513175
/*
3152-
* Target is an element of an array
3153-
*
31543176
* To handle constructs like x[1][2] := something, we have to
31553177
* be prepared to deal with a chain of arrayelem datums.
31563178
* Chase back to find the base array datum, and save the
@@ -3202,8 +3224,23 @@ exec_assign_value(PLpgSQL_execstate *estate,
32023224
subscripts[nsubscripts - 1 - i],
32033225
&subisnull);
32043226
havenullsubscript |= subisnull;
3227+
3228+
/*
3229+
* Clean up in case the subscript expression wasn't simple.
3230+
* We can't do exec_eval_cleanup, but we can do this much
3231+
* (which is safe because the integer subscript value is
3232+
* surely pass-by-value), and we must do it in case the
3233+
* next subscript expression isn't simple either.
3234+
*/
3235+
if (estate->eval_tuptable != NULL)
3236+
SPI_freetuptable(estate->eval_tuptable);
3237+
estate->eval_tuptable = NULL;
32053238
}
32063239

3240+
/* Now we can restore caller's SPI_execute result if any. */
3241+
Assert(estate->eval_tuptable == NULL);
3242+
estate->eval_tuptable = save_eval_tuptable;
3243+
32073244
/*
32083245
* Skip the assignment if we have any nulls in the subscripts
32093246
* 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
@@ -2103,3 +2103,46 @@ select sp_add_user('user3');
21032103

21042104
drop function sp_add_user(text);
21052105
drop function sp_id_user(text);
2106+
-- Test for appropriate cleanup of non-simple expression evaluations
2107+
-- (bug in all versions prior to August 2010)
2108+
CREATE FUNCTION nonsimple_expr_test() RETURNS text[] AS $$
2109+
DECLARE
2110+
arr text[];
2111+
lr text;
2112+
i integer;
2113+
BEGIN
2114+
arr := array[array['foo','bar'], array['baz', 'quux']];
2115+
lr := 'fool';
2116+
i := 1;
2117+
-- use sub-SELECTs to make expressions non-simple
2118+
arr[(SELECT i)][(SELECT i+1)] := (SELECT lr);
2119+
RETURN arr;
2120+
END;
2121+
$$ LANGUAGE plpgsql;
2122+
SELECT nonsimple_expr_test();
2123+
nonsimple_expr_test
2124+
-------------------------
2125+
{{foo,fool},{baz,quux}}
2126+
(1 row)
2127+
2128+
DROP FUNCTION nonsimple_expr_test();
2129+
CREATE FUNCTION nonsimple_expr_test() RETURNS integer AS $$
2130+
declare
2131+
i integer NOT NULL := 0;
2132+
begin
2133+
begin
2134+
i := (SELECT NULL::integer); -- should throw error
2135+
exception
2136+
WHEN OTHERS THEN
2137+
i := (SELECT 1::integer);
2138+
end;
2139+
return i;
2140+
end;
2141+
$$ LANGUAGE plpgsql;
2142+
SELECT nonsimple_expr_test();
2143+
nonsimple_expr_test
2144+
---------------------
2145+
1
2146+
(1 row)
2147+
2148+
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
@@ -1807,3 +1807,43 @@ select sp_add_user('user3');
18071807

18081808
drop function sp_add_user(text);
18091809
drop function sp_id_user(text);
1810+
1811+
-- Test for appropriate cleanup of non-simple expression evaluations
1812+
-- (bug in all versions prior to August 2010)
1813+
1814+
CREATE FUNCTION nonsimple_expr_test() RETURNS text[] AS $$
1815+
DECLARE
1816+
arr text[];
1817+
lr text;
1818+
i integer;
1819+
BEGIN
1820+
arr := array[array['foo','bar'], array['baz', 'quux']];
1821+
lr := 'fool';
1822+
i := 1;
1823+
-- use sub-SELECTs to make expressions non-simple
1824+
arr[(SELECT i)][(SELECT i+1)] := (SELECT lr);
1825+
RETURN arr;
1826+
END;
1827+
$$ LANGUAGE plpgsql;
1828+
1829+
SELECT nonsimple_expr_test();
1830+
1831+
DROP FUNCTION nonsimple_expr_test();
1832+
1833+
CREATE FUNCTION nonsimple_expr_test() RETURNS integer AS $$
1834+
declare
1835+
i integer NOT NULL := 0;
1836+
begin
1837+
begin
1838+
i := (SELECT NULL::integer); -- should throw error
1839+
exception
1840+
WHEN OTHERS THEN
1841+
i := (SELECT 1::integer);
1842+
end;
1843+
return i;
1844+
end;
1845+
$$ LANGUAGE plpgsql;
1846+
1847+
SELECT nonsimple_expr_test();
1848+
1849+
DROP FUNCTION nonsimple_expr_test();

0 commit comments

Comments
 (0)
0