Rethink plpgsql's way of handling SPI execution during an exception block.

We don't really want to start a new SPI connection, just keep using the old
one; otherwise we have memory management problems as illustrated by
John Kennedy's bug report of today.  This requires a bit of a hack to
ensure the SPI stack state is properly restored, but then again what we
were doing before was a hack too, strictly speaking.  Add a regression
test to cover this case.

2 files changed, 48 insertions, 0 deletions

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index e1b57f9716..8674fa9531 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -1931,6 +1931,36 @@ select * from foo;
  20
 (2 rows)
 
+-- Test for pass-by-ref values being stored in proper context
+create function test_variable_storage() returns text as $$
+declare x text;
+begin
+  x := '1234';
+  begin
+    x := x || '5678';
+    -- force error inside subtransaction SPI context
+    perform trap_zero_divide(-100);
+  exception
+    when others then
+      x := x || '9012';
+  end;
+  return x;
+end$$ language plpgsql;
+select test_variable_storage();
+NOTICE:  should see this
+CONTEXT:  SQL statement "SELECT  trap_zero_divide(-100)"
+PL/pgSQL function "test_variable_storage" line 7 at perform
+NOTICE:  should see this only if -100 <> 0
+CONTEXT:  SQL statement "SELECT  trap_zero_divide(-100)"
+PL/pgSQL function "test_variable_storage" line 7 at perform
+NOTICE:  should see this only if -100 fits in smallint
+CONTEXT:  SQL statement "SELECT  trap_zero_divide(-100)"
+PL/pgSQL function "test_variable_storage" line 7 at perform
+ test_variable_storage 
+-----------------------
+ 123456789012
+(1 row)
+
 --
 -- test foreign key error trapping
 --
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 367a73986e..f9f307b1ac 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -1696,6 +1696,24 @@ reset statement_timeout;
 
 select * from foo;
 
+-- Test for pass-by-ref values being stored in proper context
+create function test_variable_storage() returns text as $$
+declare x text;
+begin
+  x := '1234';
+  begin
+    x := x || '5678';
+    -- force error inside subtransaction SPI context
+    perform trap_zero_divide(-100);
+  exception
+    when others then
+      x := x || '9012';
+  end;
+  return x;
+end$$ language plpgsql;
+
+select test_variable_storage();
+
 --
 -- test foreign key error trapping
 --