Thread: Odd(?) RI-trigger behavior
I was just fooling around with replacing the existing plain index on
pg_trigger.tgrelid with a unique index on (tgrelid, tgname). In theory
this should not affect anything --- the code already enforced that two
triggers on the same relation can't have the same name. The index
should merely provide a backup check.
So I was a tad surprised to get a regression test failure:
*** ./expected/foreign_key.out Thu Apr 11 15:13:36 2002
--- ./results/foreign_key.out Thu Apr 18 21:26:20 2002
***************
*** 899,905 **** ERROR: <unnamed> referential integrity violation - key in pktable still referenced from pktable --
fails(1,1) is being referenced (twice) update pktable set base1=3 where base1=1;
! ERROR: <unnamed> referential integrity violation - key in pktable still referenced from pktable -- this sequence of
twodeletes will work, since after the first there will be no (2,*) references delete from pktable where base2=2; delete
frompktable where base1=2;
--- 899,905 ---- ERROR: <unnamed> referential integrity violation - key in pktable still referenced from pktable --
fails(1,1) is being referenced (twice) update pktable set base1=3 where base1=1;
! ERROR: <unnamed> referential integrity violation - key referenced from pktable not found in pktable -- this sequence
oftwo deletes will work, since after the first there will be no (2,*) references delete from pktable where base2=2;
deletefrom pktable where base1=2;
======================================================================
This particular test involves a table with a foreign-key reference to
itself, ie, it's both PK and FK. What apparently is happening is that
the two RI triggers are now being fired in a different order than
before. While either of them would have detected an error, we now get
the other error first.
Does this bother anyone? It seems to me that the old code essentially
had no guarantee at all about the order in which the triggers would
fire, and so it was pure luck that the regression test never showed
the other message.
With the modified code, because we load the triggers by scanning
an index on (tgrelid, tgname), it is actually true that triggers are
fired in name order. We've had requests in the past to provide a
well-defined firing order for triggers --- should we document this
behavior and support it, or should we pretend it ain't there?
BTW, the same goes for rules: it would now be pretty easy to guarantee
that rules are fired in name order.
regards, tom lane
On Thu, 18 Apr 2002, Tom Lane wrote: > This particular test involves a table with a foreign-key reference to > itself, ie, it's both PK and FK. What apparently is happening is that > the two RI triggers are now being fired in a different order than > before. While either of them would have detected an error, we now get > the other error first. > > Does this bother anyone? It seems to me that the old code essentially > had no guarantee at all about the order in which the triggers would > fire, and so it was pure luck that the regression test never showed > the other message. That's probably a bad thing even if I doubt that it'd ever come up the other way barring changes to other regression tests in practice. Forcing an order probably helps with this case anyway. > With the modified code, because we load the triggers by scanning > an index on (tgrelid, tgname), it is actually true that triggers are > fired in name order. We've had requests in the past to provide a > well-defined firing order for triggers --- should we document this > behavior and support it, or should we pretend it ain't there? Didn't someone (Peter?) say that the mandated firing order was based on creation order/time in SQL99?
Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> Didn't someone (Peter?) say that the mandated firing order was based on
> creation order/time in SQL99?
It does say that:
The order of execution of a set of triggers is ascending by value of their timestamp of creation in
theirdescriptors, such that the oldest trigger executes first. If one or more triggers have the same
timestampvalue, then their relative order of execution is implementation-defined.
However, this strikes me as fairly brain-dead; it's unnecessarily hard
to control the order of trigger execution. You have to drop and
recreate triggers if you want to insert a new one at a desired position.
Worse, if you create several triggers in the same transaction, they'll
have the same timestamp --- leaving you right back in the
implementation-defined case. But if you want to make your rearrangement
atomically with respect to other transactions, you have little choice
but to drop/recreate in one xact. Looks like a catch-22 to me.
ISTM we had discussed this before and concluded that name order was
a more reasonable definition. Nobody had got round to doing anything
about it though. (Indeed my current hack was not intended to provide
a predictable firing order, it just fell out that way...)
regards, tom lane
On Thu, 18 Apr 2002, Tom Lane wrote: > Stephan Szabo <sszabo@megazone23.bigpanda.com> writes: > > Didn't someone (Peter?) say that the mandated firing order was based on > > creation order/time in SQL99? > > It does say that: > > The order of execution of a set of triggers is ascending by value > of their timestamp of creation in their descriptors, such that the > oldest trigger executes first. If one or more triggers have the > same timestamp value, then their relative order of execution is > implementation-defined. > > However, this strikes me as fairly brain-dead; it's unnecessarily hard > to control the order of trigger execution. You have to drop and > recreate triggers if you want to insert a new one at a desired position. > Worse, if you create several triggers in the same transaction, they'll > have the same timestamp --- leaving you right back in the > implementation-defined case. But if you want to make your rearrangement > atomically with respect to other transactions, you have little choice > but to drop/recreate in one xact. Looks like a catch-22 to me. > > ISTM we had discussed this before and concluded that name order was > a more reasonable definition. Nobody had got round to doing anything > about it though. (Indeed my current hack was not intended to provide > a predictable firing order, it just fell out that way...) I agree that name is better, I wasn't sure if we'd reached a consensus on it or if the conversation drifted away due to the fact that noone was looking at it at the time.
En Thu, 18 Apr 2002 20:43:54 -0700 (PDT) Stephan Szabo <sszabo@megazone23.bigpanda.com> escribió: > I agree that name is better, I wasn't sure if we'd reached a consensus on > it or if the conversation drifted away due to the fact that noone was > looking at it at the time. http://archives.postgresql.org/pgsql-general/2001-09/msg00234.php Nobody opposed to the idea of name ordering in that thread. But note that this is on TODO: * Allow user to control trigger firing order That probably means that the user should have some reasonable way to change the name, besides fiddling with system catalogs. -- Alvaro Herrera (<alvherre[a]atentus.com>) "Siempre hay que alimentar a los dioses, aunque la tierra este seca" (Orual)
> But note that this is on TODO: > > * Allow user to control trigger firing order > > That probably means that the user should have some reasonable way to > change the name, besides fiddling with system catalogs. An ALTER TRIGGER command? Of course, it should not allow modification of constraint triggers... Chris
Alvaro Herrera <alvherre@atentus.com> writes: > http://archives.postgresql.org/pgsql-general/2001-09/msg00234.php > Nobody opposed to the idea of name ordering in that thread. Okay, I've committed the fixes that implement this. > But note that this is on TODO: > * Allow user to control trigger firing order > That probably means that the user should have some reasonable way to > change the name, besides fiddling with system catalogs. Yeah. As of CVS tip, to reshuffle the order of existing triggers you must (a) do a manual UPDATE pg_trigger SET tgname = 'something' ... then (b) restart your backend(s), because the relcache code does not notice that you did that, so it'll keep using the trigger data it already had loaded. This is pretty ugly. An ALTER TRIGGER command seems called for if we want to call the TODO item really done. I haven't got time for that at the moment; any volunteers? regards, tom lane
Tom Lane wrote: > > Yeah. As of CVS tip, to reshuffle the order of existing triggers you > must (a) do a manual UPDATE pg_trigger SET tgname = 'something' ... > then (b) restart your backend(s), because the relcache code does not > notice that you did that, so it'll keep using the trigger data it > already had loaded. This is pretty ugly. An ALTER TRIGGER command > seems called for if we want to call the TODO item really done. > I haven't got time for that at the moment; any volunteers? > I'll take it. Joe
Joe Conway wrote: > Tom Lane wrote: > >> >> Yeah. As of CVS tip, to reshuffle the order of existing triggers you >> must (a) do a manual UPDATE pg_trigger SET tgname = 'something' ... >> then (b) restart your backend(s), because the relcache code does not >> notice that you did that, so it'll keep using the trigger data it >> already had loaded. This is pretty ugly. An ALTER TRIGGER command >> seems called for if we want to call the TODO item really done. >> I haven't got time for that at the moment; any volunteers? >> > > I'll take it. > There is already a RenameStmt node which is currently only used to rename tables or table column names. Is there any objection to modifying it to handle trigger names (and possibly other things in the future) also? Joe
Joe Conway <mail@joeconway.com> writes:
> There is already a RenameStmt node which is currently only used to
> rename tables or table column names. Is there any objection to modifying
> it to handle trigger names (and possibly other things in the future) also?
You'd need to add a field so you could distinguish the type of rename,
but on the whole that seems a reasonable thing to do; probably better
than adding a brand new node type. We're already sharing node types
for DROPs, for example, so I see no reason not to do it for RENAMEs.
(Cf 'DropPropertyStmt' in current sources)
Renaming rules seems like something that should be on the list too,
so you're right that there will be more stuff later.
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@atentus.com> writes: > > http://archives.postgresql.org/pgsql-general/2001-09/msg00234.php > > Nobody opposed to the idea of name ordering in that thread. > > Okay, I've committed the fixes that implement this. > > > But note that this is on TODO: > > * Allow user to control trigger firing order > > That probably means that the user should have some reasonable way to > > change the name, besides fiddling with system catalogs. > > Yeah. As of CVS tip, to reshuffle the order of existing triggers you > must (a) do a manual UPDATE pg_trigger SET tgname = 'something' ... > then (b) restart your backend(s), because the relcache code does not > notice that you did that, so it'll keep using the trigger data it > already had loaded. This is pretty ugly. An ALTER TRIGGER command > seems called for if we want to call the TODO item really done. > I haven't got time for that at the moment; any volunteers? TODO updated with: * -Allow user to control trigger firing order* Add ALTER TRIGGER ... RENAME -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026