Skip to content

Commit 52120ee

Browse files
tglsfdctglsfdc
committed
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.
1 parent 706493a commit 52120ee

File tree

6 files changed

+67
-4
lines changed

6 files changed

+67
-4
lines changed

‎src/backend/commands/trigger.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2569,13 +2569,13 @@ TriggerEnabled(EState *estate, ResultRelInfo *relinfo,
25692569
}
25702570
if (HeapTupleIsValid(newtup))
25712571
{
2572-
if (estate->es_trig_tuple_slot == NULL)
2572+
if (estate->es_trig_newtup_slot == NULL)
25732573
{
25742574
oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
2575-
estate->es_trig_tuple_slot = ExecInitExtraTupleSlot(estate);
2575+
estate->es_trig_newtup_slot = ExecInitExtraTupleSlot(estate);
25762576
MemoryContextSwitchTo(oldContext);
25772577
}
2578-
newslot = estate->es_trig_tuple_slot;
2578+
newslot = estate->es_trig_newtup_slot;
25792579
if (newslot->tts_tupleDescriptor != tupdesc)
25802580
ExecSetSlotDescriptor(newslot, tupdesc);
25812581
ExecStoreTuple(newtup, newslot, InvalidBuffer, false);

‎src/backend/executor/execMain.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
790790
estate->es_tupleTable = NIL;
791791
estate->es_trig_tuple_slot = NULL;
792792
estate->es_trig_oldtup_slot = NULL;
793+
estate->es_trig_newtup_slot = NULL;
793794

794795
/* mark EvalPlanQual not active */
795796
estate->es_epqTuple = NULL;

‎src/backend/executor/execUtils.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ CreateExecutorState(void)
124124
estate->es_trig_target_relations = NIL;
125125
estate->es_trig_tuple_slot = NULL;
126126
estate->es_trig_oldtup_slot = NULL;
127+
estate->es_trig_newtup_slot = NULL;
127128

128129
estate->es_param_list_info = NULL;
129130
estate->es_param_exec_vals = NULL;

‎src/include/nodes/execnodes.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ typedef struct EState
354354
/* Stuff used for firing triggers: */
355355
List *es_trig_target_relations; /* trigger-only ResultRelInfos */
356356
TupleTableSlot *es_trig_tuple_slot; /* for trigger output tuples */
357-
TupleTableSlot *es_trig_oldtup_slot; /* for trigger old tuples */
357+
TupleTableSlot *es_trig_oldtup_slot; /* for TriggerEnabled */
358358

359359
/* Parameter info: */
360360
ParamListInfo es_param_list_info; /* values of external params */
@@ -397,6 +397,12 @@ typedef struct EState
397397
HeapTuple *es_epqTuple; /* array of EPQ substitute tuples */
398398
bool *es_epqTupleSet; /* true if EPQ tuple is provided */
399399
bool *es_epqScanDone; /* true if EPQ tuple has been fetched */
400+
401+
/*
402+
* this field added at end of struct to avoid post-release ABI breakage in
403+
* existing release branches. It'll be in a more logical place in 9.2.
404+
*/
405+
TupleTableSlot *es_trig_newtup_slot; /* for TriggerEnabled */
400406
} EState;
401407

402408

