Trigger with WHEN clause (WIP)
I'm working on WHEN clause support in triggers.
I heard that the feature is in the SQL standard.
We can rewrite triggers that uses suppress_redundant_updates_trigger
http://www.postgresql.org/docs/8.4/static/functions-trigger.html
with WHEN clause:
CREATE TRIGGER modified_any
BEFORE UPDATE ON tbl
FOR EACH ROW
WHEN (OLD.* IS DISTINCT FROM NEW.*)
EXECUTE PROCEDURE trigger_func();
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.
WIP-patch attached. It adds 'tgqual' text field into pg_trigger.
It works at first glance, but surely needs some adjustments
especially in the usage of TupleTableSlot in TriggerEnabled().
I have a question about executing qualification expression.
I used ecxt_innertuple for old tuples and ecxt_outertuple
for new tuples, but I'm not sure it is the right way.
Comments and suggestions welcome.
James Pye <lists@jwp.name> wrote:
Well, looks like WHEN is, or is going to be standard:
<triggered action> ::=
[ FOREACH { ROW | STATEMENT } ]
[ WHEN<left paren><search condition> <right paren> ]
<triggered SQL statement>(page 653 from 5CD2-02-Foundation-2006-01)
David Fetter <david@fetter.org> wrote:
Page 674 of 6WD_02_Foundation_2007-12 has a similar thing:
<triggered action> ::=
[ FOR EACH { ROW | STATEMENT } ]
[ <triggered when clause> ]
<triggered SQL statement><triggered when clause> ::=
WHEN <left paren> <search condition> <right paren>
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Attachments:
trigger-when_20091015.patchapplication/octet-stream; name=trigger-when_20091015.patchDownload
diff -cprN head/src/backend/catalog/index.c work/src/backend/catalog/index.c
*** head/src/backend/catalog/index.c 2009-10-15 10:26:48.573966000 +0900
--- work/src/backend/catalog/index.c 2009-10-15 13:38:54.289177704 +0900
*************** index_create(Oid heapRelationId,
*** 798,804 ****
trigger->initdeferred = initdeferred;
trigger->constrrel = NULL;
! (void) CreateTrigger(trigger, conOid, indexRelationId,
isprimary ? "PK_ConstraintTrigger" :
"Unique_ConstraintTrigger",
false);
--- 798,804 ----
trigger->initdeferred = initdeferred;
trigger->constrrel = NULL;
! (void) CreateTrigger(trigger, NULL, conOid, indexRelationId,
isprimary ? "PK_ConstraintTrigger" :
"Unique_ConstraintTrigger",
false);
diff -cprN head/src/backend/commands/tablecmds.c work/src/backend/commands/tablecmds.c
*** head/src/backend/commands/tablecmds.c 2009-10-15 10:26:48.573966000 +0900
--- work/src/backend/commands/tablecmds.c 2009-10-15 13:38:54.291174547 +0900
*************** CreateFKCheckTrigger(RangeVar *myRel, Co
*** 5448,5454 ****
fk_trigger->constrrel = fkconstraint->pktable;
fk_trigger->args = NIL;
! (void) CreateTrigger(fk_trigger, constraintOid, indexOid,
"RI_ConstraintTrigger", false);
/* Make changes-so-far visible */
--- 5448,5454 ----
fk_trigger->constrrel = fkconstraint->pktable;
fk_trigger->args = NIL;
! (void) CreateTrigger(fk_trigger, NULL, constraintOid, indexOid,
"RI_ConstraintTrigger", false);
/* Make changes-so-far visible */
*************** createForeignKeyTriggers(Relation rel, C
*** 5489,5494 ****
--- 5489,5495 ----
fk_trigger = makeNode(CreateTrigStmt);
fk_trigger->trigname = fkconstraint->conname;
fk_trigger->relation = fkconstraint->pktable;
+ fk_trigger->whenClause = NULL;
fk_trigger->before = false;
fk_trigger->row = true;
fk_trigger->events = TRIGGER_TYPE_DELETE;
*************** createForeignKeyTriggers(Relation rel, C
*** 5529,5535 ****
}
fk_trigger->args = NIL;
! (void) CreateTrigger(fk_trigger, constraintOid, indexOid,
"RI_ConstraintTrigger", false);
/* Make changes-so-far visible */
--- 5530,5536 ----
}
fk_trigger->args = NIL;
! (void) CreateTrigger(fk_trigger, NULL, constraintOid, indexOid,
"RI_ConstraintTrigger", false);
/* Make changes-so-far visible */
*************** createForeignKeyTriggers(Relation rel, C
*** 5542,5547 ****
--- 5543,5549 ----
fk_trigger = makeNode(CreateTrigStmt);
fk_trigger->trigname = fkconstraint->conname;
fk_trigger->relation = fkconstraint->pktable;
+ fk_trigger->whenClause = NULL;
fk_trigger->before = false;
fk_trigger->row = true;
fk_trigger->events = TRIGGER_TYPE_UPDATE;
*************** createForeignKeyTriggers(Relation rel, C
*** 5582,5588 ****
}
fk_trigger->args = NIL;
! (void) CreateTrigger(fk_trigger, constraintOid, indexOid,
"RI_ConstraintTrigger", false);
}
--- 5584,5590 ----
}
fk_trigger->args = NIL;
! (void) CreateTrigger(fk_trigger, NULL, constraintOid, indexOid,
"RI_ConstraintTrigger", false);
}
diff -cprN head/src/backend/commands/trigger.c work/src/backend/commands/trigger.c
*** head/src/backend/commands/trigger.c 2009-10-15 10:26:48.573966000 +0900
--- work/src/backend/commands/trigger.c 2009-10-15 16:05:22.628352863 +0900
***************
*** 32,41 ****
--- 32,43 ----
#include "miscadmin.h"
#include "nodes/bitmapset.h"
#include "nodes/makefuncs.h"
+ #include "parser/parse_clause.h"
#include "parser/parse_func.h"
#include "parser/parse_relation.h"
#include "parser/parsetree.h"
#include "pgstat.h"
+ #include "rewrite/rewriteManip.h"
#include "storage/bufmgr.h"
#include "tcop/utility.h"
#include "utils/acl.h"
*************** static HeapTuple GetTupleForTrigger(ESta
*** 66,78 ****
ItemPointer tid,
TupleTableSlot **newSlot);
static bool TriggerEnabled(Trigger *trigger, TriggerEvent event,
! Bitmapset *modifiedCols);
static HeapTuple ExecCallTriggerFunc(TriggerData *trigdata,
int tgindx,
FmgrInfo *finfo,
Instrumentation *instr,
MemoryContext per_tuple_context);
! static void AfterTriggerSaveEvent(ResultRelInfo *relinfo, int event,
bool row_trigger, HeapTuple oldtup, HeapTuple newtup,
List *recheckIndexes, Bitmapset *modifiedCols);
--- 68,81 ----
ItemPointer tid,
TupleTableSlot **newSlot);
static bool TriggerEnabled(Trigger *trigger, TriggerEvent event,
! Bitmapset *modifiedCols, EState *estate,
! TupleDesc tupdesc, HeapTuple oldtup, HeapTuple newtup);
static HeapTuple ExecCallTriggerFunc(TriggerData *trigdata,
int tgindx,
FmgrInfo *finfo,
Instrumentation *instr,
MemoryContext per_tuple_context);
! static void AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, int event,
bool row_trigger, HeapTuple oldtup, HeapTuple newtup,
List *recheckIndexes, Bitmapset *modifiedCols);
*************** static void AfterTriggerSaveEvent(Result
*** 101,107 ****
* but a foreign-key constraint. This is a kluge for backwards compatibility.
*/
Oid
! CreateTrigger(CreateTrigStmt *stmt,
Oid constraintOid, Oid indexOid, const char *prefix,
bool checkPermissions)
{
--- 104,110 ----
* but a foreign-key constraint. This is a kluge for backwards compatibility.
*/
Oid
! CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
Oid constraintOid, Oid indexOid, const char *prefix,
bool checkPermissions)
{
*************** CreateTrigger(CreateTrigStmt *stmt,
*** 128,133 ****
--- 131,137 ----
Oid constrrelid = InvalidOid;
ObjectAddress myself,
referenced;
+ char *qual;
rel = heap_openrv(stmt->relation, AccessExclusiveLock);
*************** CreateTrigger(CreateTrigStmt *stmt,
*** 224,229 ****
--- 228,298 ----
return InvalidOid;
}
+ if (stmt->whenClause == NULL)
+ qual = NULL;
+ else
+ {
+ ParseState *pstate;
+ Node *whenClause;
+ int num_rte;
+
+ /* Set up pstate */
+ pstate = make_parsestate(NULL);
+ pstate->p_sourcetext = queryString;
+
+ /*
+ * NOTE: 'OLD' must always have a varno equal to 1 and 'NEW' equal to 2.
+ * Set up their RTEs in the main pstate for use in parsing the rule
+ * qualification.
+ */
+ 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);
+ }
+ }
+ num_rte = list_length(pstate->p_rtable);
+
+ /* take care of the when clause */
+ whenClause = transformWhereClause(pstate,
+ (Node *) copyObject(stmt->whenClause), "WHEN");
+
+ if (list_length(pstate->p_rtable) != num_rte)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("trigger WHEN condition cannot contain references to other relations")));
+
+ /* aggregates not allowed (but subselects are okay) */
+ if (pstate->p_hasAggs)
+ ereport(ERROR,
+ (errcode(ERRCODE_GROUPING_ERROR),
+ errmsg("cannot use aggregate function in trigger WHEN condition")));
+ if (pstate->p_hasWindowFuncs)
+ ereport(ERROR,
+ (errcode(ERRCODE_WINDOWING_ERROR),
+ errmsg("cannot use window function in trigger WHEN condition")));
+
+ free_parsestate(pstate);
+
+ qual = nodeToString(whenClause);
+ }
+
/*
* Generate the trigger's OID now, so that we can use it in the name if
* needed.
*************** CreateTrigger(CreateTrigStmt *stmt,
*** 387,392 ****
--- 456,472 ----
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);
/* force tuple to have the desired OID */
*************** RelationBuildTriggers(Relation relation)
*** 1184,1189 ****
--- 1264,1271 ----
{
Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(htup);
Trigger *build;
+ Datum qual_datum;
+ bool isnull;
if (numtrigs >= maxtrigs)
{
*************** RelationBuildTriggers(Relation relation)
*** 1238,1243 ****
--- 1320,1341 ----
else
build->tgargs = NULL;
+ qual_datum = heap_getattr(htup, Anum_pg_trigger_tgqual,
+ RelationGetDescr(tgrel), &isnull);
+ if (!isnull)
+ {
+ char *qual_str = TextDatumGetCString(qual_datum);
+
+ /* Fix references to OLD and NEW to INNER and OUTER */
+ build->tgqual = stringToNode(qual_str);
+ ChangeVarNodes(build->tgqual, PRS2_OLD_VARNO, INNER, 0);
+ ChangeVarNodes(build->tgqual, PRS2_NEW_VARNO, OUTER, 0);
+
+ pfree(qual_str);
+ }
+ else
+ build->tgqual = NULL;
+
numtrigs++;
}
*************** CopyTriggerDesc(TriggerDesc *trigdesc)
*** 1396,1401 ****
--- 1494,1500 ----
newargs[j] = pstrdup(trigger->tgargs[j]);
trigger->tgargs = newargs;
}
+ trigger->tgqual = copyObject(trigger->tgqual);
trigger++;
}
*************** ExecBSInsertTriggers(EState *estate, Res
*** 1687,1693 ****
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
HeapTuple newtuple;
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL))
continue;
LocTriggerData.tg_trigger = trigger;
--- 1786,1793 ----
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
HeapTuple newtuple;
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL,
! NULL, NULL, NULL, NULL))
continue;
LocTriggerData.tg_trigger = trigger;
*************** ExecASInsertTriggers(EState *estate, Res
*** 1710,1716 ****
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_statement[TRIGGER_EVENT_INSERT] > 0)
! AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_INSERT,
false, NULL, NULL, NIL, NULL);
}
--- 1810,1816 ----
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_statement[TRIGGER_EVENT_INSERT] > 0)
! AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_INSERT,
false, NULL, NULL, NIL, NULL);
}
*************** ExecBRInsertTriggers(EState *estate, Res
*** 1737,1743 ****
{
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL))
continue;
LocTriggerData.tg_trigtuple = oldtuple = newtuple;
--- 1837,1845 ----
{
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL,
! estate, RelationGetDescr(relinfo->ri_RelationDesc),
! NULL, newtuple))
continue;
LocTriggerData.tg_trigtuple = oldtuple = newtuple;
*************** ExecARInsertTriggers(EState *estate, Res
*** 1763,1769 ****
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_row[TRIGGER_EVENT_INSERT] > 0)
! AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_INSERT,
true, NULL, trigtuple, recheckIndexes, NULL);
}
--- 1865,1871 ----
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_row[TRIGGER_EVENT_INSERT] > 0)
! AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_INSERT,
true, NULL, trigtuple, recheckIndexes, NULL);
}
*************** ExecBSDeleteTriggers(EState *estate, Res
*** 1800,1806 ****
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
HeapTuple newtuple;
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL))
continue;
LocTriggerData.tg_trigger = trigger;
--- 1902,1909 ----
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
HeapTuple newtuple;
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL,
! NULL, NULL, NULL, NULL))
continue;
LocTriggerData.tg_trigger = trigger;
*************** ExecASDeleteTriggers(EState *estate, Res
*** 1823,1829 ****
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_statement[TRIGGER_EVENT_DELETE] > 0)
! AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_DELETE,
false, NULL, NULL, NIL, NULL);
}
--- 1926,1932 ----
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_statement[TRIGGER_EVENT_DELETE] > 0)
! AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_DELETE,
false, NULL, NULL, NIL, NULL);
}
*************** ExecBRDeleteTriggers(EState *estate, Pla
*** 1858,1864 ****
{
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL))
continue;
LocTriggerData.tg_trigtuple = trigtuple;
--- 1961,1969 ----
{
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL,
! estate, RelationGetDescr(relinfo->ri_RelationDesc),
! trigtuple, NULL))
continue;
LocTriggerData.tg_trigtuple = trigtuple;
*************** ExecARDeleteTriggers(EState *estate, Res
*** 1893,1899 ****
HeapTuple trigtuple = GetTupleForTrigger(estate, NULL, relinfo,
tupleid, NULL);
! AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_DELETE,
true, trigtuple, NULL, NIL, NULL);
heap_freetuple(trigtuple);
}
--- 1998,2004 ----
HeapTuple trigtuple = GetTupleForTrigger(estate, NULL, relinfo,
tupleid, NULL);
! AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_DELETE,
true, trigtuple, NULL, NIL, NULL);
heap_freetuple(trigtuple);
}
*************** ExecBSUpdateTriggers(EState *estate, Res
*** 1935,1941 ****
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
HeapTuple newtuple;
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, modifiedCols))
continue;
LocTriggerData.tg_trigger = trigger;
--- 2040,2047 ----
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
HeapTuple newtuple;
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, modifiedCols,
! NULL, NULL, NULL, NULL))
continue;
LocTriggerData.tg_trigger = trigger;
*************** ExecASUpdateTriggers(EState *estate, Res
*** 1958,1964 ****
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_statement[TRIGGER_EVENT_UPDATE] > 0)
! AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_UPDATE,
false, NULL, NULL, NIL,
GetModifiedColumns(relinfo, estate));
}
--- 2064,2070 ----
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_statement[TRIGGER_EVENT_UPDATE] > 0)
! AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_UPDATE,
false, NULL, NULL, NIL,
GetModifiedColumns(relinfo, estate));
}
*************** ExecBRUpdateTriggers(EState *estate, Pla
*** 2003,2009 ****
{
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, modifiedCols))
continue;
LocTriggerData.tg_trigtuple = trigtuple;
--- 2109,2117 ----
{
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, modifiedCols,
! estate, RelationGetDescr(relinfo->ri_RelationDesc),
! trigtuple, newtuple))
continue;
LocTriggerData.tg_trigtuple = trigtuple;
*************** ExecARUpdateTriggers(EState *estate, Res
*** 2037,2043 ****
HeapTuple trigtuple = GetTupleForTrigger(estate, NULL, relinfo,
tupleid, NULL);
! AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_UPDATE,
true, trigtuple, newtuple, recheckIndexes,
GetModifiedColumns(relinfo, estate));
heap_freetuple(trigtuple);
--- 2145,2151 ----
HeapTuple trigtuple = GetTupleForTrigger(estate, NULL, relinfo,
tupleid, NULL);
! AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_UPDATE,
true, trigtuple, newtuple, recheckIndexes,
GetModifiedColumns(relinfo, estate));
heap_freetuple(trigtuple);
*************** ExecBSTruncateTriggers(EState *estate, R
*** 2077,2083 ****
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
HeapTuple newtuple;
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL))
continue;
LocTriggerData.tg_trigger = trigger;
--- 2185,2192 ----
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
HeapTuple newtuple;
! if (!TriggerEnabled(trigger, LocTriggerData.tg_event, NULL,
! NULL, NULL, NULL, NULL))
continue;
LocTriggerData.tg_trigger = trigger;
*************** ExecASTruncateTriggers(EState *estate, R
*** 2100,2106 ****
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_statement[TRIGGER_EVENT_TRUNCATE] > 0)
! AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_TRUNCATE,
false, NULL, NULL, NIL, NULL);
}
--- 2209,2215 ----
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
if (trigdesc && trigdesc->n_after_statement[TRIGGER_EVENT_TRUNCATE] > 0)
! AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_TRUNCATE,
false, NULL, NULL, NIL, NULL);
}
*************** ltrmark:;
*** 2211,2217 ****
* Is trigger enabled to fire?
*/
static bool
! TriggerEnabled(Trigger *trigger, TriggerEvent event, Bitmapset *modifiedCols)
{
/* Check replication-role-dependent enable state */
if (SessionReplicationRole == SESSION_REPLICATION_ROLE_REPLICA)
--- 2320,2330 ----
* Is trigger enabled to fire?
*/
static bool
! TriggerEnabled(Trigger *trigger, TriggerEvent event, Bitmapset *modifiedCols,
! EState *estate,
! TupleDesc tupdesc,
! HeapTuple oldtup,
! HeapTuple newtup)
{
/* Check replication-role-dependent enable state */
if (SessionReplicationRole == SESSION_REPLICATION_ROLE_REPLICA)
*************** TriggerEnabled(Trigger *trigger, Trigger
*** 2250,2255 ****
--- 2363,2412 ----
return false;
}
+ /* Check for WHEN clause */
+ if (trigger->tgqual)
+ {
+ ExprContext *econtext;
+ List *predicate;
+ TupleTableSlot *oldslot = NULL;
+ TupleTableSlot *newslot = NULL;
+ bool ret;
+
+ econtext = GetPerTupleExprContext(estate);
+
+ predicate = list_make1(ExecPrepareExpr((Expr *) trigger->tgqual, estate));
+
+ /* set OLD and NEW tuples into tupleslots */
+ if (TRIGGER_FIRED_FOR_ROW(event))
+ {
+ if (TRIGGER_FIRED_BY_DELETE(event) || TRIGGER_FIRED_BY_UPDATE(event))
+ {
+ Assert(oldtup != NULL);
+ oldslot = MakeSingleTupleTableSlot(tupdesc);
+ ExecStoreTuple(oldtup, oldslot, InvalidBuffer, false);
+ }
+ if (TRIGGER_FIRED_BY_INSERT(event) || TRIGGER_FIRED_BY_UPDATE(event))
+ {
+ Assert(newtup != NULL);
+ newslot = MakeSingleTupleTableSlot(tupdesc);
+ ExecStoreTuple(newtup, newslot, InvalidBuffer, false);
+ }
+ }
+
+ econtext->ecxt_innertuple = oldslot;
+ econtext->ecxt_outertuple = newslot;
+ ret = ExecQual(predicate, econtext, false);
+
+ /* free tableslots */
+ if (oldslot)
+ ExecDropSingleTupleTableSlot(oldslot);
+ if (newslot)
+ ExecDropSingleTupleTableSlot(newslot);
+
+ if (!ret)
+ return false;
+ }
+
return true;
}
*************** AfterTriggerPendingOnRel(Oid relid)
*** 3875,3881 ****
* ----------
*/
static void
! AfterTriggerSaveEvent(ResultRelInfo *relinfo, int event, bool row_trigger,
HeapTuple oldtup, HeapTuple newtup,
List *recheckIndexes, Bitmapset *modifiedCols)
{
--- 4032,4038 ----
* ----------
*/
static void
! AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, int event, bool row_trigger,
HeapTuple oldtup, HeapTuple newtup,
List *recheckIndexes, Bitmapset *modifiedCols)
{
*************** AfterTriggerSaveEvent(ResultRelInfo *rel
*** 3979,3985 ****
{
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
! if (!TriggerEnabled(trigger, event, modifiedCols))
continue;
/*
--- 4136,4144 ----
{
Trigger *trigger = &trigdesc->triggers[tgindx[i]];
! if (!TriggerEnabled(trigger, event, modifiedCols,
! estate, RelationGetDescr(relinfo->ri_RelationDesc),
! oldtup, newtup))
continue;
/*
diff -cprN head/src/backend/executor/execQual.c work/src/backend/executor/execQual.c
*** head/src/backend/executor/execQual.c 2009-10-13 09:24:03.097662000 +0900
--- work/src/backend/executor/execQual.c 2009-10-15 13:38:54.296151658 +0900
*************** static Datum ExecEvalArrayCoerceExpr(Arr
*** 170,175 ****
--- 170,176 ----
bool *isNull, ExprDoneCond *isDone);
static Datum ExecEvalCurrentOfExpr(ExprState *exprstate, ExprContext *econtext,
bool *isNull, ExprDoneCond *isDone);
+ static TupleTableSlot *get_slot(ExprContext *econtext, Index varno);
/* ----------------------------------------------------------------
*************** ExecEvalVar(ExprState *exprstate, ExprCo
*** 501,523 ****
*/
attnum = variable->varattno;
! switch (variable->varno)
! {
! case INNER: /* get the tuple from the inner node */
! slot = econtext->ecxt_innertuple;
! Assert(attnum > 0);
! break;
!
! case OUTER: /* get the tuple from the outer node */
! slot = econtext->ecxt_outertuple;
! Assert(attnum > 0);
! break;
!
! default: /* get the tuple from the relation being
! * scanned */
! slot = econtext->ecxt_scantuple;
! break;
! }
if (attnum != InvalidAttrNumber)
{
--- 502,508 ----
*/
attnum = variable->varattno;
! slot = get_slot(econtext, variable->varno);
if (attnum != InvalidAttrNumber)
{
*************** ExecEvalScalarVar(ExprState *exprstate,
*** 682,702 ****
*isDone = ExprSingleResult;
/* Get the input slot and attribute number we want */
! switch (variable->varno)
! {
! case INNER: /* get the tuple from the inner node */
! slot = econtext->ecxt_innertuple;
! break;
!
! case OUTER: /* get the tuple from the outer node */
! slot = econtext->ecxt_outertuple;
! break;
!
! default: /* get the tuple from the relation being
! * scanned */
! slot = econtext->ecxt_scantuple;
! break;
! }
attnum = variable->varattno;
--- 667,673 ----
*isDone = ExprSingleResult;
/* Get the input slot and attribute number we want */
! slot = get_slot(econtext, variable->varno);
attnum = variable->varattno;
*************** ExecEvalWholeRowVar(ExprState *exprstate
*** 715,721 ****
bool *isNull, ExprDoneCond *isDone)
{
Var *variable = (Var *) exprstate->expr;
! TupleTableSlot *slot = econtext->ecxt_scantuple;
HeapTuple tuple;
TupleDesc tupleDesc;
HeapTupleHeader dtuple;
--- 686,692 ----
bool *isNull, ExprDoneCond *isDone)
{
Var *variable = (Var *) exprstate->expr;
! TupleTableSlot *slot = get_slot(econtext, variable->varno);
HeapTuple tuple;
TupleDesc tupleDesc;
HeapTupleHeader dtuple;
*************** ExecEvalWholeRowSlow(ExprState *exprstat
*** 766,772 ****
bool *isNull, ExprDoneCond *isDone)
{
Var *variable = (Var *) exprstate->expr;
! TupleTableSlot *slot = econtext->ecxt_scantuple;
HeapTuple tuple;
TupleDesc var_tupdesc;
HeapTupleHeader dtuple;
--- 737,743 ----
bool *isNull, ExprDoneCond *isDone)
{
Var *variable = (Var *) exprstate->expr;
! TupleTableSlot *slot = get_slot(econtext, variable->varno);
HeapTuple tuple;
TupleDesc var_tupdesc;
HeapTupleHeader dtuple;
*************** ExecProject(ProjectionInfo *projInfo, Ex
*** 5167,5169 ****
--- 5138,5154 ----
*/
return ExecStoreVirtualTuple(slot);
}
+
+ static TupleTableSlot *
+ get_slot(ExprContext *econtext, Index varno)
+ {
+ switch (varno)
+ {
+ case INNER: /* get the tuple from the inner node */
+ return econtext->ecxt_innertuple;
+ case OUTER: /* get the tuple from the outer node */
+ return econtext->ecxt_outertuple;
+ default: /* get the tuple from the relation being scanned */
+ return econtext->ecxt_scantuple;
+ }
+ }
diff -cprN head/src/backend/nodes/copyfuncs.c work/src/backend/nodes/copyfuncs.c
*** head/src/backend/nodes/copyfuncs.c 2009-10-15 10:26:48.573966000 +0900
--- work/src/backend/nodes/copyfuncs.c 2009-10-15 13:44:59.865128220 +0900
*************** _copyCreateTrigStmt(CreateTrigStmt *from
*** 3161,3166 ****
--- 3161,3167 ----
COPY_SCALAR_FIELD(row);
COPY_SCALAR_FIELD(events);
COPY_NODE_FIELD(columns);
+ COPY_NODE_FIELD(whenClause);
COPY_SCALAR_FIELD(isconstraint);
COPY_SCALAR_FIELD(deferrable);
COPY_SCALAR_FIELD(initdeferred);
diff -cprN head/src/backend/nodes/equalfuncs.c work/src/backend/nodes/equalfuncs.c
*** head/src/backend/nodes/equalfuncs.c 2009-10-15 10:26:48.573966000 +0900
--- work/src/backend/nodes/equalfuncs.c 2009-10-15 13:45:28.142119601 +0900
*************** _equalCreateTrigStmt(CreateTrigStmt *a,
*** 1672,1677 ****
--- 1672,1678 ----
COMPARE_SCALAR_FIELD(row);
COMPARE_SCALAR_FIELD(events);
COMPARE_NODE_FIELD(columns);
+ COMPARE_NODE_FIELD(whenClause);
COMPARE_SCALAR_FIELD(isconstraint);
COMPARE_SCALAR_FIELD(deferrable);
COMPARE_SCALAR_FIELD(initdeferred);
diff -cprN head/src/backend/parser/gram.y work/src/backend/parser/gram.y
*** head/src/backend/parser/gram.y 2009-10-15 10:26:48.573966000 +0900
--- work/src/backend/parser/gram.y 2009-10-15 13:38:54.299158534 +0900
*************** static TypeName *TableFuncTypeName(List
*** 253,258 ****
--- 253,259 ----
%type <list> TriggerEvents TriggerOneEvent
%type <value> TriggerFuncArg
+ %type <node> TriggerWhen
%type <str> relation_name copy_file_name
database_name access_method_clause access_method attr_name
*************** AlterUserMappingStmt: ALTER USER MAPPING
*** 3230,3243 ****
CreateTrigStmt:
CREATE TRIGGER name TriggerActionTime TriggerEvents ON
! qualified_name TriggerForSpec EXECUTE PROCEDURE
! func_name '(' TriggerFuncArgs ')'
{
CreateTrigStmt *n = makeNode(CreateTrigStmt);
n->trigname = $3;
n->relation = $7;
! n->funcname = $11;
! n->args = $13;
n->before = $4;
n->row = $8;
n->events = intVal(linitial($5));
--- 3231,3246 ----
CreateTrigStmt:
CREATE TRIGGER name TriggerActionTime TriggerEvents ON
! qualified_name TriggerForSpec
! { pg_yyget_extra(yyscanner)->QueryIsRule = TRUE; }
! TriggerWhen EXECUTE PROCEDURE func_name '(' TriggerFuncArgs ')'
{
CreateTrigStmt *n = makeNode(CreateTrigStmt);
n->trigname = $3;
n->relation = $7;
! n->whenClause = $10;
! n->funcname = $13;
! n->args = $15;
n->before = $4;
n->row = $8;
n->events = intVal(linitial($5));
*************** CreateTrigStmt:
*** 3247,3252 ****
--- 3250,3256 ----
n->initdeferred = FALSE;
n->constrrel = NULL;
$$ = (Node *)n;
+ pg_yyget_extra(yyscanner)->QueryIsRule = FALSE;
}
| CREATE CONSTRAINT TRIGGER name AFTER TriggerEvents ON
qualified_name OptConstrFromTable
*************** TriggerFuncArg:
*** 3358,3363 ****
--- 3362,3372 ----
| ColId { $$ = makeString($1); }
;
+ TriggerWhen:
+ WHEN '(' a_expr ')' { $$ = $3; }
+ | /*EMPTY*/ { $$ = NULL; }
+ ;
+
OptConstrFromTable:
FROM qualified_name { $$ = $2; }
| /*EMPTY*/ { $$ = NULL; }
diff -cprN head/src/backend/tcop/utility.c work/src/backend/tcop/utility.c
*** head/src/backend/tcop/utility.c 2009-10-06 16:37:35.607642000 +0900
--- work/src/backend/tcop/utility.c 2009-10-15 13:38:54.300180389 +0900
*************** ProcessUtility(Node *parsetree,
*** 939,945 ****
break;
case T_CreateTrigStmt:
! CreateTrigger((CreateTrigStmt *) parsetree,
InvalidOid, InvalidOid, NULL, true);
break;
--- 939,945 ----
break;
case T_CreateTrigStmt:
! CreateTrigger((CreateTrigStmt *) parsetree, queryString,
InvalidOid, InvalidOid, NULL, true);
break;
diff -cprN head/src/backend/utils/adt/ruleutils.c work/src/backend/utils/adt/ruleutils.c
*** head/src/backend/utils/adt/ruleutils.c 2009-10-15 10:26:48.573966000 +0900
--- work/src/backend/utils/adt/ruleutils.c 2009-10-15 14:39:24.286148874 +0900
***************
*** 42,47 ****
--- 42,48 ----
#include "parser/keywords.h"
#include "parser/parse_func.h"
#include "parser/parse_oper.h"
+ #include "parser/parse_relation.h"
#include "parser/parser.h"
#include "parser/parsetree.h"
#include "rewrite/rewriteHandler.h"
*************** pg_get_triggerdef_worker(Oid trigid, boo
*** 487,492 ****
--- 488,495 ----
SysScanDesc tgscan;
int findx = 0;
char *tgname;
+ Datum value;
+ bool isnull;
/*
* Fetch the pg_trigger tuple by the Oid of the trigger
*************** pg_get_triggerdef_worker(Oid trigid, boo
*** 600,618 ****
generate_function_name(trigrec->tgfoid, 0,
NIL, NULL, NULL));
if (trigrec->tgnargs > 0)
{
- bytea *val;
- bool isnull;
char *p;
int i;
! val = DatumGetByteaP(fastgetattr(ht_trig,
! Anum_pg_trigger_tgargs,
! tgrel->rd_att, &isnull));
if (isnull)
elog(ERROR, "tgargs is null for trigger %u", trigid);
! p = (char *) VARDATA(val);
for (i = 0; i < trigrec->tgnargs; i++)
{
if (i > 0)
--- 603,663 ----
generate_function_name(trigrec->tgfoid, 0,
NIL, NULL, NULL));
+ /* If the trigger has an event qualification, add it */
+ value = fastgetattr(ht_trig, Anum_pg_trigger_tgqual,
+ tgrel->rd_att, &isnull);
+ if (!isnull)
+ {
+ Node *qual;
+ deparse_context context;
+ deparse_namespace dpns;
+ RangeTblEntry *oldrte;
+ RangeTblEntry *newrte;
+
+ appendStringInfo(&buf, "WHEN (");
+
+ qual = stringToNode(TextDatumGetCString(value));
+
+ context.buf = &buf;
+ context.namespaces = list_make1(&dpns);
+ context.windowClause = NIL;
+ context.windowTList = NIL;
+ context.varprefix = true;
+ context.prettyFlags = PRETTYFLAG_PAREN | (pretty ? PRETTYFLAG_INDENT : 0);
+ context.indentLevel = PRETTYINDENT_STD;
+
+ /* Build a minimal NEW and OLD RTEs for the rel */
+ oldrte = makeNode(RangeTblEntry);
+ oldrte->rtekind = RTE_RELATION;
+ oldrte->alias = oldrte->eref = makeAlias("*OLD*", NIL);
+ oldrte->relid = trigrec->tgrelid;
+
+ newrte = makeNode(RangeTblEntry);
+ newrte->rtekind = RTE_RELATION;
+ newrte->alias = newrte->eref = makeAlias("*NEW*", NIL);
+ newrte->relid = trigrec->tgrelid;
+
+ /* Build two-elements rtable */
+ dpns.rtable = list_make2(oldrte, newrte);
+ dpns.ctes = NIL;
+ dpns.subplans = NIL;
+ dpns.outer_plan = dpns.inner_plan = NULL;
+
+ get_rule_expr(qual, &context, false);
+
+ appendStringInfo(&buf, ")%s", pretty ? "\n " : " ");
+ }
+
if (trigrec->tgnargs > 0)
{
char *p;
int i;
! value = fastgetattr(ht_trig, Anum_pg_trigger_tgargs,
! tgrel->rd_att, &isnull);
if (isnull)
elog(ERROR, "tgargs is null for trigger %u", trigid);
! p = (char *) VARDATA(DatumGetByteaP(value));
for (i = 0; i < trigrec->tgnargs; i++)
{
if (i > 0)
diff -cprN head/src/include/catalog/pg_trigger.h work/src/include/catalog/pg_trigger.h
*** head/src/include/catalog/pg_trigger.h 2009-10-15 10:26:48.573966000 +0900
--- work/src/include/catalog/pg_trigger.h 2009-10-15 13:38:54.301169692 +0900
*************** CATALOG(pg_trigger,2620)
*** 55,60 ****
--- 55,61 ----
/* VARIABLE LENGTH FIELDS (note: these are not supposed to be null) */
int2vector tgattr; /* column numbers, if trigger is on columns */
bytea tgargs; /* first\000second\000tgnargs\000 */
+ text tgqual; /* WHEN clause */
} FormData_pg_trigger;
/* ----------------
*************** typedef FormData_pg_trigger *Form_pg_tri
*** 68,74 ****
* compiler constants for pg_trigger
* ----------------
*/
! #define Natts_pg_trigger 15
#define Anum_pg_trigger_tgrelid 1
#define Anum_pg_trigger_tgname 2
#define Anum_pg_trigger_tgfoid 3
--- 69,75 ----
* compiler constants for pg_trigger
* ----------------
*/
! #define Natts_pg_trigger 16
#define Anum_pg_trigger_tgrelid 1
#define Anum_pg_trigger_tgname 2
#define Anum_pg_trigger_tgfoid 3
*************** typedef FormData_pg_trigger *Form_pg_tri
*** 84,89 ****
--- 85,91 ----
#define Anum_pg_trigger_tgnargs 13
#define Anum_pg_trigger_tgattr 14
#define Anum_pg_trigger_tgargs 15
+ #define Anum_pg_trigger_tgqual 16
/* Bits within tgtype */
#define TRIGGER_TYPE_ROW (1 << 0)
diff -cprN head/src/include/commands/trigger.h work/src/include/commands/trigger.h
*** head/src/include/commands/trigger.h 2009-10-13 09:24:03.097662000 +0900
--- work/src/include/commands/trigger.h 2009-10-15 13:38:54.302152509 +0900
*************** extern PGDLLIMPORT int SessionReplicatio
*** 104,110 ****
#define TRIGGER_FIRES_ON_REPLICA 'R'
#define TRIGGER_DISABLED 'D'
! extern Oid CreateTrigger(CreateTrigStmt *stmt,
Oid constraintOid, Oid indexOid, const char *prefix,
bool checkPermissions);
--- 104,110 ----
#define TRIGGER_FIRES_ON_REPLICA 'R'
#define TRIGGER_DISABLED 'D'
! extern Oid CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
Oid constraintOid, Oid indexOid, const char *prefix,
bool checkPermissions);
diff -cprN head/src/include/nodes/parsenodes.h work/src/include/nodes/parsenodes.h
*** head/src/include/nodes/parsenodes.h 2009-10-15 10:26:48.573966000 +0900
--- work/src/include/nodes/parsenodes.h 2009-10-15 13:38:54.302152509 +0900
*************** typedef struct CreateTrigStmt
*** 1575,1580 ****
--- 1575,1581 ----
/* events uses the TRIGGER_TYPE bits defined in catalog/pg_trigger.h */
int16 events; /* INSERT/UPDATE/DELETE/TRUNCATE */
List *columns; /* column names, or NIL for all columns */
+ Node *whenClause; /* qualifications */
/* The following are used for constraint triggers (RI and unique checks) */
bool isconstraint; /* This is a constraint trigger */
diff -cprN head/src/include/utils/rel.h work/src/include/utils/rel.h
*** head/src/include/utils/rel.h 2009-08-03 10:58:29.885713000 +0900
--- work/src/include/utils/rel.h 2009-10-15 13:38:54.302152509 +0900
*************** typedef struct Trigger
*** 66,71 ****
--- 66,72 ----
int16 tgnattr;
int16 *tgattr;
char **tgargs;
+ Node *tgqual;
} Trigger;
typedef struct TriggerDesc
diff -cprN head/src/test/regress/expected/triggers.out work/src/test/regress/expected/triggers.out
*** head/src/test/regress/expected/triggers.out 2009-10-15 10:26:48.573966000 +0900
--- work/src/test/regress/expected/triggers.out 2009-10-15 16:09:21.886355469 +0900
*************** SELECT * FROM main_table ORDER BY a, b;
*** 322,327 ****
--- 322,377 ----
|
(8 rows)
+ -- Trigger with WHEN clause
+ CREATE TRIGGER modified_a BEFORE UPDATE OF a ON main_table
+ FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE trigger_func('modified_a');
+ CREATE TRIGGER modified_any BEFORE UPDATE OF a ON main_table
+ FOR EACH ROW WHEN (OLD.* IS DISTINCT FROM NEW.*) EXECUTE PROCEDURE trigger_func('modified_any');
+ UPDATE main_table SET a = 50, b = 60;
+ NOTICE: trigger_func(modified_any) called: action = UPDATE, when = BEFORE, level = ROW
+ NOTICE: trigger_func(modified_any) called: action = UPDATE, when = BEFORE, level = ROW
+ NOTICE: trigger_func(modified_a) called: action = UPDATE, when = BEFORE, level = ROW
+ NOTICE: trigger_func(modified_a) called: action = UPDATE, when = BEFORE, level = ROW
+ NOTICE: trigger_func(modified_a) called: action = UPDATE, when = BEFORE, level = ROW
+ NOTICE: trigger_func(modified_a) called: action = UPDATE, when = BEFORE, level = ROW
+ NOTICE: trigger_func(modified_a) called: action = UPDATE, when = BEFORE, level = ROW
+ NOTICE: trigger_func(after_upd_row) called: action = UPDATE, when = AFTER, level = ROW
+ NOTICE: trigger_func(after_upd_stmt) called: action = UPDATE, when = AFTER, level = STATEMENT
+ SELECT * FROM main_table ORDER BY a, b;
+ a | b
+ ----+----
+ 6 | 10
+ 21 | 20
+ 30 | 40
+ 31 | 10
+ 50 | 35
+ 50 | 60
+ 81 | 15
+ |
+ (8 rows)
+
+ SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_a';
+ pg_get_triggerdef
+ ----------------------------------------------------------
+ CREATE TRIGGER modified_a
+ BEFORE UPDATE OF a ON main_table
+ FOR EACH ROW
+ EXECUTE PROCEDURE trigger_func(WHEN (old.a <> new.a)
+ 'modified_a')
+ (1 row)
+
+ SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_any';
+ pg_get_triggerdef
+ ------------------------------------------------------------------------
+ CREATE TRIGGER modified_any
+ BEFORE UPDATE OF a ON main_table
+ FOR EACH ROW
+ EXECUTE PROCEDURE trigger_func(WHEN (old.* IS DISTINCT FROM new.*)
+ 'modified_any')
+ (1 row)
+
+ DROP TRIGGER modified_a ON main_table;
+ DROP TRIGGER modified_any ON main_table;
-- Test column-level triggers
DROP TRIGGER after_upd_row_trig ON main_table;
CREATE TRIGGER before_upd_a_row_trig BEFORE UPDATE OF a ON main_table
*************** FOR EACH ROW EXECUTE PROCEDURE trigger_f
*** 393,398 ****
--- 443,463 ----
ERROR: syntax error at or near "OF"
LINE 1: CREATE TRIGGER error_ins_a BEFORE INSERT OF a ON main_table
^
+ CREATE TRIGGER error_ins_when BEFORE INSERT OR UPDATE ON main_table
+ FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE trigger_func('error_ins_when');
+ ERROR: missing FROM-clause entry for table "*OLD*"
+ LINE 2: FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE trigger...
+ ^
+ CREATE TRIGGER error_del_when BEFORE DELETE OR UPDATE ON main_table
+ FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE trigger_func('error_del_when');
+ ERROR: missing FROM-clause entry for table "*NEW*"
+ LINE 2: FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE trigger...
+ ^
+ CREATE TRIGGER error_stmt_when BEFORE UPDATE OF a ON main_table
+ FOR EACH STATEMENT WHEN (OLD.* IS DISTINCT FROM NEW.*) EXECUTE PROCEDURE trigger_func('error_stmt_when');
+ ERROR: missing FROM-clause entry for table "*OLD*"
+ LINE 2: FOR EACH STATEMENT WHEN (OLD.* IS DISTINCT FROM NEW.*) EXECU...
+ ^
-- check dependency restrictions
ALTER TABLE main_table DROP COLUMN b;
ERROR: cannot drop table main_table column b because other objects depend on it
diff -cprN head/src/test/regress/sql/triggers.sql work/src/test/regress/sql/triggers.sql
*** head/src/test/regress/sql/triggers.sql 2009-10-15 10:26:48.573966000 +0900
--- work/src/test/regress/sql/triggers.sql 2009-10-15 16:08:06.536354097 +0900
*************** COPY main_table (a, b) FROM stdin;
*** 254,259 ****
--- 254,271 ----
SELECT * FROM main_table ORDER BY a, b;
+ -- Trigger with WHEN clause
+ CREATE TRIGGER modified_a BEFORE UPDATE OF a ON main_table
+ FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE trigger_func('modified_a');
+ CREATE TRIGGER modified_any BEFORE UPDATE OF a ON main_table
+ FOR EACH ROW WHEN (OLD.* IS DISTINCT FROM NEW.*) EXECUTE PROCEDURE trigger_func('modified_any');
+ UPDATE main_table SET a = 50, b = 60;
+ SELECT * FROM main_table ORDER BY a, b;
+ SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_a';
+ SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_any';
+ DROP TRIGGER modified_a ON main_table;
+ DROP TRIGGER modified_any ON main_table;
+
-- Test column-level triggers
DROP TRIGGER after_upd_row_trig ON main_table;
*************** FOR EACH ROW EXECUTE PROCEDURE trigger_f
*** 283,288 ****
--- 295,307 ----
CREATE TRIGGER error_ins_a BEFORE INSERT OF a ON main_table
FOR EACH ROW EXECUTE PROCEDURE trigger_func('error_ins_a');
+ CREATE TRIGGER error_ins_when BEFORE INSERT OR UPDATE ON main_table
+ FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE trigger_func('error_ins_when');
+ CREATE TRIGGER error_del_when BEFORE DELETE OR UPDATE ON main_table
+ FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE trigger_func('error_del_when');
+ CREATE TRIGGER error_stmt_when BEFORE UPDATE OF a ON main_table
+ FOR EACH STATEMENT WHEN (OLD.* IS DISTINCT FROM NEW.*) EXECUTE PROCEDURE trigger_func('error_stmt_when');
+
-- check dependency restrictions
ALTER TABLE main_table DROP COLUMN b;
-- this should succeed, but we'll roll it back to keep the triggers around
Itagaki Takahiro <itagaki.takahiro@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
2009/10/15 Tom Lane <tgl@sss.pgh.pa.us>:
Itagaki Takahiro <itagaki.takahiro@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
Show quoted text
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
Pavel Stehule <pavel.stehule@gmail.com> writes:
2009/10/15 Tom Lane <tgl@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
Tom Lane <tgl@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
Dimitri Fontaine <dfontaine@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
Tom Lane <tgl@sss.pgh.pa.us> wrote:
Itagaki Takahiro <itagaki.takahiro@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
On Thu, 2009-10-15 at 16:54 -0500, Kevin Grittner wrote:
Dimitri Fontaine <dfontaine@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
On Fri, 2009-10-16 at 10:14 +0900, Itagaki Takahiro wrote:
Tom Lane <tgl@sss.pgh.pa.us> wrote:
Itagaki Takahiro <itagaki.takahiro@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
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.
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
Peter Eisentraut <peter_e@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
On Fri, 2009-10-16 at 10:02 -0400, Tom Lane wrote:
Peter Eisentraut <peter_e@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
Simon Riggs <simon@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
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.
Stephan Szabo <sszabo@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