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.

3 files changed, 134 insertions, 7 deletions

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index f8563b3e0bf..5366cd1acf2 100644
--- 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 8674fa9531d..1f11bbb75c9 100644
--- 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 f9f307b1ac0..48618b95085 100644
--- 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);