Re: WIP fix proposal for bug #6123

On Aug8, 2011, at 17:35 , Florian Pflug wrote:
> I think it would be helpful if we had a more precise idea about the
> intended use-cases. So far, the only use-case that has been described in
> detail is the one which made Kevin aware of the problem. But if I
> understood Kevin correctly, that fact that they use BEFORE and not AFTER
> triggers it more of an accident than the result of a conscious design
> decision. Though he did also mention that there might actually be advantages
> to using BEFORE instead of AFTER triggers, because that prevents other
> triggers from seeing a non-consistent state.

I pondered this some more, and came up with the following use-case of
same-row modification from a BEFORE trigger which are not easily moved
to an AFTER trigger.

Assume you have a partitioned table T with two partitions. Partition
Tcompleted contains records which are "completed" in some sense, while
Tcommencing contains records which are not. A record's state is indicated
by a boolean column "completed", and CHECK constraints guarantee that
"completed" is indeed false for all records in Tcommencing and true for all
records in Tcompleted. Now, if the partitioning is supposed to be transparent
to applications, a record must automatically be moved from Tcommencing to
Tcompleted if it's "completed" flag is changed from false to true. The
following BEFORE UPDATE trigger on Tcommencing accomplishes that.

IF NEW.completed THEN
DELETE FROM Tcommencing WHERE id = OLD.id;
INSERT INTO Tcompleted VALUES (NEW.*); -- Probably a syntax error,
-- but intent is clear
RETURN NULL;
ELSE
RETURN NEW;
END IF;

Doing this from within an AFTER trigger doesn't work because the CHECK
constraint on Tcommencing would prevent the UPDATE from occurring.

This would also fail if we did as I suggested, and forbade any modifications
to a row for which a BEFORE trigger was in progress. I take this as evidence
that my suggestion was miss-guided and we should in fact only report an error
if the row was modified since the before trigger started *and* the trigger
returns non-NULL.

I still believe we shouldn't distinguish between UPDATES and DELETES, so
a DELETE should fail if the row is updated from within a BEFORE DELETE trigger
which returns non-NULL.

BTW, the case of multiple BEFORE triggers also raises some interesting questions.
Assume there are two BEFORE UPDATE triggers T1 and T2 defined on a table T, and T1
causes a same-row UPDATE and returns non-NULL. Also assume that T1 is coded in
a way that prevents infinite recursion in that case (i.e., the same-row UPDATE
is skipped if is T1 invocation already as a result of a same-row UPDATE). The
sequence of invocations after an update of a row would be.

1) T1 (for the original UPDATE)
2) T1 (for the same-row UPDATE initiated in (1))
3) T2 (for the same-row UPDATE initiated in (1))
4) T2 (for the original UPDATE)

Now, T2 in invoked in (4) for a row which isn't visible anymore. So I'm thinking
that maybe we should check after every trigger invocation whether or not a same-row
UPDATE occurred, and immediately raise an error if it did and the trigger didn't
return NULL, and not delay the check until all triggers have run. It seems unlikely
that delaying the check until all triggers have run adds any significant functionality.
For that to be the case, T2 would have to know whether T1 did a same-row update, and
return NULL, since otherwise we'd raise an error anyway. That beauty of doing the
check after every trigger lies in the fact that it makes the coding rule "Thou
shall return NULL if thou dared to modify thine own row" local (i.e. per-trigger)
instead of global (i.e. per trigger-chain), and thus less likely to be violated by
some strange interactions of triggers serving distinct purposes.

To summarize, here's what I currently believe would be a sensible approach:

After every BEFORE trigger invocation, if the trigger returned non-NULL, check if
latest row version is still the same as when the trigger started. If not, complain.

best regards,
Florian Pflug