Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

On Fri, 1 Jul 2005, Satoshi Nagayasu wrote:

> Hi all,
>
> Here is a first patch to allow these commands.
>
> > ALTER TABLE <table> ENABLE TRIGGER <trigname>
> > ALTER TABLE <table> DISABLE TRIGGER <trigname>

There are three other areas which are worth looking at:

a) We may defer the execution of some triggers to the end of the
transaction. Do we execute those if a they were later disabled?

b) There is a bug in how we execute triggers. For example, in
ExecDelete():

        bool        dodelete;

        dodelete = ExecBRDeleteTriggers(estate, resultRelInfo, tupleid,
                                        estate->es_snapshot->curcid);

        if (!dodelete)          /* "do nothing" */
            return;


This means that if a before trigger returned NULL, we short circuit and do
not delete the tuple. Consider the following in ExecBRDeleteTriggers()

    HeapTuple   newtuple = NULL;

...

   for (i = 0; i < ntrigs; i++)
    {
        Trigger    *trigger = &trigdesc->triggers[tgindx[i]];

        if (!trigger->tgenabled)
            continue;
        LocTriggerData.tg_trigtuple = trigtuple;
        LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
        LocTriggerData.tg_trigger = trigger;
        newtuple = ExecCallTriggerFunc(&LocTriggerData,
                                       tgindx[i],
                                       relinfo->ri_TrigFunctions,
                                       relinfo->ri_TrigInstrument,
                                       GetPerTupleMemoryContext(estate));
        if (newtuple == NULL)
            break;
        if (newtuple != trigtuple)
            heap_freetuple(newtuple);
    }

...

    return (newtuple == NULL) ? false : true;

This means that if all triggers on a table are disabled, we tell the
caller that a trigger returned NULL and that we should short circuit. This
does not seem to be the case for the other DML statements.

c) There has been a push over previous releases to make dumps generated by
pg_dump look like ANSI SQL. Now, ALTER TABLE ... DISABLE trigger is useful
for pg_dump but not compliant. Others have suggested something like:

SET enable_triggers = off

This would turn all triggers off in the current session. It has the added
benefit that it does not affect other sessions. It does introduce some
issues with permissions -- I wouldn't want users turning off data
validation before triggers in my database, but that's me. I'm not
enamoured of the idea but it is worth discussing, I believe.

Also, a final patch will also need documentation and regression tests :-)

Thanks,

Gavin