From e184663b242f5b9d6088d55d0bd72aeab1713cdf Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 19 Dec 2004 20:20:27 +0000
Subject: [PATCH] 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.
---
src/pl/plpgsql/src/pl_exec.c | 41 +++++++++++++++----
src/test/regress/expected/plpgsql.out | 59 +++++++++++++++++++++++++++
src/test/regress/sql/plpgsql.sql | 41 +++++++++++++++++++
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);
--
GitLab