Re: [PATCHES] Minor BEFORE DELETE trigger fix
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 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] Minor BEFORE DELETE trigger fix
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 [EMAIL PROTECTED] so that your > message can get through to the mailing list cleanly -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] Minor BEFORE DELETE trigger fix
Gavin Sherry <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Minor BEFORE DELETE trigger fix
Gavin Sherry <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Minor BEFORE DELETE trigger fix
On Sat, 7 Aug 2004, Tom Lane wrote:
> Gavin Sherry <[EMAIL PROTECTED]> 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
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
Re: [PATCHES] Minor BEFORE DELETE trigger fix
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 [EMAIL PROTECTED] so that your
> > message can get through to the mailing list cleanly
>
> --
> Bruce Momjian| http://candle.pha.pa.us
> [EMAIL PROTECTED] | (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 [EMAIL PROTECTED] so that your
> message can get through to the mailing list cleanly
>
>
> !DSPAM:41144fb520531574347913!
>
>Index: src/backend/commands/trigger.c
===
RCS file: /usr/local/cvsroot/pgsql-server/src/backend/commands/trigger.c,v
retrieving revision 1.166
diff -2 -c -r1.166 trigger.c
*** src/backend/commands/trigger.c 1 Jul 2004 00:50:11 - 1.166
--- src/backend/commands/trigger.c 5 Aug 2004 01:25:46 -
***
*** 1350,1353
--- 1350,1354
TupleTableSlot *newSlot;
int i;
+ boolret;
trigtuple = GetTupleForTrigger(estate, relinfo, tupleid, cid, &newSlot);
***
*** 1366,1369
--- 1367,1371
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
LocTriggerData.tg_newtuple = NULL;
+ ret = true;
for (i = 0; i < ntrigs; i++)
{
***
*** 1378,1382
--- 1380,1387
GetPerTupleMemoryContext(estate));
if (newtuple == NULL)
+ {
+ ret = false;
break;
+ }
if (newtuple != trigtuple)
heap_freetuple(newtuple);
***
*** 1384,1388
heap_freetuple(trigtuple);
! return (newtuple == NULL) ? false : true;
}
--- 1389,1393
heap_freetuple(trigtuple);
! return (ret);
}
---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [PATCHES] Minor BEFORE DELETE trigger fix
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 [EMAIL PROTECTED] so that your > message can get through to the mailing list cleanly -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] 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.
GavinIndex: src/backend/commands/trigger.c
===
RCS file: /usr/local/cvsroot/pgsql-server/src/backend/commands/trigger.c,v
retrieving revision 1.166
diff -r1.166 trigger.c
1351a1352
> boolret;
1367a1369
> ret = true;
1379a1382,1383
> {
> ret = false;
1380a1385
> }
1386c1391
< return (newtuple == NULL) ? false : true;
---
> return (ret);
---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
