diff options
| -rw-r--r-- | src/backend/commands/trigger.c | 53 | ||||
| -rw-r--r-- | src/backend/executor/spi.c | 38 | ||||
| -rw-r--r-- | src/backend/utils/adt/ri_triggers.c | 6 | ||||
| -rw-r--r-- | src/include/executor/spi.h | 4 | ||||
| -rw-r--r-- | src/test/regress/expected/foreign_key.out | 69 | ||||
| -rw-r--r-- | src/test/regress/sql/foreign_key.sql | 42 |
6 files changed, 170 insertions, 42 deletions
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 80b28f5e4b4..2788684d1ab 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.216 2007/07/17 17:45:28 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.217 2007/08/15 19:15:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2332,6 +2332,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, AfterTriggerEvent event, prev_event; MemoryContext per_tuple_context; + bool locally_opened = false; Relation rel = NULL; TriggerDesc *trigdesc = NULL; FmgrInfo *finfo = NULL; @@ -2364,6 +2365,19 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, */ if (rel == NULL || rel->rd_id != event->ate_relid) { + if (locally_opened) + { + /* close prior rel if any */ + if (rel) + heap_close(rel, NoLock); + if (trigdesc) + FreeTriggerDesc(trigdesc); + if (finfo) + pfree(finfo); + Assert(instr == NULL); /* never used in this case */ + } + locally_opened = true; + if (estate) { /* Find target relation among estate's result rels */ @@ -2375,28 +2389,22 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, while (nr > 0) { if (rInfo->ri_RelationDesc->rd_id == event->ate_relid) + { + rel = rInfo->ri_RelationDesc; + trigdesc = rInfo->ri_TrigDesc; + finfo = rInfo->ri_TrigFunctions; + instr = rInfo->ri_TrigInstrument; + locally_opened = false; break; + } rInfo++; nr--; } - if (nr <= 0) /* should not happen */ - elog(ERROR, "could not find relation %u among query result relations", - event->ate_relid); - rel = rInfo->ri_RelationDesc; - trigdesc = rInfo->ri_TrigDesc; - finfo = rInfo->ri_TrigFunctions; - instr = rInfo->ri_TrigInstrument; } - else + + if (locally_opened) { - /* Hard way: we manage the resources for ourselves */ - if (rel) - heap_close(rel, NoLock); - if (trigdesc) - FreeTriggerDesc(trigdesc); - if (finfo) - pfree(finfo); - Assert(instr == NULL); /* never used in this case */ + /* Hard way: open target relation for ourselves */ /* * We assume that an appropriate lock is still held by the @@ -2421,6 +2429,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, palloc0(trigdesc->numtriggers * sizeof(FmgrInfo)); /* Never any EXPLAIN info in this case */ + instr = NULL; } } @@ -2471,7 +2480,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, events->tail = prev_event; /* Release working resources */ - if (!estate) + if (locally_opened) { if (rel) heap_close(rel, NoLock); @@ -2600,11 +2609,13 @@ AfterTriggerEndQuery(EState *estate) * 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++; @@ -2648,7 +2659,7 @@ AfterTriggerFireDeferred(void) ActiveSnapshot = CopySnapshot(GetTransactionSnapshot()); /* - * Run all the remaining triggers. Loop until they are all gone, just in + * Run all the remaining triggers. Loop until they are all gone, in * case some trigger queues more for us to do. */ while (afterTriggerMarkEvents(events, NULL, false)) @@ -3211,7 +3222,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 f97a86fe214..af94ad1a3b8 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.179 2007/04/27 22:05:47 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.180 2007/08/15 19:15:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -39,9 +39,9 @@ static void _SPI_prepare_plan(const char *src, SPIPlanPtr plan); static int _SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, long tcount); + bool read_only, bool fire_triggers, long tcount); -static int _SPI_pquery(QueryDesc *queryDesc, long tcount); +static int _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, long tcount); static void _SPI_error_callback(void *arg); @@ -316,7 +316,7 @@ SPI_execute(const char *src, bool read_only, long tcount) res = _SPI_execute_plan(&plan, NULL, NULL, InvalidSnapshot, InvalidSnapshot, - read_only, tcount); + read_only, true, tcount); _SPI_end_call(true); return res; @@ -349,7 +349,7 @@ SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls, res = _SPI_execute_plan(plan, Values, Nulls, InvalidSnapshot, InvalidSnapshot, - read_only, tcount); + read_only, true, tcount); _SPI_end_call(true); return res; @@ -364,9 +364,12 @@ SPI_execp(SPIPlanPtr plan, Datum *Values, const char *Nulls, long 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. @@ -375,7 +378,7 @@ int SPI_execute_snapshot(SPIPlanPtr plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, long tcount) + bool read_only, bool fire_triggers, long tcount) { int res; @@ -392,7 +395,7 @@ SPI_execute_snapshot(SPIPlanPtr plan, res = _SPI_execute_plan(plan, Values, Nulls, snapshot, crosscheck_snapshot, - read_only, tcount); + read_only, fire_triggers, tcount); _SPI_end_call(true); return res; @@ -1428,12 +1431,14 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr 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(SPIPlanPtr plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, long tcount) + bool read_only, bool fire_triggers, long tcount) { volatile int my_res = 0; volatile uint32 my_processed = 0; @@ -1589,7 +1594,8 @@ _SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls, crosscheck_snapshot, dest, paramLI, false); - res = _SPI_pquery(qdesc, canSetTag ? tcount : 0); + res = _SPI_pquery(qdesc, fire_triggers, + canSetTag ? tcount : 0); FreeQueryDesc(qdesc); } else @@ -1680,7 +1686,7 @@ fail: } static int -_SPI_pquery(QueryDesc *queryDesc, long tcount) +_SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, long tcount) { int operation = queryDesc->operation; int res; @@ -1726,7 +1732,8 @@ _SPI_pquery(QueryDesc *queryDesc, long tcount) ResetUsage(); #endif - AfterTriggerBeginQuery(); + if (fire_triggers) + AfterTriggerBeginQuery(); ExecutorStart(queryDesc, 0); @@ -1743,7 +1750,8 @@ _SPI_pquery(QueryDesc *queryDesc, long tcount) } /* Take care of any queued AFTER triggers */ - AfterTriggerEndQuery(queryDesc->estate); + if (fire_triggers) + AfterTriggerEndQuery(queryDesc->estate); ExecutorEnd(queryDesc); diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 3f257f375f4..9add8f934d8 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -15,7 +15,7 @@ * * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.95 2007/06/05 21:31:06 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.96 2007/08/15 19:15:46 tgl Exp $ * * ---------- */ @@ -2774,7 +2774,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) NULL, NULL, CopySnapshot(GetLatestSnapshot()), InvalidSnapshot, - true, 1); + true, false, 1); /* Check result */ if (spi_result != SPI_OK_SELECT) @@ -3308,7 +3308,7 @@ ri_PerformCheck(RI_QueryKey *qkey, SPIPlanPtr 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 20beefe4401..3ef50993c62 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.62 2007/07/25 12:22:53 mha Exp $ + * $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.63 2007/08/15 19:15:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -104,7 +104,7 @@ extern int SPI_execute_snapshot(SPIPlanPtr plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, long tcount); + bool read_only, bool fire_triggers, long tcount); extern SPIPlanPtr SPI_prepare(const char *src, int nargs, Oid *argtypes); extern SPIPlanPtr SPI_prepare_cursor(const char *src, int nargs, Oid *argtypes, int cursorOptions); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 56411f06a35..e24ceb51a87 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 $1 OPERATOR(pg_catalog.=) "ftest1" AND $2 OPERATOR(pg_catalog.=) "ftest2" AND $3 OPERATOR(pg_catalog.=) "ftest3"" -- 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; @@ -1230,3 +1229,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 1ab557b9bf3..519a5056af0 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -879,3 +879,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; |