BUG #6123: DELETE fails if before trigger causes another trigger to UPDATE

The following bug has been logged online:

Bug reference: 6123
Logged by: Kevin Grittner
Email address: Kevin(dot)Grittner(at)wicourts(dot)gov
PostgreSQL version: HEAD and 9.0.4
Operating system: Linux (probably doesn't matter)
Description: DELETE fails if before trigger causes another trigger to
UPDATE
Details:

create table a (aid int not null primary key,
bcnt int not null default 0);
create table b (bid int not null primary key,
aid int not null);

create function a_del_func()
returns trigger language plpgsql as
$$begin delete from b where aid = old.aid;
return old; end;$$;
create trigger a_del_trig before delete On a
for each row execute procedure a_del_func();

create function b_ins_func()
returns trigger language plpgsql as
$$begin update a set bcnt = bcnt + 1
where aid = new.aid; return new; end;$$;
create trigger b_ins_trig after insert on b
for each row execute procedure b_ins_func();

create function b_del_func()
returns trigger language plpgsql as
$$begin update a set bcnt = bcnt - 1
where aid = old.aid; return old; end;$$;
create trigger b_del_trig after delete on b
for each row execute procedure b_del_func();

insert into a values (1, 0);
insert into b values (10, 1);
select * from a; select * from b;
delete from a where aid = 1;
select * from a; select * from b;

I expect the DELETE from a to delete related b record(s) and then succeed.
Instead the UPDATE in b_del_trig causes delete of a to silently fail.

-Kevin

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> [ circular trigger relationships ]

> I expect the DELETE from a to delete related b record(s) and then succeed.
> Instead the UPDATE in b_del_trig causes delete of a to silently fail.

The update causes the already-selected target row version of the DELETE
to be obsoleted, so heap_delete finds it has nothing to do. I'm
disinclined to mess with that logic.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
>> [ circular trigger relationships ]
>
>> I expect the DELETE from a to delete related b record(s) and then
>> succeed. Instead the UPDATE in b_del_trig causes delete of a to
>> silently fail.
>
> The update causes the already-selected target row version of the
> DELETE to be obsoleted, so heap_delete finds it has nothing to do.
> I'm disinclined to mess with that logic.

It's pretty astonishing behavior for application programmers. It's
not unusual for triggers on detail from one table to maintain a
status, count, or sum in a higher level table. When a DELETE from
the higher level table causes deletes at the lower level, the lower
level trigger really doesn't have any way to know that. I think
this should be considered a bug.

At the point where heap_delete finds that the tuple has expired, I
think it should follow the ctid chain as long as xmax is from the
same top-level transaction, and then decide if it has something to
do. Normally, that would be zero iterations, so I doubt the
performance impact is huge.

I'm willing to mess with that logic if there's a chance that it will
be accepted. I may *need* to even if it won't, to allow our
migration to native PostgreSQL triggers to succeed -- we've been
trying to work around it for the last few days, but the places where
it's popping up are becoming increasingly hard to accommodate that
way.

-Kevin

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> The update causes the already-selected target row version of the
>> DELETE to be obsoleted, so heap_delete finds it has nothing to do.
>> I'm disinclined to mess with that logic.

> It's pretty astonishing behavior for application programmers. It's
> not unusual for triggers on detail from one table to maintain a
> status, count, or sum in a higher level table. When a DELETE from
> the higher level table causes deletes at the lower level, the lower
> level trigger really doesn't have any way to know that. I think
> this should be considered a bug.

I think you have no idea how large a can of worms you're opening.

To point out just one problem, if heap_delete restarts itself and tries
to delete some other row version than it started with, should the ON
DELETE triggers be fired again? If not, why not? If they are, what
prevents an infinite loop?

IMO, application code that causes this to happen is impossibly fragile
and needs to be rewritten to not do it.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
>> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> The update causes the already-selected target row version of the
>>> DELETE to be obsoleted, so heap_delete finds it has nothing to
>>> do. I'm disinclined to mess with that logic.
>
>> It's pretty astonishing behavior for application programmers.
>> It's not unusual for triggers on detail from one table to
>> maintain a status, count, or sum in a higher level table. When a
>> DELETE from the higher level table causes deletes at the lower
>> level, the lower level trigger really doesn't have any way to
>> know that. I think this should be considered a bug.
>
> I think you have no idea how large a can of worms you're opening.
>
> To point out just one problem, if heap_delete restarts itself and
> tries to delete some other row version than it started with,
> should the ON DELETE triggers be fired again?

No.

> If not, why not?

It's the same row. The UPDATE trigger fires when it's UPDATEd; but
it isn't being DELETEd a second time. Imposing this level of
awareness of MVCC mechanics on application programmers isn't a good
idea, in my view.

I wouldn't dive into it lightly. As I said, we've been trying to
work around it, but it is neither unreasonable to maintain summary
data in high level tables, nor to cascade deletes, and with hundreds
of tables, most of which have many triggers, it's not trivial to
catch all of these or to figure out how to avoid the issue in each
case.

For those who haven't heard about our environment or have forgotten,
these triggers were written in a vendor-neutral SQL dialect based on
the standard, and we parsed the SQL to generate Java classes which
contained the procedural logic and the less portable behaviors,
calling out to JDBC through a thin DBMS-specific portability layer.
Since we've decided to commit to PostgreSQL we're converting all
triggers to native PostgreSQL, with a mechanical first pass based on
a new tree-walker (for the parser which emitted the Java classes)
which is emitting plpgsql functions and trigger creation statements.

While the database is pretty well normalized, there is some
summarization of data from detail tables to higher levels for
performance, and some cases where a delete at a higher level should
cascade to lower levels, at least in some circumstances. Both
behaviors are implemented in triggers in the old code and have been
working fine in the old framework.

> IMO, application code that causes this to happen is impossibly
> fragile

They've been working fine for over ten years in our old framework,
so "impossibly fragile" seems an overstatement.

> and needs to be rewritten to not do it.

Would you say it is more sane to sprinkle the logic to maintain
redundant summary data across the application instead of putting it
in triggers on the detail, or more sane to move logic to cascade
deletes outside the database? When the triggers were firing from
our middle tier just above the database we didn't have to choose;
you seem to be saying we should, but neither alternative looks
pretty to me.

-Kevin