diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index a6b75f06d58..14f8980a99e 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.177.4.3 2006/01/12 21:49:16 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.177.4.4 2007/08/15 19:16:12 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2340,10 +2340,12 @@ AfterTriggerEndQuery(void) * SET CONSTRAINTS ... IMMEDIATE: all events we have decided to defer * will be available for it to fire. * + * We loop in case a trigger queues more events. + * * If we find no firable events, we don't have to increment firing_counter. */ events = &afterTriggers->query_stack[afterTriggers->query_depth]; - if (afterTriggerMarkEvents(events, &afterTriggers->events, true)) + while (afterTriggerMarkEvents(events, &afterTriggers->events, true)) { CommandId firing_id = afterTriggers->firing_counter++; @@ -2834,7 +2836,7 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt) { AfterTriggerEventList *events = &afterTriggers->events; - if (afterTriggerMarkEvents(events, NULL, true)) + while (afterTriggerMarkEvents(events, NULL, true)) { CommandId firing_id = afterTriggers->firing_counter++; diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 71e40ed25b4..77e2cafb109 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.133.4.2 2007/03/17 03:16:03 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.133.4.3 2007/08/15 19:16:12 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -39,9 +39,9 @@ static void _SPI_prepare_plan(const char *src, _SPI_plan *plan); static int _SPI_execute_plan(_SPI_plan *plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, int tcount); + bool read_only, bool fire_triggers, int tcount); -static int _SPI_pquery(QueryDesc *queryDesc, int tcount); +static int _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, int tcount); static void _SPI_error_callback(void *arg); @@ -301,7 +301,7 @@ SPI_execute(const char *src, bool read_only, int tcount) res = _SPI_execute_plan(&plan, NULL, NULL, InvalidSnapshot, InvalidSnapshot, - read_only, tcount); + read_only, true, tcount); _SPI_end_call(true); return res; @@ -334,7 +334,7 @@ SPI_execute_plan(void *plan, Datum *Values, const char *Nulls, res = _SPI_execute_plan((_SPI_plan *) plan, Values, Nulls, InvalidSnapshot, InvalidSnapshot, - read_only, tcount); + read_only, true, tcount); _SPI_end_call(true); return res; @@ -349,9 +349,12 @@ SPI_execp(void *plan, Datum *Values, const char *Nulls, int tcount) /* * SPI_execute_snapshot -- identical to SPI_execute_plan, except that we allow - * the caller to specify exactly which snapshots to use. This is currently - * not documented in spi.sgml because it is only intended for use by RI - * triggers. + * the caller to specify exactly which snapshots to use. Also, the caller + * may specify that AFTER triggers should be queued as part of the outer + * query rather than being fired immediately at the end of the command. + * + * This is currently not documented in spi.sgml because it is only intended + * for use by RI triggers. * * Passing snapshot == InvalidSnapshot will select the normal behavior of * fetching a new snapshot for each query. @@ -360,7 +363,7 @@ int SPI_execute_snapshot(void *plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, int tcount) + bool read_only, bool fire_triggers, int tcount) { int res; @@ -377,7 +380,7 @@ SPI_execute_snapshot(void *plan, res = _SPI_execute_plan((_SPI_plan *) plan, Values, Nulls, snapshot, crosscheck_snapshot, - read_only, tcount); + read_only, fire_triggers, tcount); _SPI_end_call(true); return res; @@ -1317,12 +1320,14 @@ _SPI_prepare_plan(const char *src, _SPI_plan *plan) * behavior of taking a new snapshot for each query. * crosscheck_snapshot: for RI use, all others pass InvalidSnapshot * read_only: TRUE for read-only execution (no CommandCounterIncrement) + * fire_triggers: TRUE to fire AFTER triggers at end of query (normal case); + * FALSE means any AFTER triggers are postponed to end of outer query * tcount: execution tuple-count limit, or 0 for none */ static int _SPI_execute_plan(_SPI_plan *plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, int tcount) + bool read_only, bool fire_triggers, int tcount) { volatile int res = 0; Snapshot saveActiveSnapshot; @@ -1473,7 +1478,7 @@ _SPI_execute_plan(_SPI_plan *plan, Datum *Values, const char *Nulls, crosscheck_snapshot, dest, paramLI, false); - res = _SPI_pquery(qdesc, + res = _SPI_pquery(qdesc, fire_triggers, queryTree->canSetTag ? tcount : 0); FreeQueryDesc(qdesc); } @@ -1506,7 +1511,7 @@ fail: } static int -_SPI_pquery(QueryDesc *queryDesc, int tcount) +_SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, int tcount) { int operation = queryDesc->operation; int res; @@ -1540,7 +1545,8 @@ _SPI_pquery(QueryDesc *queryDesc, int tcount) ResetUsage(); #endif - AfterTriggerBeginQuery(); + if (fire_triggers) + AfterTriggerBeginQuery(); ExecutorStart(queryDesc, false); @@ -1558,7 +1564,8 @@ _SPI_pquery(QueryDesc *queryDesc, int tcount) ExecutorEnd(queryDesc); /* Take care of any queued AFTER triggers */ - AfterTriggerEndQuery(); + if (fire_triggers) + AfterTriggerEndQuery(); if (queryDesc->dest->mydest == SPI) { diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 46653423c1c..2577cdcd9b6 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -17,7 +17,7 @@ * * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.76.4.1 2007/07/17 17:45:56 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.76.4.2 2007/08/15 19:16:12 tgl Exp $ * * ---------- */ @@ -2709,7 +2709,7 @@ RI_Initial_Check(FkConstraint *fkconstraint, Relation rel, Relation pkrel) NULL, NULL, CopySnapshot(GetLatestSnapshot()), InvalidSnapshot, - true, 1); + true, false, 1); /* Check result */ if (spi_result != SPI_OK_SELECT) @@ -3139,7 +3139,7 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan, spi_result = SPI_execute_snapshot(qplan, vals, nulls, test_snapshot, crosscheck_snapshot, - false, limit); + false, false, limit); /* Restore UID */ SetUserId(save_uid); diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h index d5ba89fa3ea..8ec13ca45b8 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -2,7 +2,7 @@ * * spi.h * - * $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.50 2004/11/16 18:10:13 tgl Exp $ + * $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.50.4.1 2007/08/15 19:16:12 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -92,7 +92,7 @@ extern int SPI_execute_snapshot(void *plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, int tcount); + bool read_only, bool fire_triggers, int tcount); extern void *SPI_prepare(const char *src, int nargs, Oid *argtypes); extern void *SPI_saveplan(void *plan); extern int SPI_freeplan(void *plan); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index a479f4932a8..72e41a48947 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -646,7 +646,6 @@ SELECT * from FKTABLE; UPDATE PKTABLE set ptest2=5 where ptest2=2; ERROR: insert or update on table "fktable" violates foreign key constraint "constrname3" DETAIL: Key (ftest1,ftest2,ftest3)=(1,-1,3) is not present in table "pktable". -CONTEXT: SQL statement "UPDATE ONLY "public"."fktable" SET "ftest2" = DEFAULT WHERE "ftest1" = $1 AND "ftest2" = $2 AND "ftest3" = $3" -- Try to update something that will set default UPDATE PKTABLE set ptest1=0, ptest2=5, ptest3=10 where ptest2=2; UPDATE PKTABLE set ptest2=10 where ptest2=4; @@ -1207,3 +1206,71 @@ ROLLBACK TO savept1; COMMIT; ERROR: insert or update on table "fktable" violates foreign key constraint "fktable_fk_fkey" DETAIL: Key (fk)=(20) is not present in table "pktable". +-- test order of firing of FK triggers when several RI-induced changes need to +-- be made to the same row. This was broken by subtransaction-related +-- changes in 8.0. +CREATE TEMP TABLE users ( + id INT PRIMARY KEY, + name VARCHAR NOT NULL +); +NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "users_pkey" for table "users" +INSERT INTO users VALUES (1, 'Jozko'); +INSERT INTO users VALUES (2, 'Ferko'); +INSERT INTO users VALUES (3, 'Samko'); +CREATE TEMP TABLE tasks ( + id INT PRIMARY KEY, + owner INT REFERENCES users ON UPDATE CASCADE ON DELETE SET NULL, + worker INT REFERENCES users ON UPDATE CASCADE ON DELETE SET NULL, + checked_by INT REFERENCES users ON UPDATE CASCADE ON DELETE SET NULL +); +NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "tasks_pkey" for table "tasks" +INSERT INTO tasks VALUES (1,1,NULL,NULL); +INSERT INTO tasks VALUES (2,2,2,NULL); +INSERT INTO tasks VALUES (3,3,3,3); +SELECT * FROM tasks; + id | owner | worker | checked_by +----+-------+--------+------------ + 1 | 1 | | + 2 | 2 | 2 | + 3 | 3 | 3 | 3 +(3 rows) + +UPDATE users SET id = 4 WHERE id = 3; +SELECT * FROM tasks; + id | owner | worker | checked_by +----+-------+--------+------------ + 1 | 1 | | + 2 | 2 | 2 | + 3 | 4 | 4 | 4 +(3 rows) + +DELETE FROM users WHERE id = 4; +SELECT * FROM tasks; + id | owner | worker | checked_by +----+-------+--------+------------ + 1 | 1 | | + 2 | 2 | 2 | + 3 | | | +(3 rows) + +-- could fail with only 2 changes to make, if row was already updated +BEGIN; +UPDATE tasks set id=id WHERE id=2; +SELECT * FROM tasks; + id | owner | worker | checked_by +----+-------+--------+------------ + 1 | 1 | | + 3 | | | + 2 | 2 | 2 | +(3 rows) + +DELETE FROM users WHERE id = 2; +SELECT * FROM tasks; + id | owner | worker | checked_by +----+-------+--------+------------ + 1 | 1 | | + 3 | | | + 2 | | | +(3 rows) + +COMMIT; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index dee046a1952..562796ecceb 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -855,3 +855,45 @@ ROLLBACK TO savept1; -- should catch error from initial INSERT COMMIT; + +-- test order of firing of FK triggers when several RI-induced changes need to +-- be made to the same row. This was broken by subtransaction-related +-- changes in 8.0. + +CREATE TEMP TABLE users ( + id INT PRIMARY KEY, + name VARCHAR NOT NULL +); + +INSERT INTO users VALUES (1, 'Jozko'); +INSERT INTO users VALUES (2, 'Ferko'); +INSERT INTO users VALUES (3, 'Samko'); + +CREATE TEMP TABLE tasks ( + id INT PRIMARY KEY, + owner INT REFERENCES users ON UPDATE CASCADE ON DELETE SET NULL, + worker INT REFERENCES users ON UPDATE CASCADE ON DELETE SET NULL, + checked_by INT REFERENCES users ON UPDATE CASCADE ON DELETE SET NULL +); + +INSERT INTO tasks VALUES (1,1,NULL,NULL); +INSERT INTO tasks VALUES (2,2,2,NULL); +INSERT INTO tasks VALUES (3,3,3,3); + +SELECT * FROM tasks; + +UPDATE users SET id = 4 WHERE id = 3; + +SELECT * FROM tasks; + +DELETE FROM users WHERE id = 4; + +SELECT * FROM tasks; + +-- could fail with only 2 changes to make, if row was already updated +BEGIN; +UPDATE tasks set id=id WHERE id=2; +SELECT * FROM tasks; +DELETE FROM users WHERE id = 2; +SELECT * FROM tasks; +COMMIT;