From e184663b242f5b9d6088d55d0bd72aeab1713cdf Mon Sep 17 00:00:00 2001 From: Tom Lane 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 f8563b3e0b..5366cd1acf 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 8674fa9531..1f11bbb75c 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 f9f307b1ac..48618b9508 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); -- 2.40.0