Fix trigger WHEN conditions when both BEFORE and AFTER triggers exist.

Due to tuple-slot mismanagement, evaluation of WHEN conditions for AFTER
ROW UPDATE triggers could crash if there had been a BEFORE ROW trigger
fired for the same update.  Fix by not trying to overload the use of
estate->es_trig_tuple_slot.  Per report from Yoran Heling.

Back-patch to 9.0, when trigger WHEN conditions were introduced.

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d67611b2ce3d26226f8ffff52995d00752a94026..e18fe725ab7a1933b5ff335b47dda0b3ccfa0255 100644 (file)
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2569,13 +2569,13 @@ TriggerEnabled(EState *estate, ResultRelInfo *relinfo,
                }
                if (HeapTupleIsValid(newtup))
                {
-                       if (estate->es_trig_tuple_slot == NULL)
+                       if (estate->es_trig_newtup_slot == NULL)
                        {
                                oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
-                               estate->es_trig_tuple_slot = ExecInitExtraTupleSlot(estate);
+                               estate->es_trig_newtup_slot = ExecInitExtraTupleSlot(estate);
                                MemoryContextSwitchTo(oldContext);
                        }
-                       newslot = estate->es_trig_tuple_slot;
+                       newslot = estate->es_trig_newtup_slot;
                        if (newslot->tts_tupleDescriptor != tupdesc)
                                ExecSetSlotDescriptor(newslot, tupdesc);
                        ExecStoreTuple(newtup, newslot, InvalidBuffer, false);

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index bbe67ed8bbaf961025f8a658e2a16880ad4e9519..504f4de7013c837442885d5192c9ba74579bb60e 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -790,6 +790,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
        estate->es_tupleTable = NIL;
        estate->es_trig_tuple_slot = NULL;
        estate->es_trig_oldtup_slot = NULL;
+       estate->es_trig_newtup_slot = NULL;
 
        /* mark EvalPlanQual not active */
        estate->es_epqTuple = NULL;

diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 5886c1bbd64afa83d60e1436c57b406664394803..e76767256c043a30254d2984e149885572643cfa 100644 (file)
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -124,6 +124,7 @@ CreateExecutorState(void)
        estate->es_trig_target_relations = NIL;
        estate->es_trig_tuple_slot = NULL;
        estate->es_trig_oldtup_slot = NULL;
+       estate->es_trig_newtup_slot = NULL;
 
        estate->es_param_list_info = NULL;
        estate->es_param_exec_vals = NULL;

diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index ac0de093f1671229fb439c883547c9f27ad60b56..f7a6787b55a234815e131d08547d23b8fee32563 100644 (file)
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -354,7 +354,7 @@ typedef struct EState
        /* Stuff used for firing triggers: */
        List       *es_trig_target_relations;           /* trigger-only ResultRelInfos */
        TupleTableSlot *es_trig_tuple_slot; /* for trigger output tuples */
-       TupleTableSlot *es_trig_oldtup_slot;            /* for trigger old tuples */
+       TupleTableSlot *es_trig_oldtup_slot;            /* for TriggerEnabled */
 
        /* Parameter info: */
        ParamListInfo es_param_list_info;       /* values of external params */
@@ -397,6 +397,12 @@ typedef struct EState
        HeapTuple  *es_epqTuple;        /* array of EPQ substitute tuples */
        bool       *es_epqTupleSet; /* true if EPQ tuple is provided */
        bool       *es_epqScanDone; /* true if EPQ tuple has been fetched */
+
+       /*
+        * this field added at end of struct to avoid post-release ABI breakage in
+        * existing release branches.  It'll be in a more logical place in 9.2.
+        */
+       TupleTableSlot *es_trig_newtup_slot;            /* for TriggerEnabled */
 } EState;
 
 

diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index fe98079542007d683261f3895785d89cf0a23a25..53d8d57bd53791364aab3914bacdb468d848c306 100644 (file)
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -446,6 +446,35 @@ NOTICE:  trigger_func(after_upd_a_b_row) called: action = UPDATE, when = AFTER,
 NOTICE:  trigger_func(after_upd_b_row) called: action = UPDATE, when = AFTER, level = ROW
 NOTICE:  trigger_func(after_upd_b_stmt) called: action = UPDATE, when = AFTER, level = STATEMENT
 NOTICE:  trigger_func(after_upd_stmt) called: action = UPDATE, when = AFTER, level = STATEMENT
