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

Reply via email to