Re: Fix disabled triggers with deferred constraints

On Tue, 13 Aug 2002, Tom Lane wrote:

> Gavin Sherry <swm@linuxworld.com.au> writes:
> > ...The spec is a large one and I didn't look at all references to triggers --
> > since there are hundreds -- but I don't believe that there is any
> > precedent for an implementation of DISABLE TRIGGER.
>
> Thanks for the dig.  I was hoping we could get some guidance from the
> spec, but it looks like not.  How about other implementations --- does
> Oracle support disabled triggers?  DB2?  etc?

Oracle 8 (and I presume 9) allows you to disable/enable triggers through
alter table and alter trigger. My 8.1.7 documentation is silent on the
cases you mention below and I do not have an oracle installation handy to
test. Anyone?

>
> > FWIW, i think that in the case of deferred triggers they should all be
> > added to the queue and whether they are executed or not should be
> > evaluated inside DeferredTriggerExecute() with:
> >     if(LocTriggerData.tg_trigger->tgenabled == false)
> >         return;
>
> So check the state at execution, not when the triggering event occurs.
> I don't have any strong reason to object to that, but I have a gut
> feeling that it still needs to be thought about...
>
> Let's see, I guess there are several possible changes of state for a
> deferred trigger between the triggering event and the end of
> transaction:
>
> * Trigger deleted.  Surely the trigger shouldn't be executed, but should
> we raise an error or just silently ignore it?  (I suspect right now we
> crash :-()
>
> * Trigger created.  In some ideal world we might think that such a
> trigger should be fired, but in reality that ain't gonna happen; we're
> not going to record every possible event on the speculation that some
> trigger for it might be created later in the transaction.

It doesn't need to be an ideal world. We're only talking about deferred
triggers after all. Why couldn't CreateTrgger() just have a look through
deftrig_events, check for its relid and if its in there, call
deferredTriggerAddEvent().

> * Trigger disabled.  Your proposal is to not fire it.  Okay, comports
> with the deleted case, if we make that behavior be silently-ignore.
>
> * Trigger enabled.  Your proposal is to fire it.  Seems not to comport
> with the creation case --- does that bother anyone?
>
> * Trigger changed from not-deferred to deferred.  If we already fired it
> for the event, we surely shouldn't fire it again.  I believe the code
> gets this case right.

Agreed.

> * Trigger changed from deferred to not-deferred.  As Neil was pointing
> out recently, this really should cause the trigger to be fired for the
> pending event immediately, but we don't get that right at the moment.
> (I suppose a stricter interpretation would be to raise an error because
> we can't do anything that really comports with the intended behavior
> of either case.)

I think this should generate an error as it doesn't sit well with the
spec IMHO.

Gavin