+--
+-- Test case for bug with BEFORE trigger followed by AFTER trigger with WHEN
+--
+CREATE TABLE some_t (some_col boolean NOT NULL);
+CREATE FUNCTION dummy_update_func() RETURNS trigger AS $$
+BEGIN
+  RAISE NOTICE 'dummy_update_func(%) called: action = %, old = %, new = %',
+    TG_ARGV[0], TG_OP, OLD, NEW;
+  RETURN NEW;
+END;
+$$ LANGUAGE plpgsql;
+CREATE TRIGGER some_trig_before BEFORE UPDATE ON some_t FOR EACH ROW
+  EXECUTE PROCEDURE dummy_update_func('before');
+CREATE TRIGGER some_trig_aftera AFTER UPDATE ON some_t FOR EACH ROW
+  WHEN (NOT OLD.some_col AND NEW.some_col)
+  EXECUTE PROCEDURE dummy_update_func('aftera');
+CREATE TRIGGER some_trig_afterb AFTER UPDATE ON some_t FOR EACH ROW
+  WHEN (NOT NEW.some_col)
+  EXECUTE PROCEDURE dummy_update_func('afterb');
+INSERT INTO some_t VALUES (TRUE);
+UPDATE some_t SET some_col = TRUE;
+NOTICE:  dummy_update_func(before) called: action = UPDATE, old = (t), new = (t)
+UPDATE some_t SET some_col = FALSE;
+NOTICE:  dummy_update_func(before) called: action = UPDATE, old = (t), new = (f)
+NOTICE:  dummy_update_func(afterb) called: action = UPDATE, old = (t), new = (f)
+UPDATE some_t SET some_col = TRUE;
+NOTICE:  dummy_update_func(before) called: action = UPDATE, old = (f), new = (t)
+NOTICE:  dummy_update_func(aftera) called: action = UPDATE, old = (f), new = (t)
+DROP TABLE some_t;
 -- bogus cases
 CREATE TRIGGER error_upd_and_col BEFORE UPDATE OR UPDATE OF a ON main_table
 FOR EACH ROW EXECUTE PROCEDURE trigger_func('error_upd_and_col');

diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 4decbfb68f8939413b74f53a4a57b32d6856bbdc..f59d832f995d449e884f6cb574f10531cf3ce264 100644 (file)
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -308,6 +308,32 @@ SELECT pg_get_triggerdef(oid) FROM pg_trigger WHERE tgrelid = 'main_table'::regc
 UPDATE main_table SET a = 50;
 UPDATE main_table SET b = 10;
 
+--
+-- Test case for bug with BEFORE trigger followed by AFTER trigger with WHEN
+--
+
+CREATE TABLE some_t (some_col boolean NOT NULL);
+CREATE FUNCTION dummy_update_func() RETURNS trigger AS $$
+BEGIN
+  RAISE NOTICE 'dummy_update_func(%) called: action = %, old = %, new = %',
+    TG_ARGV[0], TG_OP, OLD, NEW;
+  RETURN NEW;
+END;
+$$ LANGUAGE plpgsql;
+CREATE TRIGGER some_trig_before BEFORE UPDATE ON some_t FOR EACH ROW
+  EXECUTE PROCEDURE dummy_update_func('before');
+CREATE TRIGGER some_trig_aftera AFTER UPDATE ON some_t FOR EACH ROW
+  WHEN (NOT OLD.some_col AND NEW.some_col)
+  EXECUTE PROCEDURE dummy_update_func('aftera');
+CREATE TRIGGER some_trig_afterb AFTER UPDATE ON some_t FOR EACH ROW
+  WHEN (NOT NEW.some_col)
+  EXECUTE PROCEDURE dummy_update_func('afterb');
+INSERT INTO some_t VALUES (TRUE);
+UPDATE some_t SET some_col = TRUE;
+UPDATE some_t SET some_col = FALSE;
+UPDATE some_t SET some_col = TRUE;
+DROP TABLE some_t;
+
 -- bogus cases
 CREATE TRIGGER error_upd_and_col BEFORE UPDATE OR UPDATE OF a ON main_table
 FOR EACH ROW EXECUTE PROCEDURE trigger_func('error_upd_and_col');