plpgsql's exec_eval_simple_expr() now has to take responsibility for

advancing ActiveSnapshot when we are inside a volatile function.
Per example from Gaetano Mendola.  Add a regression test to catch
similar problems in future.

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index f8563b3..5366cd1 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
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.124 2004/12/11 23:26:51 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.125 2004/12/19 20:20:17 tgl Exp $
  *
  *       This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -3548,9 +3548,10 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
                                          Oid *rettype)
 {
        Datum           retval;
-       ExprContext *econtext;
+       ExprContext * volatile econtext;
        ParamListInfo paramLI;
        int                     i;
+       Snapshot        saveActiveSnapshot;
 
        /*
         * Pass back previously-determined result type.
@@ -3629,13 +3630,39 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
        econtext->ecxt_param_list_info = paramLI;
 
        /*
-        * Now call the executor to evaluate the expression
+        * We have to do some of the things SPI_execute_plan would do,
+        * in particular adjust ActiveSnapshot if we are in a non-read-only
+        * function.  Without this, stable functions within the expression
+        * would fail to see updates made so far by our own function.
         */
        SPI_push();
-       retval = ExecEvalExprSwitchContext(expr->expr_simple_state,
-                                                                          econtext,
-                                                                          isNull,
-                                                                          NULL);
+       saveActiveSnapshot = ActiveSnapshot;
+
+       PG_TRY();
+       {
+               MemoryContext oldcontext;
+
+               oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
+               if (!estate->readonly_func)
+                       ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
+               /*
+                * Finally we can call the executor to evaluate the expression
+                */
+               retval = ExecEvalExpr(expr->expr_simple_state,
+                                                         econtext,
+                                                         isNull,
+                                                         NULL);
+               MemoryContextSwitchTo(oldcontext);
+       }
+       PG_CATCH();
+       {
+               /* Restore global vars and propagate error */
+               ActiveSnapshot = saveActiveSnapshot;
+               PG_RE_THROW();
+       }
+       PG_END_TRY();
+
+       ActiveSnapshot = saveActiveSnapshot;
        SPI_pop();
 
        /*

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 8674fa9..1f11bbb 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -2032,3 +2032,62 @@ ERROR:  insert or update on table "slave" violates foreign key constraint "slave
 DETAIL:  Key (f1)=(2) is not present in table "master".
 drop function trap_foreign_key(int);
 drop function trap_foreign_key_2();
+--
+-- Test proper snapshot handling in simple expressions
+--
+create temp table users(login text, id serial);
+NOTICE:  CREATE TABLE will create implicit sequence "users_id_seq" for serial column "users.id"
+create function sp_id_user(a_login text) returns int as $$
+declare x int;
+begin
+  select into x id from users where login = a_login;
+  if found then return x; end if;
+  return 0;
+end$$ language plpgsql stable;
+insert into users values('user1');
+select sp_id_user('user1');
+ sp_id_user 
+------------
+          1
+(1 row)
+
+select sp_id_user('userx');
+ sp_id_user 
+------------
+          0
+(1 row)
+
+create function sp_add_user(a_login text) returns int as $$
+declare my_id_user int;
+begin
+  my_id_user = sp_id_user( a_login );
+  IF  my_id_user > 0 THEN
+    RETURN -1;  -- error code for existing user
+  END IF;
+  INSERT INTO users ( login ) VALUES ( a_login );
+  my_id_user = sp_id_user( a_login );
+  IF  my_id_user = 0 THEN
+    RETURN -2;  -- error code for insertion failure
+  END IF;
+  RETURN my_id_user;
+end$$ language plpgsql;
+select sp_add_user('user1');
+ sp_add_user 
+-------------
+          -1
+(1 row)
+
+select sp_add_user('user2');
+ sp_add_user 
+-------------
+           2
+(1 row)
+
+select sp_add_user('user2');
+ sp_add_user 
+-------------
+          -1
+(1 row)
+
+drop function sp_add_user(text);
+drop function sp_id_user(text);

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index f9f307b..48618b9 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -1764,3 +1764,44 @@ commit;                          -- still fails
 
 drop function trap_foreign_key(int);
 drop function trap_foreign_key_2();
+
+--
+-- Test proper snapshot handling in simple expressions
+--
+
+create temp table users(login text, id serial);
+
+create function sp_id_user(a_login text) returns int as $$
+declare x int;
+begin
+  select into x id from users where login = a_login;
+  if found then return x; end if;
+  return 0;
+end$$ language plpgsql stable;
+
+insert into users values('user1');
+
+select sp_id_user('user1');
+select sp_id_user('userx');
+
+create function sp_add_user(a_login text) returns int as $$
+declare my_id_user int;
+begin
+  my_id_user = sp_id_user( a_login );
+  IF  my_id_user > 0 THEN
+    RETURN -1;  -- error code for existing user
+  END IF;
+  INSERT INTO users ( login ) VALUES ( a_login );
+  my_id_user = sp_id_user( a_login );
+  IF  my_id_user = 0 THEN
+    RETURN -2;  -- error code for insertion failure
+  END IF;
+  RETURN my_id_user;
+end$$ language plpgsql;
+
+select sp_add_user('user1');
+select sp_add_user('user2');
+select sp_add_user('user2');
+
+drop function sp_add_user(text);
+drop function sp_id_user(text);