projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 10c0565)
Fix incorrect logic in plpgsql for cleanup after evaluation of non-simple
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 9 Aug 2010 18:51:12 +0000 (18:51 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 9 Aug 2010 18:51:12 +0000 (18:51 +0000)
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

src/pl/plpgsql/src/pl_exec.c patch | blob | blame | history
src/test/regress/expected/plpgsql.out patch | blob | blame | history
src/test/regress/sql/plpgsql.sql patch | blob | blame | history

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index c58fba0b568f516c8874a36a789e21594c2a9839..0e71a7cf9583d4a44a458916366322c25c9c619d 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3,7 +3,7 @@
  *           procedural language
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.4 2010/07/05 09:27:57 heikki Exp $
+ *   $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.5 2010/08/09 18:51:12 tgl Exp $
  *
  *   This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -1919,6 +1919,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
@@ -2674,6 +2677,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
@@ -2708,6 +2715,7 @@ exec_assign_value(PLpgSQL_execstate * estate,
    Datum       oldarrayval,
                coerced_value;
    ArrayType  *newarrayval;
+   SPITupleTable *save_eval_tuptable;
    HeapTuple   newtup;
 
    switch (target->dtype)
@@ -2868,6 +2876,16 @@ exec_assign_value(PLpgSQL_execstate * estate,
            /*
             * Target is an element of an array
             *
+            * 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;
+
+           /*
             * 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
@@ -2910,8 +2928,23 @@ exec_assign_value(PLpgSQL_execstate * estate,
                                      subscripts[nsubscripts - 1 - i],
                                      &subisnull);
                havenullsubscript |= subisnull;
+
+               /*
+                * 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;
+
            /*
             * Skip the assignment if we have any nulls, either in the
             * original array value, the subscripts, or the righthand
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index a7a380b5c6c70b247005ee675401f5740efcfada..16b2ae8305df542488a4b4580ffcbeb9e2c4e012 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -1788,3 +1788,26 @@ SELECT * FROM perform_test;
 (3 rows)
 
 drop table perform_test;
+-- 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();
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index b4d0186458dc56ac7438b9916712e0399f31f439..d603efed935818b59289e9c2fe584b500d891bcc 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -1603,4 +1603,26 @@ END;' language 'plpgsql';
 SELECT perform_test_func();
 SELECT * FROM perform_test;
 
-drop table perform_test;
\ No newline at end of file
+drop table perform_test;
+
+-- 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();
This is the main PostgreSQL git repository.