Re: Disabling triggers (was Re: pgsql 7.2.3 crash) - Mailing list pgsql-hackers

On Mon, 14 Oct 2002, Tom Lane wrote:

> Gavin Sherry <swm@linuxworld.com.au> writes:
> > On Sat, 12 Oct 2002, Joe Conway wrote:
> >> Tom Lane wrote:
> >>> Hackers: we might reasonably fix this by doing a deep copy of the
> >>> relcache's trigger info during initResultRelInfo(); or we could fix it
> >>> by getting rid of ri_TrigDesc and re-fetching from the relcache every
> >>> time.  The former would imply that trigger state would remain unchanged
> >>> throughout a query, the latter would try to track currently-committed
> >>> trigger behavior.  Either way has got pitfalls I think.
> 
> >>> Any thoughts on which way to go?
> 
> >> I'd say:
> >> 1. go with the former
> 
> > I agree.
> 
> That's my leaning too, after further reflection.  Will make it so.
> 
> >> 2. we definitely should also have an ALTER command to allow
> >> disable/enable of triggers
> 
> > I thought this was worked on for 7.3?
> 
> Unless I missed it, it's not in current sources.

Here is an email I sent to pgsql-patches.

--- BEGIN

---------- Forwarded message ----------
Date: Tue, 13 Aug 2002 15:38:50 +1000 (EST)
From: Gavin Sherry <swm@linuxworld.com.au>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Neil Conway <nconway@klamath.dyndns.org>, pgsql-patches@postgresql.org
Subject: Re: [PATCHES] 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...
>
> > 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.

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

--- END

This is why I thought ALTER TABLE was being worked on.

> 
> I was wondering whether an ALTER TABLE command is really the right way
> to approach this.  If we had an ALTER-type command, presumably the
> implication is that its effects would be global to all backends.  But
> the uses that I've seen for suspending trigger invocations would be
> happier with a local, temporary setting that only affects the current
> backend.  Any thoughts about that?

Oracle supports DISABLE TRIGGER and ALTER TABLE DISABLE ALL TRIGGERS. I
cannot find anything in my version 9 manual about whether the effect is
local or global. As you say, I think the syntax suggests global. (I do not
have an oracle system to test on).

There is no trigger disablement in DB2 7.2, which is the most recent
documentation I have.

Personally, I think we should one up oracle. How about:

DISABLE TRIGGER <name> [LOCALLY|GLOBALLY];
ALTER TABLE DISABLE [ALL] TRIGGERS [LOCALLY|GLOBALLY];

and their respective complements.

This should be able to be implemented through the current invalidation
system. We simply skill registering inval messages which are local.

Gavin