Re: PL/pgSQL return value in after triggers

PL/pgSQL trigger functions currently require a value to be returned,
even though that value is not used for anything in case of a trigger
fired AFTER. I was wondering if we could relax that. It would make
things a bit more robust and produce clearer PL/pgSQL code. The
specific case I'm concerned about is that a trigger function could
accidentally be run in a BEFORE trigger even though it was not meant for
that. It is common practice that trigger functions for AFTER triggers
return NULL, which would have unpleasant effects if used in a BEFORE
trigger.

I think it is very uncommon to have the same function usable for BEFORE
and AFTER triggers, so it would be valuable to have coding support
specifically for AFTER triggers. We could just allow RETURN without
argument, or perhaps no RETURN at all.

Comments?

On Mon, Feb 28, 2011 at 12:07 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> PL/pgSQL trigger functions currently require a value to be returned,
> even though that value is not used for anything in case of a trigger
> fired AFTER.  I was wondering if we could relax that.  It would make
> things a bit more robust and produce clearer PL/pgSQL code.  The
> specific case I'm concerned about is that a trigger function could
> accidentally be run in a BEFORE trigger even though it was not meant for
> that.  It is common practice that trigger functions for AFTER triggers
> return NULL, which would have unpleasant effects if used in a BEFORE
> trigger.
>
> I think it is very uncommon to have the same function usable for BEFORE
> and AFTER triggers, so it would be valuable to have coding support
> specifically for AFTER triggers.  We could just allow RETURN without
> argument, or perhaps no RETURN at all.
>
> Comments?

It has bugged me for years that after triggers need to contain a
useless RETURN statement, but I'm not sure now is the time to go fix
it.

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

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> PL/pgSQL trigger functions currently require a value to be returned,
> even though that value is not used for anything in case of a trigger
> fired AFTER. I was wondering if we could relax that.

I got bit by that just a couple days ago --- I supposed that a trigger
that wasn't returning anything useful shouldn't need an explicit
RETURN. So +1 for doing something about it. However, unless it's a
very small and simple patch, I concur with Robert that it might be
a bit late to consider this for 9.1.

regards, tom lane

On mån, 2011-02-28 at 19:07 +0200, Peter Eisentraut wrote:
> PL/pgSQL trigger functions currently require a value to be returned,
> even though that value is not used for anything in case of a trigger
> fired AFTER. I was wondering if we could relax that. It would make
> things a bit more robust and produce clearer PL/pgSQL code. The
> specific case I'm concerned about is that a trigger function could
> accidentally be run in a BEFORE trigger even though it was not meant for
> that. It is common practice that trigger functions for AFTER triggers
> return NULL, which would have unpleasant effects if used in a BEFORE
> trigger.
>
> I think it is very uncommon to have the same function usable for BEFORE
> and AFTER triggers, so it would be valuable to have coding support
> specifically for AFTER triggers. We could just allow RETURN without
> argument, or perhaps no RETURN at all.

Here is a patch for that.

One thing that I'm concerned about with this is that it treats a plain
RETURN in a BEFORE trigger as RETURN NULL, whereas arguably it should be
an error. I haven't found a good way to handle that yet, but I'll keep
looking.

2012/1/2 Peter Eisentraut <peter_e(at)gmx(dot)net>:
> On mån, 2011-02-28 at 19:07 +0200, Peter Eisentraut wrote:
>> PL/pgSQL trigger functions currently require a value to be returned,
>> even though that value is not used for anything in case of a trigger
>> fired AFTER.  I was wondering if we could relax that.  It would make
>> things a bit more robust and produce clearer PL/pgSQL code.  The
>> specific case I'm concerned about is that a trigger function could
>> accidentally be run in a BEFORE trigger even though it was not meant for
>> that.  It is common practice that trigger functions for AFTER triggers
>> return NULL, which would have unpleasant effects if used in a BEFORE
>> trigger.
>>
>> I think it is very uncommon to have the same function usable for BEFORE
>> and AFTER triggers, so it would be valuable to have coding support
>> specifically for AFTER triggers.  We could just allow RETURN without
>> argument, or perhaps no RETURN at all.
>
> Here is a patch for that.
>

+1

> One thing that I'm concerned about with this is that it treats a plain
> RETURN in a BEFORE trigger as RETURN NULL, whereas arguably it should be
> an error.  I haven't found a good way to handle that yet, but I'll keep
> looking.

-1

the change of behave is significant in this case and is better require
some specific symbol. RETURN NULL is good for me.

Regards

Pavel

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

Hello Peter

I checked code, and I don't think so this is good.

A design of optional NULL is going to inconsistent syntax.

RETURN (OLD, NEW, NULL, /* nothing */) is not consistent

But my main argument is not intuitive behave of BEFORE triggers after
this change.

When somebody write BEFORE trigger function like:

BEGIN
RAISE NOTICE '%', NEW.x;
RETURN;
END;

then don't expect so all rows will be lost.

Preferred default return value for BEFORE INSERT UPDATE trigger should
be NEW, and for DELETE trigger should be OLD - not NULL.

And because we cannot to distinct between BEFORE and AFTER trigger in
parser, I propose don't change current behave. Current behave is not
too friendly - but is consistent with simple rules.

Regards

Pavel

2012/1/2 Peter Eisentraut <peter_e(at)gmx(dot)net>:
> On mån, 2011-02-28 at 19:07 +0200, Peter Eisentraut wrote:
>> PL/pgSQL trigger functions currently require a value to be returned,
>> even though that value is not used for anything in case of a trigger
>> fired AFTER.  I was wondering if we could relax that.  It would make
>> things a bit more robust and produce clearer PL/pgSQL code.  The
>> specific case I'm concerned about is that a trigger function could
>> accidentally be run in a BEFORE trigger even though it was not meant for
>> that.  It is common practice that trigger functions for AFTER triggers
>> return NULL, which would have unpleasant effects if used in a BEFORE
>> trigger.
>>
>> I think it is very uncommon to have the same function usable for BEFORE
>> and AFTER triggers, so it would be valuable to have coding support
>> specifically for AFTER triggers.  We could just allow RETURN without
>> argument, or perhaps no RETURN at all.
>
> Here is a patch for that.
>
> One thing that I'm concerned about with this is that it treats a plain
> RETURN in a BEFORE trigger as RETURN NULL, whereas arguably it should be
> an error.  I haven't found a good way to handle that yet, but I'll keep
> looking.
>
>
>
> --
> 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 Sun, Jan 1, 2012 at 11:37 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> One thing that I'm concerned about with this is that it treats a plain
> RETURN in a BEFORE trigger as RETURN NULL, whereas arguably it should be
> an error.  I haven't found a good way to handle that yet, but I'll keep
> looking.

I would be very much disinclined to change the behavior of BEFORE
triggers in this way, so I agree we need a way around that.

I'm going to mark this patch Returned with Feedback; I think it's 9.3
material at this point.

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