Make some small improvements in the accuracy of plpgsql's error location

reports; inspired by the misleading CONTEXT lines shown in recent bug report
from Stefan Kaltenbrunner.  Also, allow statement-type names shown in these
messages to be translated.

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index cd94b77c7de6368bc2012b14dd2d49a311cb17e4..67bcbf0cc408398607f5e3856a47879cbe66baf1 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.184 2007/01/28 16:15:49 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.185 2007/01/28 17:58:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -267,6 +267,8 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
                }
        }
 
+       estate.err_text = gettext_noop("during function entry");
+
        /*
         * Set the magic variable FOUND to false
         */
@@ -414,6 +416,8 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
                }
        }
 
+       estate.err_text = gettext_noop("during function exit");
+
        /*
         * Let the instrumentation plugin peek at this function
         */
@@ -608,6 +612,8 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
                                                   CStringGetDatum(trigdata->tg_trigger->tgargs[i]));
        }
 
+       estate.err_text = gettext_noop("during function entry");
+
        /*
         * Set the magic variable FOUND to false
         */
@@ -644,6 +650,9 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
                                errmsg("control reached end of trigger procedure without RETURN")));
        }
 
+       estate.err_stmt = NULL;
+       estate.err_text = gettext_noop("during function exit");
+
        if (estate.retisset)
                ereport(ERROR,
                                (errcode(ERRCODE_DATATYPE_MISMATCH),
@@ -711,30 +720,48 @@ plpgsql_exec_error_callback(void *arg)
        if (estate->err_text == raise_skip_msg)
                return;
 
-       if (estate->err_stmt != NULL)
-       {
-               /* translator: last %s is a plpgsql statement type name */
-               errcontext("PL/pgSQL function \"%s\" line %d at %s",
-                                  estate->err_func->fn_name,
-                                  estate->err_stmt->lineno,
-                                  plpgsql_stmt_typename(estate->err_stmt));
-       }
-       else if (estate->err_text != NULL)
+       if (estate->err_text != NULL)
        {
                /*
                 * We don't expend the cycles to run gettext() on err_text unless we
                 * actually need it.  Therefore, places that set up err_text should
                 * use gettext_noop() to ensure the strings get recorded in the
                 * message dictionary.
+                *
+                * If both err_text and err_stmt are set, use the err_text as
+                * description, but report the err_stmt's line number.  When
+                * err_stmt is not set, we're in function entry/exit, or some such
+                * place not attached to a specific line number.
                 */
-
-               /*
-                * translator: last %s is a phrase such as "while storing call
-                * arguments into local variables"
-                */
-               errcontext("PL/pgSQL function \"%s\" %s",
+               if (estate->err_stmt != NULL)
+               {
+                       /*
+                        * translator: last %s is a phrase such as "during statement
+                        * block local variable initialization"
+                        */
+                       errcontext("PL/pgSQL function \"%s\" line %d %s",
+                                          estate->err_func->fn_name,
+                                          estate->err_stmt->lineno,
+                                          gettext(estate->err_text));
+               }
+               else
+               {
+                       /*
+                        * translator: last %s is a phrase such as "while storing call
+                        * arguments into local variables"
+                        */
+                       errcontext("PL/pgSQL function \"%s\" %s",
+                                          estate->err_func->fn_name,
+                                          gettext(estate->err_text));
+               }
+       }
+       else if (estate->err_stmt != NULL)
+       {
+               /* translator: last %s is a plpgsql statement type name */
+               errcontext("PL/pgSQL function \"%s\" line %d at %s",
                                   estate->err_func->fn_name,
-                                  gettext(estate->err_text));
+                                  estate->err_stmt->lineno,
+                                  plpgsql_stmt_typename(estate->err_stmt));
        }
        else
                errcontext("PL/pgSQL function \"%s\"",
@@ -846,6 +873,8 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
        /*
         * First initialize all variables declared in this block
         */
+       estate->err_text = gettext_noop("during statement block local variable initialization");
+
        for (i = 0; i < block->n_initvars; i++)
        {
                n = block->initvarnos[i];
@@ -915,6 +944,8 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
                EState     *old_eval_estate = estate->eval_estate;
                long int        old_eval_estate_simple_id = estate->eval_estate_simple_id;
 
+               estate->err_text = gettext_noop("during statement block entry");
+
                BeginInternalSubTransaction(NULL);
                /* Want to run statements inside function's memory context */
                MemoryContextSwitchTo(oldcontext);
@@ -929,9 +960,13 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
                         */
                        plpgsql_create_econtext(estate);
 
+                       estate->err_text = NULL;
+
                        /* Run the block's statements */
                        rc = exec_stmts(estate, block->body);
 
+                       estate->err_text = gettext_noop("during statement block exit");
+
                        /* Commit the inner transaction, return to outer xact context */
                        ReleaseCurrentSubTransaction();
                        MemoryContextSwitchTo(oldcontext);
@@ -953,6 +988,8 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
                        ErrorData  *edata;
                        ListCell   *e;
 
+                       estate->err_text = gettext_noop("during exception cleanup");
+
                        /* Save error info */
                        MemoryContextSwitchTo(oldcontext);
                        edata = CopyErrorData();
@@ -1004,6 +1041,8 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
                                        errm_var->freeval = true;
                                        errm_var->isnull = false;
 
+                                       estate->err_text = NULL;
+
                                        rc = exec_stmts(estate, exception->action);
 
                                        free_var(state_var);
@@ -1025,9 +1064,13 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
                /*
                 * Just execute the statements in the block's body
                 */
+               estate->err_text = NULL;
+
                rc = exec_stmts(estate, block->body);
        }
 
+       estate->err_text = NULL;
+
        /*
         * Handle the return code.
         */

diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index 79c9cb6cb033cccf3863e4678e75212a4d62c82e..398da8fae1677f38ab91eee29a518600a9553ca5 100644 (file)
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_funcs.c,v 1.56 2007/01/05 22:20:02 momjian Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_funcs.c,v 1.57 2007/01/28 17:58:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -426,43 +426,43 @@ plpgsql_stmt_typename(PLpgSQL_stmt *stmt)
        switch (stmt->cmd_type)
        {
                case PLPGSQL_STMT_BLOCK:
-                       return "block variables initialization";
+                       return _("statement block");
                case PLPGSQL_STMT_ASSIGN:
-                       return "assignment";
+                       return _("assignment");
                case PLPGSQL_STMT_IF:
-                       return "if";
+                       return _("if");
                case PLPGSQL_STMT_LOOP:
-                       return "loop";
+                       return _("loop");
                case PLPGSQL_STMT_WHILE:
-                       return "while";
+                       return _("while");
                case PLPGSQL_STMT_FORI:
-                       return "for with integer loopvar";
+                       return _("for with integer loop variable");
                case PLPGSQL_STMT_FORS:
-                       return "for over select rows";
+                       return _("for over select rows");
                case PLPGSQL_STMT_EXIT:
-                       return "exit";
+                       return _("exit");
                case PLPGSQL_STMT_RETURN:
-                       return "return";
+                       return _("return");
                case PLPGSQL_STMT_RETURN_NEXT:
-                       return "return next";
+                       return _("return next");
                case PLPGSQL_STMT_RAISE:
-                       return "raise";
+                       return _("raise");
                case PLPGSQL_STMT_EXECSQL:
-                       return "SQL statement";
+                       return _("SQL statement");
                case PLPGSQL_STMT_DYNEXECUTE:
-                       return "execute statement";
+                       return _("execute statement");
                case PLPGSQL_STMT_DYNFORS:
-                       return "for over execute statement";
+                       return _("for over execute statement");
                case PLPGSQL_STMT_GETDIAG:
-                       return "get diagnostics";
+                       return _("get diagnostics");
                case PLPGSQL_STMT_OPEN:
-                       return "open";
+                       return _("open");
                case PLPGSQL_STMT_FETCH:
-                       return "fetch";
+                       return _("fetch");
                case PLPGSQL_STMT_CLOSE:
-                       return "close";
+                       return _("close");
                case PLPGSQL_STMT_PERFORM:
-                       return "perform";
+                       return _("perform");
        }
 
        return "unknown";