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 f8563b3e0bf814834c7a661373660afd528cc2c0..5366cd1acf279e2fc0ab24b0aea97764d950a9fb 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 8674fa9531d4423e31d3d39dd8f0c12da8706650..1f11bbb75c92d843e06f96dbe8afb4b6ba0b08c9 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 f9f307b1ac041b2a6101175c41373c0f76368fa3..48618b95085cb2739f3adc4da9117fabbc76f67e 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);