On 5/25/18 12:16, Tom Lane wrote: > I notice there are still several places in pl_exec.c like this: > > case SPI_ERROR_TRANSACTION: > ereport(ERROR, > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > errmsg("cannot begin/end transactions in PL/pgSQL"), > errhint("Use a BEGIN block with an EXCEPTION clause > instead."))); > break; > > At best, the wording of these error messages is now obsolete. I'm not > sure if we expect them to be reachable at all. If they should be > can't-happen cases, I'd suggest just deleting them and letting control > fall to the generic default: cases in each switch. If they are reachable, > the messages need work.
There are three occurrences: The occurrence in exec_prepare_plan() could never be reached AFAICT, because SPI_prepare_params() does not produce those SPI_prepare_params() error values in the first place. So remove them altogether. The occurrence in exec_stmt_execsql() can be reached by running for example SAVEPOINT, or any other TransactionStmt other than COMMIT and ROLLBACK, which are intercepted by PL/pgSQL. So we still need an error message there. Unfortunately, we don't know which TransactionStmt caused the error, so the error has to be pretty generic. The occurrence in exec_stmt_dynexecute() can be reached using something like EXECUTE 'COMMIT', which is not supported/not implemented. Hence a tweaked error message there as well. Possible patch attached. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 94a5dc7ad0b3a9d1a500a3220f7cbbc9af7c9465 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Thu, 31 May 2018 07:51:02 -0400 Subject: [PATCH] Reword SPI_ERROR_TRANSACTION errors in PL/pgSQL The previous message for SPI_ERROR_TRANSACTION claimed "cannot begin/end transactions in PL/pgSQL", but that is no longer true. Nevertheless, the error can still happen, so reword the messages. The error cases in exec_prepare_plan() could never happen, so remove them. --- src/pl/plpgsql/src/pl_exec.c | 29 ++++------------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index ef013bcdc7..66ecf5eb55 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -3965,27 +3965,8 @@ exec_prepare_plan(PLpgSQL_execstate *estate, (void *) expr, cursorOptions); if (plan == NULL) - { - /* Some SPI errors deserve specific error messages */ - switch (SPI_result) - { - case SPI_ERROR_COPY: - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot COPY to/from client in PL/pgSQL"))); - break; - case SPI_ERROR_TRANSACTION: - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot begin/end transactions in PL/pgSQL"), - errhint("Use a BEGIN block with an EXCEPTION clause instead."))); - break; - default: - elog(ERROR, "SPI_prepare_params failed for \"%s\": %s", - expr->query, SPI_result_code_string(SPI_result)); - break; - } - } + elog(ERROR, "SPI_prepare_params failed for \"%s\": %s", + expr->query, SPI_result_code_string(SPI_result)); if (keepplan) SPI_keepplan(plan); expr->plan = plan; @@ -4129,8 +4110,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate, case SPI_ERROR_TRANSACTION: ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot begin/end transactions in PL/pgSQL"), - errhint("Use a BEGIN block with an EXCEPTION clause instead."))); + errmsg("unsupported transaction command in PL/pgSQL"))); break; default: @@ -4317,8 +4297,7 @@ exec_stmt_dynexecute(PLpgSQL_execstate *estate, case SPI_ERROR_TRANSACTION: ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot begin/end transactions in PL/pgSQL"), - errhint("Use a BEGIN block with an EXCEPTION clause instead."))); + errmsg("EXECUTE of transaction commands is not implemented"))); break; default: -- 2.17.0