Thread: Minor BEFORE DELETE trigger fix
Attached is a minor patch to make BEFORE DELETE triggers honour tgenabled properly. I know that we cannot, currently, use this feature through a DDL command but just in case someone is updating the catalogs to do it and since it is necessary in order to implement disabling of triggers, I thought I'd send it in. Gavin
Attachment
Can I get a context diff please? --------------------------------------------------------------------------- Gavin Sherry wrote: > Attached is a minor patch to make BEFORE DELETE triggers honour tgenabled > properly. > > I know that we cannot, currently, use this feature through a DDL command > but just in case someone is updating the catalogs to do it and since it is > necessary in order to implement disabling of triggers, I thought I'd send > it in. > > Gavin Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Oops. Attached in the usual format this time. Gavin On Fri, 6 Aug 2004, Bruce Momjian wrote: > > Can I get a context diff please? > > --------------------------------------------------------------------------- > > Gavin Sherry wrote: > > Attached is a minor patch to make BEFORE DELETE triggers honour tgenabled > > properly. > > > > I know that we cannot, currently, use this feature through a DDL command > > but just in case someone is updating the catalogs to do it and since it is > > necessary in order to implement disabling of triggers, I thought I'd send > > it in. > > > > Gavin > > Content-Description: > > [ Attachment, skipping... ] > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 3: if posting/reading through Usenet, please send an appropriate > > subscribe-nomail command to majordomo@postgresql.org so that your > > message can get through to the mailing list cleanly > > -- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 359-1001 > + If your life is a hard drive, | 13 Roberts Road > + Christ can be your backup. | Newtown Square, Pennsylvania 19073 > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > > > !DSPAM:41144fb520531574347913! > >
Attachment
On Sat, 7 Aug 2004, Tom Lane wrote:
> Gavin Sherry <swm@linuxworld.com.au> writes:
> > Attached in the usual format this time.
>
> AFAICS this patch makes exactly zero change in behavior. What was
> the point again?
With BEFORE DELETE triggers, if the trigger returns NULL, then the DELETE
will not take place. The following is the existing code:
for (i = 0; i < ntrigs; i++)
{
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
if (!trigger->tgenabled)
continue;
LocTriggerData.tg_trigtuple = trigtuple;
LocTriggerData.tg_trigger = trigger;
newtuple = ExecCallTriggerFunc(&LocTriggerData,
relinfo->ri_TrigFunctions + tgindx[i],
GetPerTupleMemoryContext(estate));
if (newtuple == NULL)
break;
if (newtuple != trigtuple)
heap_freetuple(newtuple);
}
heap_freetuple(trigtuple);
return (newtuple == NULL) ? false : true;
Now, if for all the triggers on the base relation, !trigger->tgenabled is
true, then newtuple will always be NULL.
At the moment, this situation shouldn't come up. But it will when we
support DISABLE trigger. Arul, from Fujitsu, is planning to implement that
for 8.1 so I thought I'd ease the way.
>
> Also, if there is a point, why are we changing only one of the
> several ExecFOOTriggers functions?
Because only BEFORE DELETE worries about trigger procedures returning
NULL, from memory.
Thanks,
Gavin
Gavin Sherry <swm@linuxworld.com.au> writes:
> Attached in the usual format this time.
AFAICS this patch makes exactly zero change in behavior. What was
the point again?
Also, if there is a point, why are we changing only one of the
several ExecFOOTriggers functions?
regards, tom lane
Gavin Sherry <swm@linuxworld.com.au> writes:
> Now, if for all the triggers on the base relation, !trigger->tgenabled is
> true, then newtuple will always be NULL.
Hmm ... seems like the all-triggers-disabled case should act the same as
the no-triggers-at-all case, which this doesn't seem to do. It does
look broken but this doesn't seem like the correct fix.
>> Also, if there is a point, why are we changing only one of the
>> several ExecFOOTriggers functions?
> Because only BEFORE DELETE worries about trigger procedures returning
> NULL, from memory.
Do I have to do more than raise my eyebrow here?
regards, tom lane
Did this get resolved? --------------------------------------------------------------------------- Gavin Sherry wrote: > Attached is a minor patch to make BEFORE DELETE triggers honour tgenabled > properly. > > I know that we cannot, currently, use this feature through a DDL command > but just in case someone is updating the catalogs to do it and since it is > necessary in order to implement disabling of triggers, I thought I'd send > it in. > > Gavin Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Thu, 12 Aug 2004, Bruce Momjian wrote: > > Did this get resolved? > > --------------------------------------------------------------------------- > > Gavin Sherry wrote: > > Attached is a minor patch to make BEFORE DELETE triggers honour tgenabled > > properly. > > > > I know that we cannot, currently, use this feature through a DDL command > > but just in case someone is updating the catalogs to do it and since it is > > necessary in order to implement disabling of triggers, I thought I'd send > > it in. > > > > Gavin After taking a proper look, I agree with Tom that my patch was not the proper solution to the problem. What we really need for the BEFORE ROW triggers is the ability to say that there were no triggers executeed. At the moment, if no triggers are executed (ie, they're all disabled), then the executor thinks that a trigger returned NULL. I'll try to find some time to fix this soon. As I noted, though, its not critical because there's no DDL to disable a trigger. Gavin