Re: PL/pgSQL, RAISE and error context

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Marko Tiikkaja <marko(at)joh(dot)to>, Merlin Moncure <mmoncure(at)gmail(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/pgSQL, RAISE and error context
Date: 2015-07-07 13:13:00
Message-ID: 559BD05C.2050307@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 01/26/2015 05:14 PM, Tom Lane wrote:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> 2015-01-26 14:02 GMT+01:00 Marko Tiikkaja <marko(at)joh(dot)to>:
>> I am thinking, so solution
>
>> /* if we are doing RAISE, don't report its location */
>> if (estate->err_text == raise_skip_msg)
>> return;
>
>> is too simple, and this part should be fixed. This change can be done by on
>> plpgsql or libpq side. This is bug, and it should be fixed.
>
> Doing this in libpq is utterly insane. It has not got sufficient context
> to do anything intelligent. The fact that it's not intelligent is exposed
> by the regression test changes that the proposed patch causes, most of
> which do not look like improvements.

I think doing this in libpq (or psql) is the way to go. How can the
server know if the client wants to display context information? We just
have to make sure the client has enough information to make a smart
decision. If the client doesn't have enough information today, then
let's work on that.

Note that Marko's patch didn't change libpq's default printing mode,
which is why you got all the extra CONTEXT lines in the regression tests
that were not there before. Just as if we just removed the suppression
from PL/pgSQL and did nothing else. I think we need to also change the
default behaviour to not print CONTEXT lines for NOTICE-level messages,
getting us closer to the current behaviour again.

If you run the regression tests in the "compact" verbosity, the
regression test output changes look quite sensible to me. See attached.

> Another problem is that past requests to change this behavior have
> generally been to the effect that people wanted *more* context suppressed
> not less, ie they didn't want any CONTEXT lines at all on certain
> messages. So the proposed patch seems to me to be going in exactly the
> wrong direction.

After changing the default to "compact", it prints less CONTEXT lines.

> The design I thought had been agreed on was to add some new option to
> plpgsql's RAISE command which would cause suppression of all CONTEXT lines
> not just the most closely nested one.

I don't understand how you came to that conclusion. In particular, see
http://www.postgresql.org/message-id/6656.1377100039@sss.pgh.pa.us. If
you changed your mind, you forgot to tell why.

- Heikki

Attachment Content-Type Size
regression.diffs text/plain 35.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2015-07-07 13:15:08 Re: FPW compression leaks information
Previous Message Andres Freund 2015-07-07 12:55:51 Re: pg_receivexlog --create-slot-if-not-exists