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
