On 23/08/2013 10:36, I wrote:
On 8/23/13 8:38 AM, Pavel Stehule wrote:
do you prepare patch ?

I should have the time to produce one for the September commitfest, but
if you (or anyone else) want to work on this, I won't object.

My opinion at this very moment is that we should leave the the DEFAULT
verbosity alone and add a new one (call it COMPACT or such) with the
suppressed context for non-ERRORs.

Well, turns out there isn't really any way to preserve complete backwards compatibility if we want to do this change.

The attached patch (based on Pavel's patch) changes the default to be slightly more verbose (the CONTEXT lines which were previously omitted will be visible), but adds a new PGVerbosity called COMPACT which suppresses CONTEXT in non-error messages. Now DEFAULT will be more useful when debugging PL/PgSQL, and people who are annoyed by the new behaviour can use the COMPACT mode.

Any thoughts?



Regards,
Marko Tiikkaja

*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
***************
*** 5418,5423 **** int PQsetClientEncoding(PGconn 
*<replaceable>conn</replaceable>, const char *<re
--- 5418,5424 ----
  typedef enum
  {
      PQERRORS_TERSE,
+     PQERRORS_COMPACT,
      PQERRORS_DEFAULT,
      PQERRORS_VERBOSE
  } PGVerbosity;
***************
*** 5430,5439 **** PGVerbosity PQsetErrorVerbosity(PGconn *conn, PGVerbosity 
verbosity);
        returned messages include severity, primary text, and position only;
        this will normally fit on a single line.  The default mode produces
        messages that include the above plus any detail, hint, or context
!       fields (these might span multiple lines).  The <firstterm>VERBOSE</>
!       mode includes all available fields.  Changing the verbosity does not
!       affect the messages available from already-existing
!       <structname>PGresult</> objects, only subsequently-created ones.
       </para>
      </listitem>
     </varlistentry>
--- 5431,5442 ----
        returned messages include severity, primary text, and position only;
        this will normally fit on a single line.  The default mode produces
        messages that include the above plus any detail, hint, or context
!       fields (these might span multiple lines).  The COMPACT mode is otherwise
!       the same as the default, except the context field will be omitted for
!       non-error messages.  The <firstterm>VERBOSE</> mode includes all
!       available fields.  Changing the verbosity does not affect the messages
!       available from already-existing <structname>PGresult</> objects, only
!       subsequently-created ones.
       </para>
      </listitem>
     </varlistentry>
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
***************
*** 796,801 **** verbosity_hook(const char *newval)
--- 796,803 ----
                pset.verbosity = PQERRORS_DEFAULT;
        else if (strcmp(newval, "terse") == 0)
                pset.verbosity = PQERRORS_TERSE;
+       else if (strcmp(newval, "compact") == 0)
+               pset.verbosity = PQERRORS_COMPACT;
        else if (strcmp(newval, "verbose") == 0)
                pset.verbosity = PQERRORS_VERBOSE;
        else
*** a/src/interfaces/libpq/fe-protocol3.c
--- b/src/interfaces/libpq/fe-protocol3.c
***************
*** 915,920 **** pqGetErrorNotice3(PGconn *conn, bool isError)
--- 915,924 ----
                if (val)
                        appendPQExpBuffer(&workBuf, libpq_gettext("QUERY:  
%s\n"), val);
                val = PQresultErrorField(res, PG_DIAG_CONTEXT);
+       }
+       if (isError || (conn->verbosity != PQERRORS_TERSE &&
+                                       conn->verbosity != PQERRORS_COMPACT))
+       {
                if (val)
                        appendPQExpBuffer(&workBuf, libpq_gettext("CONTEXT:  
%s\n"), val);
        }
*** a/src/interfaces/libpq/libpq-fe.h
--- b/src/interfaces/libpq/libpq-fe.h
***************
*** 106,111 **** typedef enum
--- 106,112 ----
  typedef enum
  {
        PQERRORS_TERSE,                         /* single-line error messages */
+       PQERRORS_COMPACT,                       /* single-line error messages 
on non-error messags */
        PQERRORS_DEFAULT,                       /* recommended style */
        PQERRORS_VERBOSE                        /* all the facts, ma'am */
  } PGVerbosity;
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 39,46 ****
  #include "utils/typcache.h"
  
  
- static const char *const raise_skip_msg = "RAISE";
- 
  typedef struct
  {
        int                     nargs;                  /* number of arguments 
*/
--- 39,44 ----
***************
*** 867,876 **** plpgsql_exec_error_callback(void *arg)
  {
        PLpgSQL_execstate *estate = (PLpgSQL_execstate *) arg;
  
-       /* if we are doing RAISE, don't report its location */
-       if (estate->err_text == raise_skip_msg)
-               return;
- 
        if (estate->err_text != NULL)
        {
                /*
--- 865,870 ----
***************
*** 3032,3038 **** exec_stmt_raise(PLpgSQL_execstate *estate, 
PLpgSQL_stmt_raise *stmt)
        /*
         * Throw the error (may or may not come back)
         */
-       estate->err_text = raise_skip_msg;      /* suppress traceback of raise 
*/
  
        ereport(stmt->elog_level,
                        (err_code ? errcode(err_code) : 0,
--- 3026,3031 ----
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to