Re: PL/PgSQL: EXIT USING ROLLBACK

Hello,

Today I'd like to present a way to get rid of code like this:

$$
BEGIN

BEGIN
INSERT INTO foo VALUES (1);
-- run some tests/checks/whatever
RAISE EXCEPTION 'OK';
EXCEPTION WHEN raise_exception THEN
IF SQLERRM <> 'OK' THEN
RAISE;
END IF;
END;

RETURN 'success';
END
$$

And replace it with code like this:

$$
BEGIN

<<testsomething>>
BEGIN
INSERT INTO foo VALUES (1);
-- run some tests/checks/whatever
EXIT USING ROLLBACK testsomething;
EXCEPTION WHEN others THEN
RAISE;
END;

RETURN 'success';
END
$$

I'm not set on the USING ROLLBACK syntax; it was the only thing I could
come up with that seemed even remotely sane and didn't break backwards
compatibility.

Thoughts? Patch attached, if someone cares.

.marko

Marko Tiikkaja <marko(at)joh(dot)to> writes:
> Hello,
> Today I'd like to present a way to get rid of code like this:

> $$
> BEGIN

> BEGIN
> INSERT INTO foo VALUES (1);
> -- run some tests/checks/whatever
> RAISE EXCEPTION 'OK';
> EXCEPTION WHEN raise_exception THEN
> IF SQLERRM <> 'OK' THEN
> RAISE;
> END IF;
> END;

> RETURN 'success';
> END
> $$

> And replace it with code like this:

> $$
> BEGIN

> <<testsomething>>
> BEGIN
> INSERT INTO foo VALUES (1);
> -- run some tests/checks/whatever
> EXIT USING ROLLBACK testsomething;
> EXCEPTION WHEN others THEN
> RAISE;
> END;

> RETURN 'success';
> END
> $$

Somehow I'm failing to see that as much of an improvement;
in fact, it's probably less clear than before. I don't much
care for the idea that EXIT should take on some transaction-control
properties instead of being a simple transfer of control.
In particular, what happens if someone attaches USING ROLLBACK
to an EXIT that does not lead from inside to outside a BEGIN/EXCEPTION
block?

regards, tom lane

On 7/26/14, 8:22 PM, Tom Lane wrote:
> In particular, what happens if someone attaches USING ROLLBACK
> to an EXIT that does not lead from inside to outside a BEGIN/EXCEPTION
> block?

I'm not sure which case you're envisioning. A label is required, and
the label must be that of a BEGIN block with an EXCEPTION block if USING
ROLLBACK is specified. If that doesn't answer your question, could try
and explain (perhaps in the form of an example) which problem you're seeing?

.marko

Marko Tiikkaja <marko(at)joh(dot)to> writes:
> On 7/26/14, 8:22 PM, Tom Lane wrote:
>> In particular, what happens if someone attaches USING ROLLBACK
>> to an EXIT that does not lead from inside to outside a BEGIN/EXCEPTION
>> block?

> I'm not sure which case you're envisioning. A label is required, and
> the label must be that of a BEGIN block with an EXCEPTION block if USING
> ROLLBACK is specified. If that doesn't answer your question, could try
> and explain (perhaps in the form of an example) which problem you're seeing?

Well, restrictions of that sort might dodge the implementation problem,
but they make the construct even less orthogonal. (And the restriction as
stated isn't good enough anyway, since I could still place such an EXIT in
the EXCEPTION part of the block.)

Basically my point is that this just seems like inventing another way to
do what one can already do with RAISE, and it doesn't have much redeeming
social value to justify the cognitive load of inventing another construct.

regards, tom lane

On 7/26/14, 8:39 PM, Tom Lane wrote:
> Marko Tiikkaja <marko(at)joh(dot)to> writes:
>> I'm not sure which case you're envisioning. A label is required, and
>> the label must be that of a BEGIN block with an EXCEPTION block if USING
>> ROLLBACK is specified. If that doesn't answer your question, could try
>> and explain (perhaps in the form of an example) which problem you're seeing?
>
> Well, restrictions of that sort might dodge the implementation problem,
> but they make the construct even less orthogonal. (And the restriction as
> stated isn't good enough anyway, since I could still place such an EXIT in
> the EXCEPTION part of the block.)

That's a good point; the patch would have to be changed to disallow this
case.

> Basically my point is that this just seems like inventing another way to
> do what one can already do with RAISE, and it doesn't have much redeeming
> social value to justify the cognitive load of inventing another construct.

Yes, you can already do this with RAISE but that seems more like an
accident than anything else. I feel a dedicated syntax is less error
prone and makes the intent clearer to people reading the code. But I
realize I might be in the minority with this.

.marko

Hello

2014-07-26 19:14 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> Hello,
>
> Today I'd like to present a way to get rid of code like this:
>
> $$
> BEGIN
>
> BEGIN
> INSERT INTO foo VALUES (1);
> -- run some tests/checks/whatever
> RAISE EXCEPTION 'OK';
> EXCEPTION WHEN raise_exception THEN
> IF SQLERRM <> 'OK' THEN
> RAISE;
> END IF;
> END;
>
> RETURN 'success';
> END
> $$
>
> And replace it with code like this:
>
> $$
> BEGIN
>
> <<testsomething>>
> BEGIN
> INSERT INTO foo VALUES (1);
> -- run some tests/checks/whatever
> EXIT USING ROLLBACK testsomething;
> EXCEPTION WHEN others THEN
> RAISE;
> END;
>
> RETURN 'success';
> END
> $$
>
> I'm not set on the USING ROLLBACK syntax; it was the only thing I could
> come up with that seemed even remotely sane and didn't break backwards
> compatibility.
>
> Thoughts? Patch attached, if someone cares.
>