‎src/test/regress/expected/triggers.out

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,35 @@ NOTICE: trigger_func(after_upd_a_b_row) called: action = UPDATE, when = AFTER,
446446
NOTICE: trigger_func(after_upd_b_row) called: action = UPDATE, when = AFTER, level = ROW
447447
NOTICE: trigger_func(after_upd_b_stmt) called: action = UPDATE, when = AFTER, level = STATEMENT
448448
NOTICE: trigger_func(after_upd_stmt) called: action = UPDATE, when = AFTER, level = STATEMENT
449+
--
450+
-- Test case for bug with BEFORE trigger followed by AFTER trigger with WHEN
451+
--
452+
CREATE TABLE some_t (some_col boolean NOT NULL);
453+
CREATE FUNCTION dummy_update_func() RETURNS trigger AS $$
454+
BEGIN
455+
RAISE NOTICE 'dummy_update_func(%) called: action = %, old = %, new = %',
456+
TG_ARGV[0], TG_OP, OLD, NEW;
457+
RETURN NEW;
458+
END;
459+
$$ LANGUAGE plpgsql;
460+
CREATE TRIGGER some_trig_before BEFORE UPDATE ON some_t FOR EACH ROW
461+
EXECUTE PROCEDURE dummy_update_func('before');
462+
CREATE TRIGGER some_trig_aftera AFTER UPDATE ON some_t FOR EACH ROW
463+
WHEN (NOT OLD.some_col AND NEW.some_col)
464+
EXECUTE PROCEDURE dummy_update_func('aftera');
465+
CREATE TRIGGER some_trig_afterb AFTER UPDATE ON some_t FOR EACH ROW
466+
WHEN (NOT NEW.some_col)
467+
EXECUTE PROCEDURE dummy_update_func('afterb');
468+
INSERT INTO some_t VALUES (TRUE);
469+
UPDATE some_t SET some_col = TRUE;
470+
NOTICE: dummy_update_func(before) called: action = UPDATE, old = (t), new = (t)
471+
UPDATE some_t SET some_col = FALSE;
472+
NOTICE: dummy_update_func(before) called: action = UPDATE, old = (t), new = (f)
473+
NOTICE: dummy_update_func(afterb) called: action = UPDATE, old = (t), new = (f)
474+
UPDATE some_t SET some_col = TRUE;
475+
NOTICE: dummy_update_func(before) called: action = UPDATE, old = (f), new = (t)
476+
NOTICE: dummy_update_func(aftera) called: action = UPDATE, old = (f), new = (t)
477+
DROP TABLE some_t;
449478
-- bogus cases
450479
CREATE TRIGGER error_upd_and_col BEFORE UPDATE OR UPDATE OF a ON main_table
451480
FOR EACH ROW EXECUTE PROCEDURE trigger_func('error_upd_and_col');

‎src/test/regress/sql/triggers.sql

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,32 @@ SELECT pg_get_triggerdef(oid) FROM pg_trigger WHERE tgrelid = 'main_table'::regc
308308
UPDATE main_table SET a = 50;
309309
UPDATE main_table SET b = 10;
310310

311+
--
312+
-- Test case for bug with BEFORE trigger followed by AFTER trigger with WHEN
313+
--
314+
315+
CREATE TABLE some_t (some_col boolean NOT NULL);
316+
CREATE FUNCTION dummy_update_func() RETURNS trigger AS $$
317+
BEGIN
318+
RAISE NOTICE 'dummy_update_func(%) called: action = %, old = %, new = %',
319+
TG_ARGV[0], TG_OP, OLD, NEW;
320+
RETURN NEW;
321+
END;
322+
$$ LANGUAGE plpgsql;
323+
CREATE TRIGGER some_trig_before BEFORE UPDATE ON some_t FOR EACH ROW
324+
EXECUTE PROCEDURE dummy_update_func('before');
325+
CREATE TRIGGER some_trig_aftera AFTER UPDATE ON some_t FOR EACH ROW
326+
WHEN (NOT OLD.some_col AND NEW.some_col)
327+
EXECUTE PROCEDURE dummy_update_func('aftera');
328+
CREATE TRIGGER some_trig_afterb AFTER UPDATE ON some_t FOR EACH ROW
329+
WHEN (NOT NEW.some_col)
330+
EXECUTE PROCEDURE dummy_update_func('afterb');
331+
INSERT INTO some_t VALUES (TRUE);
332+
UPDATE some_t SET some_col = TRUE;
333+
UPDATE some_t SET some_col = FALSE;
334+
UPDATE some_t SET some_col = TRUE;
335+
DROP TABLE some_t;
336+
311337
-- bogus cases
312338
CREATE TRIGGER error_upd_and_col BEFORE UPDATE OR UPDATE OF a ON main_table
313339
FOR EACH ROW EXECUTE PROCEDURE trigger_func('error_upd_and_col');

0 commit comments

Comments
 (0)