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";