-1

I don't think, so we need to cobolize PL/pgSQL more.

There is not any strong reason why we should to introduce it. You don't
save a code, you don't increase a performance

Regards

Pavel

>
>
> .marko
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

On 26 July 2014 18:14, Marko Tiikkaja <marko(at)joh(dot)to> wrote:

> Today I'd like to present a way to get rid of code like this:

You haven't explained this very well... there is nothing that explains
WHY you want this.

In the absence of a good explanation and a viable benefit, I would
vote -1 for this feature suggestion.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 7/28/14 11:27 AM, Simon Riggs wrote:
> On 26 July 2014 18:14, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
>
>> Today I'd like to present a way to get rid of code like this:
>
> You haven't explained this very well... there is nothing that explains
> WHY you want this.
>
> In the absence of a good explanation and a viable benefit, I would
> vote -1 for this feature suggestion.

Yes, I did a poor job in the original email, but I did explain my
reasoning later:

> Yes, you can already do this with RAISE but that seems more like an
> accident than anything else. I feel a dedicated syntax is less error
> prone and makes the intent clearer to people reading the code. But I
> realize I might be in the minority with this.

I guess -3, +0 is enough that I'll be dropping the patch. Thanks to
everyone who had a look.

.marko

On 28 July 2014 10:34, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
> On 7/28/14 11:27 AM, Simon Riggs wrote:
>>
>> On 26 July 2014 18:14, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
>>
>>> Today I'd like to present a way to get rid of code like this:
>>
>>
>> You haven't explained this very well... there is nothing that explains
>> WHY you want this.
>>
>> In the absence of a good explanation and a viable benefit, I would
>> vote -1 for this feature suggestion.
>
>
> Yes, I did a poor job in the original email, but I did explain my reasoning
> later:

With respect, I think you did a poor job the second time too. I can't
find a clearly explained reasoning behind the proposal, nor do I
understand what the problem was.

One of the things I do is work hard on my initial explanations and
reasoning. This helps me because I frequently end up not proposing
something because my reasoning was poor, but it also helps me focus on
whether I am solving a real problem by sharepening my understanding of
the actual problem. And it also helps Tom (or others) demolish things
more quickly with a well placed "indeed" ;-)

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Sat, Jul 26, 2014 at 8:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Basically my point is that this just seems like inventing another way to
> do what one can already do with RAISE, and it doesn't have much redeeming
> social value to justify the cognitive load of inventing another construct.

The main difference is with RAISE EXCEPTION 'OK'; you cannot know if
it was *your* line of code which throw the 'OK'-exception or if it
came from some other function which was called in the block of code.

This means with the current way you have to inspect all lines of code
in all functions in the entire call graph for the block of code for
which you want to capture the 'OK'-exception (or whatever name one
wishes to use),
alternatively to use a name which is guaranteed to be unique, such as
a UUID or something which no other line of code could possibly emmit
as an exception.

Both approaches are ugly and hackish. I think the language should
provide a documented and safe way of exiting from a begin block
without having to worry about other code raising exceptions of the
same name.

On Mon, Sep 1, 2014 at 5:08 AM, Joel Jacobson <joel(at)trustly(dot)com> wrote:
> On Sat, Jul 26, 2014 at 8:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Basically my point is that this just seems like inventing another way to
>> do what one can already do with RAISE, and it doesn't have much redeeming
>> social value to justify the cognitive load of inventing another construct.
>
> The main difference is with RAISE EXCEPTION 'OK'; you cannot know if
> it was *your* line of code which throw the 'OK'-exception or if it
> came from some other function which was called in the block of code.

The real problem here is that if you're using PL/pgsql exceptions for
control-flow reasons, you are taking a huge performance hit for that
notational convenience. I do agree that the syntax of PL/pgsql is
clunky and maybe we should fix that anyway, but I honestly can't
imagine too many people actually wanting to do this once they realize
what it does to the run time of their procedure (and in some cases,
the XID-consumption rate of their database).

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

> On 3 sep 2014, at 16:20, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> On Mon, Sep 1, 2014 at 5:08 AM, Joel Jacobson <joel(at)trustly(dot)com> wrote:
>>> On Sat, Jul 26, 2014 at 8:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Basically my point is that this just seems like inventing another way to
>>> do what one can already do with RAISE, and it doesn't have much redeeming
>>> social value to justify the cognitive load of inventing another construct.
>>
>> The main difference is with RAISE EXCEPTION 'OK'; you cannot know if
>> it was *your* line of code which throw the 'OK'-exception or if it
>> came from some other function which was called in the block of code.
>
> The real problem here is that if you're using PL/pgsql exceptions for
> control-flow reasons, you are taking a huge performance hit for that
> notational convenience. I do agree that the syntax of PL/pgsql is
> clunky and maybe we should fix that anyway, but I honestly can't
> imagine too many people actually wanting to do this once they realize
> what it does to the run time of their procedure (and in some cases,
> the XID-consumption rate of their database).

Exceptions in plpgsql is indeed an exception itself :-)

There are a few use cases when they are crucial though, I would say I
use this code pattern in 0.1% of all functions, but still, when I need
this, it gets ugly.

Glad to hear you might consider the idea of fixing this.

>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company