Returning NEW in an on-delete trigger

On Thu, 2008-09-18 at 14:21 -0400, Michael Nolan wrote:

Recently I discovered a coding error of mine in a trigger that is
called only for deletes.

I was returning NEW instead of OLD.

Since NEW is undefined when deleting a row, it was failing and the row
wasn't being deleted.

In PL/pgSQL, NEW is NULL for BEFORE DELETE triggers. It doesn't appear
to be obviously documented that this is the case (which is, I assume,
why you said it is undefined), and I agree that it can cause confusion.

However, it was failing silently. Shouldn't this have recorded an
error or warning somewhere?

This is a feature, not a bug. Sometimes you don't want to delete a
record, and returning NULL is the way to do that.

Regards,
Jeff Davis

"Michael Nolan" <htfoot@gmail.com> writes:

Recently I discovered a coding error of mine in a trigger that is called
only for deletes.
I was returning NEW instead of OLD.
Since NEW is undefined when deleting a row, it was failing and the row
wasn't being deleted.
However, it was failing silently. Shouldn't this have recorded an error or
warning somewhere?

The NEW variable is just set up as being NULL in an ON DELETE trigger,
likewise for OLD in an ON INSERT trigger.

This does seem like a bit of a gotcha for someone who writes RETURN NEW
instead of RETURN OLD or vice versa, but I'm not sure how much we can do
about that. Lots of people like to write triggers that fire on multiple
event types, so we couldn't throw a syntax error for such a reference.
A runtime error for a use of the variable might be possible, but a quick
look at the code doesn't make it look easy.

regards, tom lane

On Thu, 2008-09-18 at 15:04 -0400, Tom Lane wrote:

This does seem like a bit of a gotcha for someone who writes RETURN NEW
instead of RETURN OLD or vice versa, but I'm not sure how much we can do
about that. Lots of people like to write triggers that fire on multiple
event types, so we couldn't throw a syntax error for such a reference.
A runtime error for a use of the variable might be possible, but a quick
look at the code doesn't make it look easy.

Here's a doc patch that may clear up some of the confusion.

Regards,
Jeff Davis

Jeff Davis wrote:

On Thu, 2008-09-18 at 15:04 -0400, Tom Lane wrote:

This does seem like a bit of a gotcha for someone who writes RETURN NEW
instead of RETURN OLD or vice versa, but I'm not sure how much we can do
about that. Lots of people like to write triggers that fire on multiple
event types, so we couldn't throw a syntax error for such a reference.
A runtime error for a use of the variable might be possible, but a quick
look at the code doesn't make it look easy.

Here's a doc patch that may clear up some of the confusion.

Thanks, patch applied.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +