Thread: TRIGGER with WHEN clause
Here is a patch to provide WHEN clause on triggers discussed here: http://archives.postgresql.org/pgsql-hackers/2009-10/msg00869.php Of course we can check conditions in the body of triggers, but there are some benefits to have separate WHEN clause for SQL standard compliance, portability, and saving AFTER trigger queues. There might be still incomplete and debatable part in the patch: * Usages of TupleTableSlot and RangeTblEntry might be ugly. Please notice me if there are better ways. * There are no "retry" feature to evaluate condition when the NEW tuple is modifed by another trigger. Triggers are still executed in alphabetical order, but it is adjustable. Comments welcome. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
Attachment
Here is a updated TRIGGER with WHEN cluase patch. I adjusted it to work with recent parser cleanup (*OLD* and *NEW*). Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
Attachment
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 <kaigai@ak.jp.nec.com>
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
closedthe connection unexpectedly This probably means the server terminated abnormally before or while
processingthe 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=<valueoptimized out>, numberTuples=<value optimized out>, sendTuples=<value optimized out>,
operation=<valueoptimized 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=<valueoptimized out>) at execMain.c:278 #7 0x0827a016 in ProcessQuery (plan=<value optimized out>,
sourceText=<valueoptimized 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
optimizedout>, 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
0x08277f43in 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>
+ <term><replaceable class="parameter">condition</replaceable></term>
+ <listitem>
+ <para>
+ Any <acronym>SQL</acronym> conditional expression (returning
+ <type>boolean</type>). Only <literal>FOR EACH ROW</literal> triggers
+ can refer <literal>NEW</> and <literal>OLD</> tuples.
+ <literal>INSERT</literal> trigger can refer <literal>NEW</>,
+ <literal>DELETE</literal> trigger can refer <literal>OLD</>,
+ and <literal>UPDATE</literal> trigger can refer both of them.
+ </para>
+ </listitem>
+ </varlistentry>
It saids, NEW and OLD are only available and ...o INSERT can refer NEWo UPDATE can refer NEW, OLDo 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;
+
+ if ((TRIGGER_FOR_DELETE(tgtype) || TRIGGER_FOR_UPDATE(tgtype)) &&
+ !TRIGGER_FOR_INSERT(tgtype))
+ {
+ rte = addRangeTableEntryForRelation(pstate, rel,
+ makeAlias("old", NIL), false, false);
+ rte->requiredPerms = 0;
+ addRTEtoQuery(pstate, rte, false, true, true);
+ }
+
+ if ((TRIGGER_FOR_INSERT(tgtype) || TRIGGER_FOR_UPDATE(tgtype)) &&
+ !TRIGGER_FOR_DELETE(tgtype))
+ {
+ rte = addRangeTableEntryForRelation(pstate, rel,
+ makeAlias("new", NIL), false, false);
+ rte->requiredPerms = 0;
+ addRTEtoQuery(pstate, rte, false, true, true);
+ }
+ }
I don't think the code is correct, but the SGML documentation does not reflect
the behavior correctly.
postgres=# CREATE TRIGGER t1_trig BEFORE update OR insert ON t1 FOR EACH ROW WHEN (OLD.a % 2 = 1) EXECUTE
PROCEDUREtrig_func();
ERROR: missing FROM-clause entry for table "old"
LINE 1: ... BEFORE update OR insert ON t1 FOR EACH ROW WHEN (OLD.a % 2 ...
* A minor coding style
*************** CreateTrigger(CreateTrigStmt *stmt, *** 387,392 **** --- 453,469 ---- tgattr =
buildint2vector(columns,ncolumns); values[Anum_pg_trigger_tgattr - 1] = PointerGetDatum(tgattr);
+ /* set tgqual if trigger has WHEN clause */ + if (qual) + { + values[Anum_pg_trigger_tgqual - 1] =
CStringGetTextDatum(qual);+ } + else + { + values[Anum_pg_trigger_tgqual - 1] = InvalidOid; +
nulls[Anum_pg_trigger_tgqual- 1] = true; + } + tuple = heap_form_tuple(tgrel->rd_att, values, nulls);
Is it unnecessary to set InvalidOid on the values[Anum_pg_trigger_tgqual - 1]?
* doc/src/sgml/catalogs.sgml is not updated
Could you add a short description about pg_trigger.tgqual system catalog?
Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
Thank for your reviewing!
KaiGai Kohei <kaigai@kaigai.gr.jp> wrote:
> However, I could find a few matters in this patch, as follows:
>
> * It does not prevent set up a conditional "statement" trigger.
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.
> 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.
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();
> * 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.
> * A minor coding style
> Is it unnecessary to set InvalidOid on the values[Anum_pg_trigger_tgqual - 1]?
Strange. There was something wrong with me.
> * doc/src/sgml/catalogs.sgml is not updated
> Could you add a short description about pg_trigger.tgqual system catalog?
Added.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Attachment
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
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 <kaigai@kaigai.gr.jp>
KaiGai Kohei <kaigai@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
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> + <term><replaceable class="parameter">condition</replaceable></term> + <listitem> +
<para>+ Any <acronym>SQL</acronym> conditional expression (returning + <type>boolean</type>). Only
<literal>FOREACH ROW</literal> triggers + can refer <literal>NEW</> and <literal>OLD</> tuples. +
<literal>INSERT</literal>trigger can refer <literal>NEW</>, + <literal>DELETE</literal> trigger can refer
<literal>OLD</>,+ and <literal>UPDATE</literal> trigger can refer both of them. + Note that if a trigger
hasmultiple events, it can refer only tuples + that can be referred in all of the events. For example, +
<literal>INSERT</><literal>OR</> <literal>DELETE</> trigger cannot + refer neither <literal>NEW</> nor
<literal>OLD</>tuples. + </para> + </listitem> + </varlistentry>
In addition, I could find a few matters.
* TOAST may be necessary for pg_trigger?
----------------------------------------
If we give very looooooong 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 ...])
executeprocedure 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();
CREATETRIGGER postgres=# COPY t1 FROM stdin; Enter data to be copied followed by a newline. End with a backslash and a
periodon a line by itself. >> 2 bbb >> server closed the connection unexpectedly This probably means the
serverterminated abnormally before or while processing the request. The connection to the server was lost.
Attemptingreset: 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
optimizedout>, lineNumber=<value optimized out>) at assert.c:57 #4 0x081956d9 in TriggerEnabled (trigger=<value
optimizedout>, event=<value optimized out>, modifiedCols=<value optimized out>, estate=<value optimized out>,
tupdesc=<valueoptimized out>, oldtup=<value optimized out>, newtup=<value optimized out>) at trigger.c:2410 #5
0x08196410in ExecBRInsertTriggers (estate=<value optimized out>, relinfo=<value optimized out>, trigtuple=<value
optimizedout>) at trigger.c:1835 #6 0x08162e0e in CopyFrom (cstate=<value optimized out>) at copy.c:2137 #7 DoCopy
(cstate=<valueoptimized out>) at copy.c:1189 #8 0x0827c653 in ProcessUtility (parsetree=<value optimized out>,
queryString=<valueoptimized out>, params=<value optimized out>, isTopLevel=<value optimized out>, dest=<value
optimizedout>, completionTag=<value optimized out>) at utility.c:581 #9 0x0827931d in PortalRunUtility
(portal=0x94a0e4c,utilityStmt=0x944133c, isTopLevel=<value optimized out>, dest=<value optimized out>,
completionTag=<valueoptimized out>) at pquery.c:1192 #10 0x0827a227 in PortalRunMulti (portal=0x94a0e4c,
isTopLevel=<valueoptimized out>, dest=<value optimized out>, altdest=<value optimized out>, completionTag=<value
optimizedout>) at pquery.c:1299 #11 0x0827aa64 in PortalRun (portal=<value optimized out>, count=<value optimized
out>, isTopLevel=<value optimized out>, dest=<value optimized out>, altdest=<value optimized out>,
completionTag=<valueoptimized out>) at pquery.c:823 #12 0x08276d80 in exec_simple_query (query_string=<value optimized
out>)at postgres.c:1046 #13 0x08277fd3 in PostgresMain (argc=<value optimized out>, argv=<value optimized out>,
username=<valueoptimized out>) at postgres.c:3624 #14 0x08241228 in BackendRun (port=<value optimized out>) at
postmaster.c:3366#15 BackendStartup (port=<value optimized out>) at postmaster.c:3073 #16 ServerLoop (port=<value
optimizedout>) at postmaster.c:1399 #17 0x08243c68 in PostmasterMain (argc=<value optimized out>, argv=<value optimized
out>)at postmaster.c:1064 #18 0x081e3d5f in main (argc=<value optimized out>, argv=0x93c5b38) at main.c:188
2407 if (TRIGGER_FIRED_BY_INSERT(event) || TRIGGER_FIRED_BY_UPDATE(event)) 2408 { 2409
Assert(newtup!= NULL); 2410 Assert(estate->es_trig_tuple_slot != NULL); <---- (*) 2411 2412 newslot
=estate->es_trig_tuple_slot; 2413 if (newslot->tts_tupleDescriptor != tupdesc) 2414
ExecSetSlotDescriptor(newslot,tupdesc); 2415 if (newslot->tts_tuple != newtup) 2416
ExecStoreTuple(newtup,newslot, InvalidBuffer, false); 2417 }
* Using system column in WHEN clause
------------------------------------
If we use references to the system columns in the WHEN clause, its result may
be unexpected one from user's perspective.
postgres=# CREATE TRIGGER tg_upd_row BEFORE UPDATE ON t1 FOR ROW WHEN (OLD.oid != NEW.oid) EXECUTE PROCEDURE
trig_func();CREATE TRIGGER postgres=# UPDATE t1 SET b = b; NOTICE: old=(1,aaa) new=(1,aaa) NOTICE: old=(2,bbb)
new=(2,bbb)NOTICE: old=(3,ccc) new=(3,ccc) UPDATE 3
From develope's perspective, it is quite natural because we know "oid" is
set within heap_update(), so these identifiers still have InvalidOid when
triggers are invoked.
I think we have two options for the matter:
1) Set correct values on the "oid" just before trigger invocations. (However, what values should be visible for
"tableoid","xmin", "ctid" and others? Is it really reasonable?)
2) Describe a notice on the user documentation not to use system columns in the WHEN clause, because these are
assignedon after the trigger invocations.
Here was a similar issue on the discussion related to suppress_redundant_updates_trigger().
See the following thread: http://archives.postgresql.org/pgsql-hackers/2008-11/thrd7.php#00273
In this case, we put an expected oid prior to comparison.
However, note that it is a special purpose function to avoid nonsense
updating, so this kind of workaround performs fine.
In my opinion, the (2) is more reasonable solution.
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>
KaiGai Kohei <kaigai@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 <literal>WHEN</> 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
Attachment
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 <kaigai@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 <literal>WHEN</> 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 <kaigai@ak.jp.nec.com>
Itagaki Takahiro <itagaki.takahiro@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
Itagaki Takahiro <itagaki.takahiro@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
Tom Lane <tgl@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