On 3/22/18 11:50, Tomas Vondra wrote: > 1) plpgsql.sgml > > The new paragraph talks about "intervening command" - I've been unsure > what that refers too, until I read the comment for ExecutCallStmt(), > which explains this as CALL -> SELECT -> CALL. Perhaps we should add > that to the sgml docs too.
done > 2) spi.c > > I generally find it confusing when there are negative flags, which are > then negated whenever used. That is pretty the "no_snapshots" case, > because it means "don't manage snapshots" and all references to the flag > look like this: > > if (snapshot != InvalidSnapshot && !plan->no_snapshots) > > Why not to have "positive" flag instead, e.g. "manage_snapshots"? Yeah, I'm not too fond of this either. But there is a bunch of code in spi.c that assumes that it can initialize an _SPI_plan to all zeros to get it into a default state. (See all the memset() calls.) If we flipped this struct field around, we would have to change all those places, and all future such places, leading to potential headaches. > FWIW the comment in_SPI_execute_plan talking about "four distinct > snapshot management behaviors" should mention that with > no_snapshots=true none of those really matters. done > I also wonder why _SPI_make_plan_non_temp/_SPI_save_plan changes palloc > to palloc0. It is just to initialize the no_snapshots flag explicitly? > It seems a bit wasteful to do a memset and then go and initialize all > the fields anyway, although I don't know how sensitive this code is. As mentioned above, this actually makes it a bit more consistent with all the memset() calls. I have updated the new patch to remove the now-redundant initializations. > 3) utility.c > > I find this condition rather confusing: > > (!(context == PROCESS_UTILITY_TOPLEVEL || > context == PROCESS_UTILITY_QUERY_NONATOMIC) || > IsTransactionBlock()) > > I mean, negated || with another || - it took me a while to parse what > that means. I suggest doing this instead: > > #define ProcessUtilityIsAtomic(context) \ > (!(context == PROCESS_UTILITY_TOPLEVEL || > context == PROCESS_UTILITY_QUERY_NONATOMIC)) > > (ProcessUtilityIsAtomic(context) || IsTransactionBlock()) fixed > 4) spi_priv.h > > Shouldn't the comment for _SPI_plan also mention what no_snapshots does? There is a comment for the field. You mean the comment at the top? I think it's OK the way it is. > 5) utility.h > > So now that we have PROCESS_UTILITY_QUERY and > PROCESS_UTILITY_QUERY_NONATOMIC, does that mean PROCESS_UTILITY_QUERY is > always atomic? Yes, that's just the pre-existing behavior. > 6) pl_exec.h > > The exec_prepare_plan in exec_stmt_perform was expanded into a local > copy of the code, but ISTM the new code just removes handling of some > error types when (plan==NULL), and doesn't call SPI_keepplan or > exec_simple_check_plan. Why not to keep using exec_prepare_plan and add > a new parameter to skip those calls? Good idea. Done. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 48e703670579b5a5049a0638a156975178d71ed5 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Sat, 24 Mar 2018 10:05:06 -0400 Subject: [PATCH v3] PL/pgSQL: Nested CALL with transactions So far, a nested CALL or DO in PL/pgSQL would not establish a context where transaction control statements were allowed. This fixes that by handling CALL and DO specially in PL/pgSQL, passing the atomic/nonatomic execution context through and doing the required management around transaction boundaries. --- doc/src/sgml/plpgsql.sgml | 20 +++++- src/backend/executor/spi.c | 34 +++++---- src/backend/tcop/utility.c | 2 +- src/include/executor/spi_priv.h | 1 + src/include/tcop/utility.h | 1 + .../plpgsql/src/expected/plpgsql_transaction.out | 83 +++++++++++++++++----- src/pl/plpgsql/src/pl_exec.c | 69 +++++++++++++----- src/pl/plpgsql/src/pl_funcs.c | 4 +- src/pl/plpgsql/src/pl_gram.y | 18 +++++ src/pl/plpgsql/src/pl_handler.c | 4 +- src/pl/plpgsql/src/pl_scanner.c | 1 + src/pl/plpgsql/src/plpgsql.h | 5 +- src/pl/plpgsql/src/sql/plpgsql_transaction.sql | 52 ++++++++++++-- 13 files changed, 235 insertions(+), 59 deletions(-) diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 7ed926fd51..b63b8496c7 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -3463,9 +3463,9 @@ <title>Looping Through a Cursor's Result</title> <title>Transaction Management</title> <para> - In procedures invoked by the <command>CALL</command> command from the top - level as well as in anonymous code blocks (<command>DO</command> command) - called from the top level, it is possible to end transactions using the + In procedures invoked by the <command>CALL</command> command + as well as in anonymous code blocks (<command>DO</command> command), + it is possible to end transactions using the commands <command>COMMIT</command> and <command>ROLLBACK</command>. A new transaction is started automatically after a transaction is ended using these commands, so there is no separate <command>START @@ -3495,6 +3495,20 @@ <title>Transaction Management</title> </programlisting> </para> + <para> + Transaction control is only possible in <command>CALL</command> or + <command>DO</command> invocations from the top level or nested + <command>CALL</command> or <command>DO</command> invocations without any + other intervening command. For example, if the call stack is + <command>CALL proc1()</command> → <command>CALL proc2()</command> + → <command>CALL proc3()</command>, then the second and third + procedures can perform transaction control actions. But if the call stack + is <command>CALL proc1()</command> → <command>SELECT + func2()</command> → <command>CALL proc3()</command>, then the last + procedure cannot do transaction control, because of the + <command>SELECT</command> in between. + </para> + <para> A transaction cannot be ended inside a loop over a query result, nor inside a block with exception handlers. diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 9fc4431b80..08f6f67a15 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -2041,8 +2041,11 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, * * In the first two cases, we can just push the snap onto the stack once * for the whole plan list. + * + * But if the plan has no_snapshots set to true, then don't manage + * snapshots at all. The caller should then take care of that. */ - if (snapshot != InvalidSnapshot) + if (snapshot != InvalidSnapshot && !plan->no_snapshots) { if (read_only) { @@ -2121,7 +2124,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, * In the default non-read-only case, get a new snapshot, replacing * any that we pushed in a previous cycle. */ - if (snapshot == InvalidSnapshot && !read_only) + if (snapshot == InvalidSnapshot && !read_only && !plan->no_snapshots) { if (pushed_active_snap) PopActiveSnapshot(); @@ -2172,7 +2175,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, * If not read-only mode, advance the command counter before each * command and update the snapshot. */ - if (!read_only) + if (!read_only && !plan->no_snapshots) { CommandCounterIncrement(); UpdateActiveSnapshotCommandId(); @@ -2203,10 +2206,23 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, else { char completionTag[COMPLETION_TAG_BUFSIZE]; + ProcessUtilityContext context; + + /* + * If the SPI context is atomic, or we are asked to manage + * snapshots, then we are in an atomic execution context. + * Conversely, to propagate a nonatomic execution context, the + * caller must be in a nonatomic SPI context and manage + * snapshots itself. + */ + if (_SPI_current->atomic || !plan->no_snapshots) + context = PROCESS_UTILITY_QUERY; + else + context = PROCESS_UTILITY_QUERY_NONATOMIC; ProcessUtility(stmt, plansource->query_string, - PROCESS_UTILITY_QUERY, + context, paramLI, _SPI_current->queryEnv, dest, @@ -2638,11 +2654,8 @@ _SPI_make_plan_non_temp(SPIPlanPtr plan) oldcxt = MemoryContextSwitchTo(plancxt); /* Copy the SPI_plan struct and subsidiary data into the new context */ - newplan = (SPIPlanPtr) palloc(sizeof(_SPI_plan)); + newplan = (SPIPlanPtr) palloc0(sizeof(_SPI_plan)); newplan->magic = _SPI_PLAN_MAGIC; - newplan->saved = false; - newplan->oneshot = false; - newplan->plancache_list = NIL; newplan->plancxt = plancxt; newplan->cursor_options = plan->cursor_options; newplan->nargs = plan->nargs; @@ -2705,11 +2718,8 @@ _SPI_save_plan(SPIPlanPtr plan) oldcxt = MemoryContextSwitchTo(plancxt); /* Copy the SPI plan into its own context */ - newplan = (SPIPlanPtr) palloc(sizeof(_SPI_plan)); + newplan = (SPIPlanPtr) palloc0(sizeof(_SPI_plan)); newplan->magic = _SPI_PLAN_MAGIC; - newplan->saved = false; - newplan->oneshot = false; - newplan->plancache_list = NIL; newplan->plancxt = plancxt; newplan->cursor_options = plan->cursor_options; newplan->nargs = plan->nargs; diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index e144583bd1..d355bef606 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -382,7 +382,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, { Node *parsetree = pstmt->utilityStmt; bool isTopLevel = (context == PROCESS_UTILITY_TOPLEVEL); - bool isAtomicContext = (context != PROCESS_UTILITY_TOPLEVEL || IsTransactionBlock()); + bool isAtomicContext = (!(context == PROCESS_UTILITY_TOPLEVEL || context == PROCESS_UTILITY_QUERY_NONATOMIC) || IsTransactionBlock()); ParseState *pstate; check_xact_readonly(parsetree); diff --git a/src/include/executor/spi_priv.h b/src/include/executor/spi_priv.h index 263c8f1453..376fae0bbc 100644 --- a/src/include/executor/spi_priv.h +++ b/src/include/executor/spi_priv.h @@ -86,6 +86,7 @@ typedef struct _SPI_plan int magic; /* should equal _SPI_PLAN_MAGIC */ bool saved; /* saved or unsaved plan? */ bool oneshot; /* one-shot plan? */ + bool no_snapshots; /* let the caller handle the snapshots */ List *plancache_list; /* one CachedPlanSource per parsetree */ MemoryContext plancxt; /* Context containing _SPI_plan and data */ int cursor_options; /* Cursor options used for planning */ diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h index 5550055710..880d19311a 100644 --- a/src/include/tcop/utility.h +++ b/src/include/tcop/utility.h @@ -20,6 +20,7 @@ typedef enum { PROCESS_UTILITY_TOPLEVEL, /* toplevel interactive command */ PROCESS_UTILITY_QUERY, /* a complete query, but not toplevel */ + PROCESS_UTILITY_QUERY_NONATOMIC, /* a complete query, nonatomic execution context */ PROCESS_UTILITY_SUBCOMMAND /* a portion of a query */ } ProcessUtilityContext; diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out index ce66487137..b601f6aef6 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out +++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out @@ -1,10 +1,10 @@ CREATE TABLE test1 (a int, b text); -CREATE PROCEDURE transaction_test1() +CREATE PROCEDURE transaction_test1(x int, y text) LANGUAGE plpgsql AS $$ BEGIN - FOR i IN 0..9 LOOP - INSERT INTO test1 (a) VALUES (i); + FOR i IN 0..x LOOP + INSERT INTO test1 (a, b) VALUES (i, y); IF i % 2 = 0 THEN COMMIT; ELSE @@ -13,15 +13,15 @@ BEGIN END LOOP; END $$; -CALL transaction_test1(); +CALL transaction_test1(9, 'foo'); SELECT * FROM test1; - a | b ----+--- - 0 | - 2 | - 4 | - 6 | - 8 | + a | b +---+----- + 0 | foo + 2 | foo + 4 | foo + 6 | foo + 8 | foo (5 rows) TRUNCATE test1; @@ -51,9 +51,9 @@ SELECT * FROM test1; -- transaction commands not allowed when called in transaction block START TRANSACTION; -CALL transaction_test1(); +CALL transaction_test1(9, 'error'); ERROR: invalid transaction termination -CONTEXT: PL/pgSQL function transaction_test1() line 6 at COMMIT +CONTEXT: PL/pgSQL function transaction_test1(integer,text) line 6 at COMMIT COMMIT; START TRANSACTION; DO LANGUAGE plpgsql $$ BEGIN COMMIT; END $$; @@ -90,14 +90,14 @@ CREATE FUNCTION transaction_test3() RETURNS int LANGUAGE plpgsql AS $$ BEGIN - CALL transaction_test1(); + CALL transaction_test1(9, 'error'); RETURN 1; END; $$; SELECT transaction_test3(); ERROR: invalid transaction termination -CONTEXT: PL/pgSQL function transaction_test1() line 6 at COMMIT -SQL statement "CALL transaction_test1()" +CONTEXT: PL/pgSQL function transaction_test1(integer,text) line 6 at COMMIT +SQL statement "CALL transaction_test1(9, 'error')" PL/pgSQL function transaction_test3() line 3 at CALL SELECT * FROM test1; a | b @@ -130,6 +130,57 @@ $$; CALL transaction_test5(); ERROR: invalid transaction termination CONTEXT: PL/pgSQL function transaction_test5() line 3 at COMMIT +TRUNCATE test1; +-- nested procedure calls +CREATE PROCEDURE transaction_test6(c text) +LANGUAGE plpgsql +AS $$ +BEGIN + CALL transaction_test1(9, c); +END; +$$; +CALL transaction_test6('bar'); +SELECT * FROM test1; + a | b +---+----- + 0 | bar + 2 | bar + 4 | bar + 6 | bar + 8 | bar +(5 rows) + +TRUNCATE test1; +CREATE PROCEDURE transaction_test7() +LANGUAGE plpgsql +AS $$ +BEGIN + DO 'BEGIN CALL transaction_test1(9, $x$baz$x$); END;'; +END; +$$; +CALL transaction_test7(); +SELECT * FROM test1; + a | b +---+----- + 0 | baz + 2 | baz + 4 | baz + 6 | baz + 8 | baz +(5 rows) + +CREATE PROCEDURE transaction_test8() +LANGUAGE plpgsql +AS $$ +BEGIN + EXECUTE 'CALL transaction_test1(10, $x$baz$x$)'; +END; +$$; +CALL transaction_test8(); +ERROR: invalid transaction termination +CONTEXT: PL/pgSQL function transaction_test1(integer,text) line 6 at COMMIT +SQL statement "CALL transaction_test1(10, $x$baz$x$)" +PL/pgSQL function transaction_test8() line 3 at EXECUTE -- commit inside cursor loop CREATE TABLE test2 (x int); INSERT INTO test2 VALUES (0), (1), (2), (3), (4); diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 38ea7a091f..fc0f0f0480 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -22,6 +22,7 @@ #include "access/tupconvert.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" +#include "commands/defrem.h" #include "executor/execExpr.h" #include "executor/spi.h" #include "executor/spi_priv.h" @@ -33,6 +34,7 @@ #include "parser/scansup.h" #include "storage/proc.h" #include "tcop/tcopprot.h" +#include "tcop/utility.h" #include "utils/array.h" #include "utils/builtins.h" #include "utils/datum.h" @@ -311,7 +313,8 @@ static void plpgsql_estate_setup(PLpgSQL_execstate *estate, static void exec_eval_cleanup(PLpgSQL_execstate *estate); static void exec_prepare_plan(PLpgSQL_execstate *estate, - PLpgSQL_expr *expr, int cursorOptions); + PLpgSQL_expr *expr, int cursorOptions, + bool keepplan); static void exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr); static void exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan); static void exec_check_rw_parameter(PLpgSQL_expr *expr, int target_dno); @@ -440,7 +443,7 @@ static char *format_preparedparamsdata(PLpgSQL_execstate *estate, */ Datum plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo, - EState *simple_eval_estate) + EState *simple_eval_estate, bool atomic) { PLpgSQL_execstate estate; ErrorContextCallback plerrcontext; @@ -452,6 +455,7 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo, */ plpgsql_estate_setup(&estate, func, (ReturnSetInfo *) fcinfo->resultinfo, simple_eval_estate); + estate.atomic = atomic; /* * Setup error traceback support for ereport() @@ -2057,20 +2061,48 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) { PLpgSQL_expr *expr = stmt->expr; ParamListInfo paramLI; + LocalTransactionId before_lxid; + LocalTransactionId after_lxid; int rc; if (expr->plan == NULL) - exec_prepare_plan(estate, expr, 0); + { + /* + * Don't save the plan if not in atomic context. Otherwise, + * transaction ends would cause warnings about plan leaks. + */ + exec_prepare_plan(estate, expr, 0, estate->atomic); + + /* + * The procedure call could end transactions, which would upset the + * snapshot management in SPI_execute*, so don't let it do it. + */ + expr->plan->no_snapshots = true; + } paramLI = setup_param_list(estate, expr); + before_lxid = MyProc->lxid; + rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI, estate->readonly_func, 0); + after_lxid = MyProc->lxid; + if (rc < 0) elog(ERROR, "SPI_execute_plan_with_paramlist failed executing query \"%s\": %s", expr->query, SPI_result_code_string(rc)); + /* + * If we are in a new transaction after the call, we need to reset some + * internal state. + */ + if (before_lxid != after_lxid) + { + estate->simple_eval_estate = NULL; + plpgsql_create_econtext(estate); + } + if (SPI_processed == 1) { SPITupleTable *tuptab = SPI_tuptable; @@ -2705,7 +2737,7 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt) Assert(query); if (query->plan == NULL) - exec_prepare_plan(estate, query, curvar->cursor_options); + exec_prepare_plan(estate, query, curvar->cursor_options, true); /* * Set up ParamListInfo for this query @@ -3719,6 +3751,7 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate, estate->retisset = func->fn_retset; estate->readonly_func = func->fn_readonly; + estate->atomic = true; estate->exitlabel = NULL; estate->cur_error = NULL; @@ -3863,7 +3896,8 @@ exec_eval_cleanup(PLpgSQL_execstate *estate) */ static void exec_prepare_plan(PLpgSQL_execstate *estate, - PLpgSQL_expr *expr, int cursorOptions) + PLpgSQL_expr *expr, int cursorOptions, + bool keepplan) { SPIPlanPtr plan; @@ -3899,7 +3933,8 @@ exec_prepare_plan(PLpgSQL_execstate *estate, expr->query, SPI_result_code_string(SPI_result)); } } - SPI_keepplan(plan); + if (keepplan) + SPI_keepplan(plan); expr->plan = plan; /* Check to see if it's a simple expression */ @@ -3938,7 +3973,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate, { ListCell *l; - exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK); + exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK, true); stmt->mod_stmt = false; foreach(l, SPI_plan_get_plan_sources(expr->plan)) { @@ -4396,7 +4431,7 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt) */ query = stmt->query; if (query->plan == NULL) - exec_prepare_plan(estate, query, stmt->cursor_options); + exec_prepare_plan(estate, query, stmt->cursor_options, true); } else if (stmt->dynquery != NULL) { @@ -4467,7 +4502,7 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt) query = curvar->cursor_explicit_expr; if (query->plan == NULL) - exec_prepare_plan(estate, query, curvar->cursor_options); + exec_prepare_plan(estate, query, curvar->cursor_options, true); } /* @@ -4707,7 +4742,7 @@ exec_assign_expr(PLpgSQL_execstate *estate, PLpgSQL_datum *target, */ if (expr->plan == NULL) { - exec_prepare_plan(estate, expr, 0); + exec_prepare_plan(estate, expr, 0, true); if (target->dtype == PLPGSQL_DTYPE_VAR) exec_check_rw_parameter(expr, target->dno); } @@ -5566,7 +5601,7 @@ exec_eval_expr(PLpgSQL_execstate *estate, * If first time through, create a plan for this expression. */ if (expr->plan == NULL) - exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK); + exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK, true); /* * If this is a simple expression, bypass SPI and use the executor @@ -5652,7 +5687,7 @@ exec_run_select(PLpgSQL_execstate *estate, */ if (expr->plan == NULL) exec_prepare_plan(estate, expr, - portalP == NULL ? CURSOR_OPT_PARALLEL_OK : 0); + portalP == NULL ? CURSOR_OPT_PARALLEL_OK : 0, true); /* * Set up ParamListInfo to pass to executor @@ -7834,11 +7869,13 @@ plpgsql_create_econtext(PLpgSQL_execstate *estate) { MemoryContext oldcontext; - Assert(shared_simple_eval_estate == NULL); - oldcontext = MemoryContextSwitchTo(TopTransactionContext); - shared_simple_eval_estate = CreateExecutorState(); + if (shared_simple_eval_estate == NULL) + { + oldcontext = MemoryContextSwitchTo(TopTransactionContext); + shared_simple_eval_estate = CreateExecutorState(); + MemoryContextSwitchTo(oldcontext); + } estate->simple_eval_estate = shared_simple_eval_estate; - MemoryContextSwitchTo(oldcontext); } /* diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c index 39d6a54663..fc96fb5f4d 100644 --- a/src/pl/plpgsql/src/pl_funcs.c +++ b/src/pl/plpgsql/src/pl_funcs.c @@ -285,7 +285,7 @@ plpgsql_stmt_typename(PLpgSQL_stmt *stmt) case PLPGSQL_STMT_PERFORM: return "PERFORM"; case PLPGSQL_STMT_CALL: - return "CALL"; + return ((PLpgSQL_stmt_call *) stmt)->is_call ? "CALL" : "DO"; case PLPGSQL_STMT_COMMIT: return "COMMIT"; case PLPGSQL_STMT_ROLLBACK: @@ -1295,7 +1295,7 @@ static void dump_call(PLpgSQL_stmt_call *stmt) { dump_ind(); - printf("CALL expr = "); + printf("%s expr = ", stmt->is_call ? "CALL" : "DO"); dump_expr(stmt->expr); printf("\n"); } diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y index 4c80936678..b8562ca8b4 100644 --- a/src/pl/plpgsql/src/pl_gram.y +++ b/src/pl/plpgsql/src/pl_gram.y @@ -276,6 +276,7 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt); %token <keyword> K_DEFAULT %token <keyword> K_DETAIL %token <keyword> K_DIAGNOSTICS +%token <keyword> K_DO %token <keyword> K_DUMP %token <keyword> K_ELSE %token <keyword> K_ELSIF @@ -914,8 +915,24 @@ stmt_call : K_CALL new->cmd_type = PLPGSQL_STMT_CALL; new->lineno = plpgsql_location_to_lineno(@1); new->expr = read_sql_stmt("CALL "); + new->is_call = true; $$ = (PLpgSQL_stmt *)new; + + } + | K_DO + { + /* use the same structures as for CALL, for simplicity */ + PLpgSQL_stmt_call *new; + + new = palloc0(sizeof(PLpgSQL_stmt_call)); + new->cmd_type = PLPGSQL_STMT_CALL; + new->lineno = plpgsql_location_to_lineno(@1); + new->expr = read_sql_stmt("DO "); + new->is_call = false; + + $$ = (PLpgSQL_stmt *)new; + } ; @@ -2434,6 +2451,7 @@ unreserved_keyword : | K_DEFAULT | K_DETAIL | K_DIAGNOSTICS + | K_DO | K_DUMP | K_ELSIF | K_ERRCODE diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c index f38ef04077..61452d9f7f 100644 --- a/src/pl/plpgsql/src/pl_handler.c +++ b/src/pl/plpgsql/src/pl_handler.c @@ -260,7 +260,7 @@ plpgsql_call_handler(PG_FUNCTION_ARGS) retval = (Datum) 0; } else - retval = plpgsql_exec_function(func, fcinfo, NULL); + retval = plpgsql_exec_function(func, fcinfo, NULL, !nonatomic); } PG_CATCH(); { @@ -332,7 +332,7 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS) /* And run the function */ PG_TRY(); { - retval = plpgsql_exec_function(func, &fake_fcinfo, simple_eval_estate); + retval = plpgsql_exec_function(func, &fake_fcinfo, simple_eval_estate, codeblock->atomic); } PG_CATCH(); { diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c index 65774f9902..08614a89a8 100644 --- a/src/pl/plpgsql/src/pl_scanner.c +++ b/src/pl/plpgsql/src/pl_scanner.c @@ -119,6 +119,7 @@ static const ScanKeyword unreserved_keywords[] = { PG_KEYWORD("default", K_DEFAULT, UNRESERVED_KEYWORD) PG_KEYWORD("detail", K_DETAIL, UNRESERVED_KEYWORD) PG_KEYWORD("diagnostics", K_DIAGNOSTICS, UNRESERVED_KEYWORD) + PG_KEYWORD("do", K_DO, UNRESERVED_KEYWORD) PG_KEYWORD("dump", K_DUMP, UNRESERVED_KEYWORD) PG_KEYWORD("elseif", K_ELSIF, UNRESERVED_KEYWORD) PG_KEYWORD("elsif", K_ELSIF, UNRESERVED_KEYWORD) diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index f7619a63f9..dc90fe532f 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -517,6 +517,7 @@ typedef struct PLpgSQL_stmt_call PLpgSQL_stmt_type cmd_type; int lineno; PLpgSQL_expr *expr; + bool is_call; PLpgSQL_variable *target; } PLpgSQL_stmt_call; @@ -979,6 +980,7 @@ typedef struct PLpgSQL_execstate bool retisset; bool readonly_func; + bool atomic; char *exitlabel; /* the "target" label of the current EXIT or * CONTINUE stmt, if any */ @@ -1194,7 +1196,8 @@ extern void _PG_init(void); */ extern Datum plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo, - EState *simple_eval_estate); + EState *simple_eval_estate, + bool atomic); extern HeapTuple plpgsql_exec_trigger(PLpgSQL_function *func, TriggerData *trigdata); extern void plpgsql_exec_event_trigger(PLpgSQL_function *func, diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql index 02ee735079..a718f50f89 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql @@ -1,12 +1,12 @@ CREATE TABLE test1 (a int, b text); -CREATE PROCEDURE transaction_test1() +CREATE PROCEDURE transaction_test1(x int, y text) LANGUAGE plpgsql AS $$ BEGIN - FOR i IN 0..9 LOOP - INSERT INTO test1 (a) VALUES (i); + FOR i IN 0..x LOOP + INSERT INTO test1 (a, b) VALUES (i, y); IF i % 2 = 0 THEN COMMIT; ELSE @@ -16,7 +16,7 @@ CREATE PROCEDURE transaction_test1() END $$; -CALL transaction_test1(); +CALL transaction_test1(9, 'foo'); SELECT * FROM test1; @@ -43,7 +43,7 @@ CREATE PROCEDURE transaction_test1() -- transaction commands not allowed when called in transaction block START TRANSACTION; -CALL transaction_test1(); +CALL transaction_test1(9, 'error'); COMMIT; START TRANSACTION; @@ -80,7 +80,7 @@ CREATE FUNCTION transaction_test3() RETURNS int LANGUAGE plpgsql AS $$ BEGIN - CALL transaction_test1(); + CALL transaction_test1(9, 'error'); RETURN 1; END; $$; @@ -116,6 +116,46 @@ CREATE PROCEDURE transaction_test5() CALL transaction_test5(); +TRUNCATE test1; + +-- nested procedure calls +CREATE PROCEDURE transaction_test6(c text) +LANGUAGE plpgsql +AS $$ +BEGIN + CALL transaction_test1(9, c); +END; +$$; + +CALL transaction_test6('bar'); + +SELECT * FROM test1; + +TRUNCATE test1; + +CREATE PROCEDURE transaction_test7() +LANGUAGE plpgsql +AS $$ +BEGIN + DO 'BEGIN CALL transaction_test1(9, $x$baz$x$); END;'; +END; +$$; + +CALL transaction_test7(); + +SELECT * FROM test1; + +CREATE PROCEDURE transaction_test8() +LANGUAGE plpgsql +AS $$ +BEGIN + EXECUTE 'CALL transaction_test1(10, $x$baz$x$)'; +END; +$$; + +CALL transaction_test8(); + + -- commit inside cursor loop CREATE TABLE test2 (x int); INSERT INTO test2 VALUES (0), (1), (2), (3), (4); base-commit: 52f3a9d6a32c0c070a15486c3aecbc4405d2da88 -- 2.16.3