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

Commit ef2b138

Browse files
tglsfdctglsfdc
committed
Fix incorrect logic in plpgsql for cleanup after evaluation of non-simple
expressions. We need to deal with this when hand 10000 ling 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 10c0565 commit ef2b138

File tree

3 files changed

+80
-2
lines changed

3 files changed

+80
-2
lines changed

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* procedural language
44
*
55
* IDENTIFICATION
6-
* $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.4 2010/07/05 09:27:57 heikki Exp $
6+
* $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.5 2010/08/09 18:51:12 tgl Exp $
77
*
88
* This software is copyrighted by Jan Wieck - Hamburg.
99
*
@@ -1919,6 +1919,9 @@ plpgsql_estate_setup(PLpgSQL_execstate * estate,
19191919
*
19201920
* NB: the result of the evaluation is no longer valid after this is done,
19211921
* unless it is a pass-by-value datatype.
1922+
*
1923+
* NB: if you change this code, see also the hacks in exec_assign_value's
1924+
* PLPGSQL_DTYPE_ARRAYELEM case.
19221925
* ----------
19231926
*/
19241927
static void
@@ -2674,6 +2677,10 @@ exec_assign_expr(PLpgSQL_execstate * estate, PLpgSQL_datum * target,
26742677

26752678
/* ----------
26762679
* exec_assign_value Put a value into a target field
2680+
*
2681+
* Note: in some code paths, this may leak memory in the eval_econtext;
2682+
* we assume that will be cleaned up later by exec_eval_cleanup. We cannot
2683+
* call exec_eval_cleanup here for fear of destroying the input Datum value.
26772684
* ----------
26782685
*/
26792686
static void
@@ -2708,6 +2715,7 @@ exec_assign_value(PLpgSQL_execstate * estate,
27082715
Datum oldarrayval,
27092716
coerced_value;
27102717
ArrayType *newarrayval;
2718+
SPITupleTable *save_eval_tuptable;
27112719
HeapTuple newtup;
27122720

27132721
switch (target->dtype)
@@ -2868,6 +2876,16 @@ exec_assign_value(PLpgSQL_execstate * estate,
28682876
/*
28692877
* Target is an element of an array
28702878
*
2879+
* We need to do subscript evaluation, which might require
2880+
* evaluating general expressions; and the caller might have
2881+
* done that too in order to prepare the input Datum. We
2882+
* have to save and restore the caller's SPI_execute result,
2883+
* if any.
2884+
*/
2885+
save_eval_tuptable = estate->eval_tuptable;
2886+
estate->eval_tuptable = NULL;
2887+
2888+
/*
28712889
* To handle constructs like x[1][2] := something, we have to be
28722890
* prepared to deal with a chain of arrayelem datums. Chase
28732891
* back to find the base array datum, and save the subscript
@@ -2910,8 +2928,23 @@ exec_assign_value(PLpgSQL_execstate * estate,
29102928
subscripts[nsubscripts - 1 - i],
29112929
&subisnull);
29122930
havenullsubscript |= subisnull;
2931+
2932+
/*
2933+
* Clean up in case the subscript expression wasn't simple.
2934+
* We can't do exec_eval_cleanup, but we can do this much
2935+
* (which is safe because the integer subscript value is
2936+
* surely pass-by-value), and we must do it in case the
2937+
* next subscript expression isn't simple either.
2938+
*/
2939+
if (estate->eval_tuptable != NULL)
2940+
SPI_freetuptable(estate->eval_tuptable);
2941+
estate->eval_tuptable = NULL;
29132942
}
29142943

2944+
/* Now we can restore caller's SPI_execute result if any. */
2945+
Assert(estate->eval_tuptable == NULL);
2946+
estate->eval_tuptable = save_eval_tuptable;
2947+
29152948
/*
29162949
* Skip the assignment if we have any nulls, either in the
29172950
* original array value, the subscripts, or the righthand

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1788,3 +1788,26 @@ SELECT * FROM perform_test;
17881788
(3 rows)
17891789

17901790
drop table perform_test;
1791+
-- Test for appropriate cleanup of non-simple expression evaluations
1792+
-- (bug in all versions prior to August 2010)
1793+
CREATE FUNCTION nonsimple_expr_test() RETURNS text[] AS '
1794+
DECLARE
1795+
arr text[];
1796+
lr text;
1797+
i integer;
1798+
BEGIN
1799+
arr := array[array[''foo'',''bar''], array[''baz'', ''quux'']];
1800+
lr := ''fool'';
1801+
i := 1;
1802+
-- use sub-SELECTs to make expressions non-simple
1803+
arr[(SELECT i)][(SELECT i+1)] := (SELECT lr);
1804+
RETURN arr;
1805+
END;
1806+
' LANGUAGE plpgsql;
1807+
SELECT nonsimple_expr_test();
1808+
nonsimple_expr_test
1809+
-------------------------
1810+
{{foo,fool},{baz,quux}}
1811+
(1 row)
1812+
1813+
DROP FUNCTION nonsimple_expr_test();

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1603,4 +1603,26 @@ END;' language 'plpgsql';
16031603
SELECT perform_test_func();
16041604
SELECT * FROM perform_test;
16051605

1606-
drop table perform_test;
1606+
drop table perform_test;
1607+
1608+
-- Test for appropriate cleanup of non-simple expression evaluations
1609+
-- (bug in all versions prior to August 2010)
1610+
1611+
CREATE FUNCTION nonsimple_expr_test() RETURNS text[] AS '
1612+
DECLARE
1613+
arr text[];
1614+
lr text;
1615+
i integer;
1616+
BEGIN
1617+
arr := array[array[''foo'',''bar''], array[''baz'', ''quux'']];
1618+
lr := ''fool'';
1619+
i := 1;
1620+
-- use sub-SELECTs to make expressions non-simple
1621+
arr[(SELECT i)][(SELECT i+1)] := (SELECT lr);
1622+
RETURN arr;
1623+
END;
1624+
' LANGUAGE plpgsql;
1625+
1626+
SELECT nonsimple_expr_test();
1627+
1628+
DROP FUNCTION nonsimple_expr_test();

0 commit comments

Comments
 (0)
0