Re: [HACKERS] TRIGGER with WHEN clause
Tom Lane t...@sss.pgh.pa.us wrote: [ TRIGGER WHEN patch ] Applied with assorted revisions. AFAICT the system column issue is only a problem for NEW references in BEFORE triggers --- those columns do read correctly in OLD, and all the time in AFTER triggers. I revised the parsing logic to enforce that. The patch also missed establishing dependencies for stuff referenced in the WHEN expression, and there were a few other minor problems. Thanks for your fix. * Keep the expression in nodeToString string form within the TriggerDesc structure; then it's just one more pfree in FreeTriggerDesc. The main disadvantage of this is that we'd have to repeat stringToNode every time the trigger is used. I read your fix for the repeated compiling issue is to cache compiled expressions in ResultRelInfo-ri_TrigWhenExprs. So, we can reuse the result of stringToNode when we modify multiple rows in a query. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TRIGGER with WHEN clause
Itagaki Takahiro itagaki.takah...@oss.ntt.co.jp writes: [ TRIGGER WHEN patch ] Applied with assorted revisions. AFAICT the system column issue is only a problem for NEW references in BEFORE triggers --- those columns do read correctly in OLD, and all the time in AFTER triggers. I revised the parsing logic to enforce that. The patch also missed establishing dependencies for stuff referenced in the WHEN expression, and there were a few other minor problems. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TRIGGER with WHEN clause
Itagaki Takahiro itagaki.takah...@oss.ntt.co.jp writes: [ TRIGGER WHEN patch ] I'm starting to work this over now, and I've found one rather serious omission: FreeTriggerDesc doesn't free the expression tree. This means that trigger WHEN clauses will leak memory in CacheMemoryContext any time we do a relcache flush on the relation having the trigger. Over time that could be pretty nasty. There is no mechanism for freeing an expression tree explicitly, and creating one is not feasible because of the possibility of multiple references to subtrees, so this isn't trivial to fix. There are two alternatives that seem reasonable to me: * Keep the expression in nodeToString string form within the TriggerDesc structure; then it's just one more pfree in FreeTriggerDesc. The main disadvantage of this is that we'd have to repeat stringToNode every time the trigger is used. This might not be a big deal considering the other overhead of preparing an expression for execution --- check constraint expressions are handled that way IIRC --- but it's still a bit annoying. * Create a separate memory context for each TriggerDesc. This would simplify FreeTriggerDesc() to a MemoryContextDelete call, which seems attractive from both speed and code maintenance standpoints; but it would probably end up wasting a fair amount of space since the context would likely be mostly empty in most cases. Not sure which way to jump. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TRIGGER with WHEN clause
Itagaki-san, I don't have any more comments in this patch, so I hope it to be reviewed by committers then upstreamed. Thanks for your good jobs. Itagaki Takahiro wrote: KaiGai Kohei kai...@ak.jp.nec.com wrote: In addition, I could find a few matters. * TOAST may be necessary for pg_trigger? I added toast relation to pg_trigger. DECLARE_TOAST(pg_trigger, 2336, 2337); I think having a toast relation for pg_trigger is reasonable because pg_trigger already has a variable field tgargs even if we don't have the new field tgqual from the patch. I'm not sure why we don't have a toast relation for pg_trigger because user might pass very long trigger arguments. * ROW INSERT TRIGGER on COPY FROM statement Thanks. Good catch! Fixed and regression test added. * Using system column in WHEN clause 2) Describe a notice on the user documentation not to use system columns in the WHEN clause, because these are assigned on after the trigger invocations. I'd like to only add documentation because I don't have a whole solution. System columns are not available in the literalWHEN/ clause because those values are initialized after triggers are called. They might return wrong values if they used in expressions of the clause. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TRIGGER with WHEN clause
KaiGai Kohei wrote: I'm uncertain how Oracle handles the condition on the statement triggers. But it seems to me WHEN clause on the statement triggers are nonsense. SQL CREATE TRIGGER dummy BEFORE DELETE ON employees WHEN (1 = 1) 2 BEGIN 3 END; 4 / CREATE TRIGGER dummy BEFORE DELETE ON employees WHEN (1 = 1) * ERROR at line 1: ORA-04077: WHEN clause cannot be used with table level triggers Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TRIGGER with WHEN clause
Albe Laurenz wrote: SQL CREATE TRIGGER dummy BEFORE DELETE ON employees WHEN (1 = 1) 2 BEGIN 3 END; 4 / CREATE TRIGGER dummy BEFORE DELETE ON employees WHEN (1 = 1) * ERROR at line 1: ORA-04077: WHEN clause cannot be used with table level triggers Thanks for your information. I am also not sure about Oracle, but I think there are usage of statement trigger with WHEN cluase something like: =# CREATE TRIGGER log_trig BEFORE UPDATE ON tbl WHEN (is_superuser()) EXECUTE PROCEDURE log_current_stmt(); Itagaki-san, I also think your example usage is enough valueable. However, Oracle does not have the feature apparently, although the purpose of this patch is to provide a compatible feature, IIRC. I don't have any preference on either of them. If you make a decision, I'll review the patch according to your decision. So, I like to ask you which is your preference again. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TRIGGER with WHEN clause
KaiGai Kohei kai...@kaigai.gr.jp wrote: Itagaki-san, I also think your example usage is enough valueable. However, Oracle does not have the feature apparently, although the purpose of this patch is to provide a compatible feature, IIRC. I don't have any preference on either of them. If you make a decision, I'll review the patch according to your decision. So, I like to ask you which is your preference again. I'd like to add support statement triggers with WHEN clause. Of cource Oracle is a good example, but it doesn't prevent us from developing a better copy :) Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TRIGGER with WHEN clause
Itagaki-san, I checked the latest patch. It seems to me the patch getting improved from the prior version. We are next to the commiter review phase. But I could find a few matters. :-( Please see the following comments, and fix it before sending it to commiters. I fixed the bug and two other bugs: * Crash in AFTER TRIGGER + WHEN clause. * Incorrect behavior when multiple tuples are modified. Also regression tests for it are added. It looks for me fine. I'd like to add support statement triggers with WHEN clause. Of cource Oracle is a good example, but it doesn't prevent us from developing a better copy :) OK, it is your decision. * the documentation seems to me misleading. It saids, NEW and OLD are only available and ... o INSERT can refer NEW o UPDATE can refer NEW, OLD o DELETE can refer OLD But, it may actually incorrect, if user gives several events on a certain trigger. For example, when a new trigger is invoked for each row on INSERT or UPDATE statement, the function cannot refer the OLD. They are bitwise-AND flags. INSERT-OR-DELETE trigger cannot refer to both OLD and NEW tuples. It is possible to use a dummy tuple (filled with NULLs?) in the cases, but I want to just throw an error as of now. I'll fix documentation to reflect the code. Ideas for better descriptions welcome. | Note that if a trigger has multiple events, it can refer only tuples | that can be referred in all of the events. For example, | INSERT OR DELETE trigger cannot refer neither NEW nor OLD tuples. At least, it seems to me meaningful. Is there any comments from native English users? varlistentry + termreplaceable class=parametercondition/replaceable/term + listitem + para + Any acronymSQL/acronym conditional expression (returning + typeboolean/type). Only literalFOR EACH ROW/literal triggers + can refer literalNEW/ and literalOLD/ tuples. + literalINSERT/literal trigger can refer literalNEW/, + literalDELETE/literal trigger can refer literalOLD/, + and literalUPDATE/literal trigger can refer both of them. + Note that if a trigger has multiple events, it can refer only tuples + that can be referred in all of the events. For example, + literalINSERT/ literalOR/ literalDELETE/ trigger cannot + refer neither literalNEW/ nor literalOLD/ tuples. + /para + /listitem +/varlistentry In addition, I could find a few matters. * TOAST may be necessary for pg_trigger? If we give very looong condition on the WHEN clause, the pg_trigger.tgqual can over the limitation of block size. postgres=# CREATE TRIGGER hoge before insert on t1 for row when (a [... very long condition ...]) execute procedure trig_func(); ERROR: row is too big: size 12940, maximum size 8164 But it is a quite corner case in my opinion. It depends on your preference. * ROW INSERT TRIGGER on COPY FROM statement --- The Assert() in TriggerEnabled (commands/trigger.c:2410) was mistaken bombing. In the code path from copy.c, NULL can be set on the estate-es_trig_tuple_slot. postgres=# CREATE TRIGGER tg_ins_row BEFORE INSERT ON t1 FOR ROW WHEN (true) EXECUTE PROCEDURE trig_func(); CREATE TRIGGER postgres=# COPY t1 FROM stdin; Enter data to be copied followed by a newline. End with a backslash and a period on a line by itself. 2bbb server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Succeeded. (gdb) bt #0 0x00cd4416 in __kernel_vsyscall () #1 0x009beba1 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #2 0x009c046a in abort () at abort.c:92 #3 0x08330e7e in ExceptionalCondition (conditionName=value optimized out, errorType=value optimized out, fileName=value optimized out, lineNumber=value optimized out) at assert.c:57 #4 0x081956d9 in TriggerEnabled (trigger=value optimized out, event=value optimized out, modifiedCols=value optimized out, estate=value optimized out, tupdesc=value optimized out, oldtup=value optimized out, newtup=value optimized out) at trigger.c:2410 #5 0x08196410 in ExecBRInsertTriggers (estate=value optimized out, relinfo=value optimized out, trigtuple=value optimized out) at trigger.c:1835 #6 0x08162e0e in CopyFrom (cstate=value optimized out) at copy.c:2137 #7 DoCopy (cstate=value optimized out) at copy.c:1189 #8 0x0827c653 in ProcessUtility (parsetree=value optimized out, queryString=value optimized out, params=value optimized out, isTopLevel=value optimized out, dest=value optimized out, completionTag=value optimized out) at utility.c:581 #9 0x0827931d in PortalRunUtility (portal=0x94a0e4c,
Re: [HACKERS] TRIGGER with WHEN clause
KaiGai Kohei wrote:
Itagaki Takahiro wrote:
Here is a updated TRIGGER with WHEN cluase patch.
I adjusted it to work with recent parser cleanup (*OLD* and *NEW*).
I would like to volunteer for reviewing the patch in RR-stage.
It seems to me an interesting feature.
It seems to me you did an impressive work.
However, I could find a few matters in this patch, as follows:
* It does not prevent set up a conditional statement trigger.
I'm uncertain how Oracle handles the condition on the statement
triggers. But it seems to me WHEN clause on the statement triggers
are nonsense.
If you have any opposition, the CreateTrigger() should raise an error
when a valid stmt-whenClause is given for statement triggers.
In addition, it makes a server crash. :-)
postgres=# CREATE or replace function trig_func() returns trigger
language plpgsql as
'begin raise notice ''trigger invoked''; return null; end';
CREATE FUNCTION
postgres=# CREATE TRIGGER t1_trig BEFORE update ON t1
WHEN (true) EXECUTE PROCEDURE trig_func();
^^^
CREATE TRIGGER
postgres=# update t1 set b = b;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The core dump says:
(gdb) bt
#0 0x08195396 in TriggerEnabled (trigger=0x9c7a0c4, event=10,
modifiedCols=0x9c484bc, estate=0x0,
tupdesc=0x0, oldtup=0x0, newtup=0x0) at trigger.c:2376
#1 0x08196783 in ExecBSUpdateTriggers (estate=0x9c79f44, relinfo=0x9c79fec)
at trigger.c:2040
#2 0x081cd4e9 in fireBSTriggers (node=value optimized out) at
nodeModifyTable.c:593
#3 ExecModifyTable (node=value optimized out) at nodeModifyTable.c:657
#4 0x081b70b8 in ExecProcNode (node=0x9c7a190) at execProcnode.c:359
#5 0x081b5c0d in ExecutePlan (dest=value optimized out, direction=value
optimized out,
numberTuples=value optimized out, sendTuples=value optimized out,
operation=value optimized out, planstate=value optimized out,
estate=value optimized out)
at execMain.c:1189
#6 standard_ExecutorRun (dest=value optimized out, direction=value
optimized out,
numberTuples=value optimized out, sendTuples=value optimized out,
operation=value optimized out, planstate=value optimized out,
estate=value optimized out)
at execMain.c:278
#7 0x0827a016 in ProcessQuery (plan=value optimized out, sourceText=value
optimized out,
params=value optimized out, dest=0x9c72024, completionTag=0xbfb8a51e
) at pquery.c:196
#8 0x0827a24e in PortalRunMulti (portal=0x9beabec, isTopLevel=value
optimized out,
dest=value optimized out, altdest=0x9c72024, completionTag=0xbfb8a51e
) at pquery.c:1269
#9 0x0827a9d4 in PortalRun (portal=0x9beabec, count=2147483647,
isTopLevel=36 '$', dest=0x9c72024,
altdest=0x9c72024, completionTag=0xbfb8a51e ) at pquery.c:823
#10 0x08276cf0 in exec_simple_query (query_string=value optimized out) at
postgres.c:1046
#11 0x08277f43 in PostgresMain (argc=2, argv=0x9bcfb70, username=0x9bcfb40
kaigai) at postgres.c:3624
#12 0x08241198 in BackendRun (port=value optimized out) at postmaster.c:3366
#13 BackendStartup (port=value optimized out) at postmaster.c:3073
#14 ServerLoop (port=value optimized out) at postmaster.c:1399
#15 0x08243bd8 in PostmasterMain (argc=1, argv=0x9bcda18) at postmaster.c:1064
#16 0x081e3d5f in main (argc=1, argv=0x9bcda18) at main.c:188
2368 /* Check for WHEN clause */
2369 if (trigger-tgqual)
2370 {
2371 ExprContext*econtext;
2372 List *predicate;
2373 TupleTableSlot *oldslot = NULL;
2374 TupleTableSlot *newslot = NULL;
2375
2376 *econtext = GetPerTupleExprContext(estate);
2377
2378 predicate = list_make1(ExecPrepareExpr((Expr *)
trigger-tgqual, estate));
2379
* the documentation seems to me misleading.
varlistentry
+ termreplaceable class=parametercondition/replaceable/term
+ listitem
+ para
+ Any acronymSQL/acronym conditional expression (returning
+ typeboolean/type). Only literalFOR EACH ROW/literal triggers
+ can refer literalNEW/ and literalOLD/ tuples.
+ literalINSERT/literal trigger can refer literalNEW/,
+ literalDELETE/literal trigger can refer literalOLD/,
+ and literalUPDATE/literal trigger can refer both of them.
+ /para
+ /listitem
+/varlistentry
It saids, NEW and OLD are only available and ...
o INSERT can refer NEW
o UPDATE can refer NEW, OLD
o DELETE can refer OLD
But, it may actually incorrect, if user gives several events on a certain
trigger. For example, when a new trigger is invoked for each row on INSERT
or UPDATE statement, the function cannot refer the OLD.
+ if (TRIGGER_FOR_ROW(tgtype))
+ {
+ RangeTblEntry *rte;
+
+
Re: [HACKERS] TRIGGER with WHEN clause
Itagaki Takahiro wrote: Here is a updated TRIGGER with WHEN cluase patch. I adjusted it to work with recent parser cleanup (*OLD* and *NEW*). I would like to volunteer for reviewing the patch in RR-stage. It seems to me an interesting feature. -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
On Thu, 2009-10-15 at 16:54 -0500, Kevin Grittner wrote: Dimitri Fontaine dfonta...@hi-media.com wrote: It's pretty often the case (IME) that calling a trigger is the only point in the session where you fire plpgsql, and that's a visible cost. Wouldn't a connection pool solve this? No -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
On Fri, 2009-10-16 at 10:14 +0900, Itagaki Takahiro wrote: Tom Lane t...@sss.pgh.pa.us wrote: Itagaki Takahiro itagaki.takah...@oss.ntt.co.jp writes: I think there is a benefit to provide WHEN cluase at least for compatibility with other DBMSs, even through we can move the expressions into the body of trigger functions. This seems to me to be a lot of code to accomplish nothing useful. It will always be the case that any nontrivial logic has to be done inside the trigger. And the compatibility argument is entirely pointless given the lack of compatibility in the trigger function itself. I see that WHEN cluase is not always needed, but I think there are several benefits to have it: * WHEN cluase is in SQL standard. * We could recheck trigger conditions when NEW tuple is modified. (not yet implemented in the patch, though) http://archives.postgresql.org/pgsql-hackers/2009-09/msg00286.php * As for compatibility, it is easy for typical users to move trigger bodies into a function, but they don't like to merge the condition and the bodies into a function in my experience. +1 because * It is SQL Standard * Other RDBMS support it (Oracle, DB2) * It will reduce size of in-memory pending trigger list (for large statements) and avoid invoking run-time of function handlers, important for heavier PLs (for small statements) I also support addition of INSTEAD OF triggers for first two reasons and because it may be an easier route to allow updateable views. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
On Fri, 2009-10-16 at 09:31 +0100, Simon Riggs wrote: * It will reduce size of in-memory pending trigger list (for large statements) But this won't be the outcome when it's implemented the way it is being proposed, which checks the where clause just before executing the trigger function. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
On Fri, 2009-10-16 at 14:39 +0300, Peter Eisentraut wrote: On Fri, 2009-10-16 at 09:31 +0100, Simon Riggs wrote: * It will reduce size of in-memory pending trigger list (for large statements) But this won't be the outcome when it's implemented the way it is being proposed, which checks the where clause just before executing the trigger function. Thanks for pointing that out. I'm giving reasons why we'd want a WHEN clause. If the current implementation doesn't do all that it could, then ISTM thats a reason to reject patch for now, but not for all time. Incidentally, re-accessing a data block at end of statement may have caused to block to fall out of cache and then be re-accessed again. So optimising that away can save on I/O as well. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
Peter Eisentraut pete...@gmx.net writes: On Fri, 2009-10-16 at 09:31 +0100, Simon Riggs wrote: * It will reduce size of in-memory pending trigger list (for large statements) But this won't be the outcome when it's implemented the way it is being proposed, which checks the where clause just before executing the trigger function. Hm, the feature could actually be worth something if it allows conditions to be checked before an AFTER trigger event gets pushed into the event list. However, if it's not doing that ... I note BTW that we have some ad-hoc logic already that arranges to suppress queuing of AFTER events for FK triggers, if the FK column value has not changed. It might be interesting to look at whether that hack could be unified with the user-accessible feature. It's essentially a WHEN OLD.x IS NOT DISTINCT FROM NEW.x test. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
On Fri, 2009-10-16 at 10:02 -0400, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On Fri, 2009-10-16 at 09:31 +0100, Simon Riggs wrote: * It will reduce size of in-memory pending trigger list (for large statements) But this won't be the outcome when it's implemented the way it is being proposed, which checks the where clause just before executing the trigger function. Hm, the feature could actually be worth something if it allows conditions to be checked before an AFTER trigger event gets pushed into the event list. However, if it's not doing that ... I note BTW that we have some ad-hoc logic already that arranges to suppress queuing of AFTER events for FK triggers, if the FK column value has not changed. It might be interesting to look at whether that hack could be unified with the user-accessible feature. It's essentially a WHEN OLD.x IS NOT DISTINCT FROM NEW.x test. If the function is idempotent then we can also optimise away multiple calls by checking whether a similar call is already queued. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
Simon Riggs si...@2ndquadrant.com writes: If the function is idempotent then we can also optimise away multiple calls by checking whether a similar call is already queued. But how would we know that? It seems orthogonal to this patch, anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
On Fri, 16 Oct 2009, Tom Lane wrote: I note BTW that we have some ad-hoc logic already that arranges to suppress queuing of AFTER events for FK triggers, if the FK column value has not changed. It might be interesting to look at whether that hack could be unified with the user-accessible feature. It's essentially a WHEN OLD.x IS NOT DISTINCT FROM NEW.x test. Note that one of those cases (RI_TRIGGER_FK) is a bit special due to the transaction id test. It might be worth seeing if a better solution is possible to cover the case in the comment if the above becomes possible, though. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
Stephan Szabo ssz...@megazone.bigpanda.com writes: On Fri, 16 Oct 2009, Tom Lane wrote: I note BTW that we have some ad-hoc logic already that arranges to suppress queuing of AFTER events for FK triggers, if the FK column value has not changed. It might be interesting to look at whether that hack could be unified with the user-accessible feature. It's essentially a WHEN OLD.x IS NOT DISTINCT FROM NEW.x test. Note that one of those cases (RI_TRIGGER_FK) is a bit special due to the transaction id test. It might be worth seeing if a better solution is possible to cover the case in the comment if the above becomes possible, though. The existing FK short-circuit code is presumably faster than generic WHEN expression evaluation would be, so I wasn't really imagining that we would take it out in favor of using the generic feature. But possibly we could clean things up to some extent by treating the FK case as a variant of generic WHEN. One particular thing I never much liked about that hack is its dependence on specific function OIDs. I could see replacing that with some representation that says this trigger has a special FK WHEN clause as opposed to providing an expression tree. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
2009/10/15 Tom Lane t...@sss.pgh.pa.us: Itagaki Takahiro itagaki.takah...@oss.ntt.co.jp writes: I think there is a benefit to provide WHEN cluase at least for compatibility with other DBMSs, even through we can move the expressions into the body of trigger functions. This seems to me to be a lot of code to accomplish nothing useful. It will always be the case that any nontrivial logic has to be done inside the trigger. And the compatibility argument is entirely pointless given the lack of compatibility in the trigger function itself. -1 I disagree. When I analysed speed of some operations, I found some unwanted trigger calls should to slow down applications. I am for any method, that could to decrease trigger calls. Regards Pavel Stehule regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
Itagaki Takahiro itagaki.takah...@oss.ntt.co.jp writes: I think there is a benefit to provide WHEN cluase at least for compatibility with other DBMSs, even through we can move the expressions into the body of trigger functions. This seems to me to be a lot of code to accomplish nothing useful. It will always be the case that any nontrivial logic has to be done inside the trigger. And the compatibility argument is entirely pointless given the lack of compatibility in the trigger function itself. -1 regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
Pavel Stehule pavel.steh...@gmail.com writes: 2009/10/15 Tom Lane t...@sss.pgh.pa.us: This seems to me to be a lot of code to accomplish nothing useful. I disagree. When I analysed speed of some operations, I found some unwanted trigger calls should to slow down applications. I am for any method, that could to decrease trigger calls. That argument is based on a completely evidence-free assumption, namely that this patch would make your case faster. Executing the WHEN tests is hardly going to be zero cost. It's not too hard to postulate cases where implementing a filter this way would be *slower* than doing it inside the trigger. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
Tom Lane t...@sss.pgh.pa.us writes: That argument is based on a completely evidence-free assumption, namely that this patch would make your case faster. Executing the WHEN tests is hardly going to be zero cost. It's not too hard to postulate cases where implementing a filter this way would be *slower* than doing it inside the trigger. It's pretty often the case (IME) that calling a trigger is the only point in the session where you fire plpgsql, and that's a visible cost. Last time I had to measure it, it was 1ms per call. We were trying to optimize queries running in 3ms to 4ms, called more than 100 times a second (in parallel on multi core architecture, but still). The way I understand it, having the WHEN clause in CREATE TRIGGER would allow to filter out some interpreter initialisations. Regards, -- dim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
Dimitri Fontaine dfonta...@hi-media.com wrote: It's pretty often the case (IME) that calling a trigger is the only point in the session where you fire plpgsql, and that's a visible cost. Wouldn't a connection pool solve this? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trigger with WHEN clause (WIP)
Tom Lane t...@sss.pgh.pa.us wrote: Itagaki Takahiro itagaki.takah...@oss.ntt.co.jp writes: I think there is a benefit to provide WHEN cluase at least for compatibility with other DBMSs, even through we can move the expressions into the body of trigger functions. This seems to me to be a lot of code to accomplish nothing useful. It will always be the case that any nontrivial logic has to be done inside the trigger. And the compatibility argument is entirely pointless given the lack of compatibility in the trigger function itself. I see that WHEN cluase is not always needed, but I think there are several benefits to have it: * WHEN cluase is in SQL standard. * We could recheck trigger conditions when NEW tuple is modified. (not yet implemented in the patch, though) http://archives.postgresql.org/pgsql-hackers/2009-09/msg00286.php * As for compatibility, it is easy for typical users to move trigger bodies into a function, but they don't like to merge the condition and the bodies into a function in my experience. * As for performance, I don't have any evidence. But there was a discussion about benefits of writing partitioning trigger function with C instead of PL/pgSQL. He said pgplsql is slow. http://archives.postgresql.org/pgsql-hackers/2008-12/msg01221.php Also, to separate trigger bodies and conditions are useful when we write triggers in C because we don't have to recomplie the code. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
