Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9cb8409

Browse files
committed
Repair problems occurring when multiple RI updates have to be done to the same
row within one query: we were firing check triggers before all the updateswere done, leading to bogus failures. Fix by making the triggers queued byan RI update go at the end of the outer query's trigger event list, therebyeffectively making the processing "breadth-first". This was indeed how itworked pre-8.0, so the bug does not occur in the 7.x branches.Per report from Pavel Stehule.
1 parent5ff95e6 commit9cb8409

File tree

6 files changed

+170
-42
lines changed

6 files changed

+170
-42
lines changed

‎src/backend/commands/trigger.c

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.216 2007/07/17 17:45:28 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.217 2007/08/15 19:15:46 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -2332,6 +2332,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
23322332
AfterTriggerEventevent,
23332333
prev_event;
23342334
MemoryContextper_tuple_context;
2335+
boollocally_opened= false;
23352336
Relationrel=NULL;
23362337
TriggerDesc*trigdesc=NULL;
23372338
FmgrInfo*finfo=NULL;
@@ -2364,6 +2365,19 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
23642365
*/
23652366
if (rel==NULL||rel->rd_id!=event->ate_relid)
23662367
{
2368+
if (locally_opened)
2369+
{
2370+
/* close prior rel if any */
2371+
if (rel)
2372+
heap_close(rel,NoLock);
2373+
if (trigdesc)
2374+
FreeTriggerDesc(trigdesc);
2375+
if (finfo)
2376+
pfree(finfo);
2377+
Assert(instr==NULL);/* never used in this case */
2378+
}
2379+
locally_opened= true;
2380+
23672381
if (estate)
23682382
{
23692383
/* Find target relation among estate's result rels */
@@ -2375,28 +2389,22 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
23752389
while (nr>0)
23762390
{
23772391
if (rInfo->ri_RelationDesc->rd_id==event->ate_relid)
2392+
{
2393+
rel=rInfo->ri_RelationDesc;
2394+
trigdesc=rInfo->ri_TrigDesc;
2395+
finfo=rInfo->ri_TrigFunctions;
2396+
instr=rInfo->ri_TrigInstrument;
2397+
locally_opened= false;
23782398
break;
2399+
}
23792400
rInfo++;
23802401
nr--;
23812402
}
2382-
if (nr <=0)/* should not happen */
2383-
elog(ERROR,"could not find relation %u among query result relations",
2384-
event->ate_relid);
2385-
rel=rInfo->ri_RelationDesc;
2386-
trigdesc=rInfo->ri_TrigDesc;
2387-
finfo=rInfo->ri_TrigFunctions;
2388-
instr=rInfo->ri_TrigInstrument;
23892403
}
2390-
else
2404+
2405+
if (locally_opened)
23912406
{
2392-
/* Hard way: we manage the resources for ourselves */
2393-
if (rel)
2394-
heap_close(rel,NoLock);
2395-
if (trigdesc)
2396-
FreeTriggerDesc(trigdesc);
2397-
if (finfo)
2398-
pfree(finfo);
2399-
Assert(instr==NULL);/* never used in this case */
2407+
/* Hard way: open target relation for ourselves */
24002408

24012409
/*
24022410
* We assume that an appropriate lock is still held by the
@@ -2421,6 +2429,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
24212429
palloc0(trigdesc->numtriggers*sizeof(FmgrInfo));
24222430

24232431
/* Never any EXPLAIN info in this case */
2432+
instr=NULL;
24242433
}
24252434
}
24262435

@@ -2471,7 +2480,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
24712480
events->tail=prev_event;
24722481

24732482
/* Release working resources */
2474-
if (!estate)
2483+
if (locally_opened)
24752484
{
24762485
if (rel)
24772486
heap_close(rel,NoLock);
@@ -2600,11 +2609,13 @@ AfterTriggerEndQuery(EState *estate)
26002609
* IMMEDIATE: all events we have decided to defer will be available for it
26012610
* to fire.
26022611
*
2612+
* We loop in case a trigger queues more events.
2613+
*
26032614
* If we find no firable events, we don't have to increment
26042615
* firing_counter.
26052616
*/
26062617
events=&afterTriggers->query_stack[afterTriggers->query_depth];
2607-
if (afterTriggerMarkEvents(events,&afterTriggers->events, true))
2618+
while (afterTriggerMarkEvents(events,&afterTriggers->events, true))
26082619
{
26092620
CommandIdfiring_id=afterTriggers->firing_counter++;
26102621

@@ -2648,7 +2659,7 @@ AfterTriggerFireDeferred(void)
26482659
ActiveSnapshot=CopySnapshot(GetTransactionSnapshot());
26492660

26502661
/*
2651-
* Run all the remaining triggers.Loop until they are all gone,justin
2662+
* Run all the remaining triggers.Loop until they are all gone, in
26522663
* case some trigger queues more for us to do.
26532664
*/
26542665
while (afterTriggerMarkEvents(events,NULL, false))
@@ -3211,7 +3222,7 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
32113222
{
32123223
AfterTriggerEventList*events=&afterTriggers->events;
32133224

3214-
if (afterTriggerMarkEvents(events,NULL, true))
3225+
while (afterTriggerMarkEvents(events,NULL, true))
32153226
{
32163227
CommandIdfiring_id=afterTriggers->firing_counter++;
32173228

‎src/backend/executor/spi.c

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.179 2007/04/27 22:05:47 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.180 2007/08/15 19:15:46 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -39,9 +39,9 @@ static void _SPI_prepare_plan(const char *src, SPIPlanPtr plan);
3939
staticint_SPI_execute_plan(SPIPlanPtrplan,
4040
Datum*Values,constchar*Nulls,
4141
Snapshotsnapshot,Snapshotcrosscheck_snapshot,
42-
boolread_only,longtcount);
42+
boolread_only,boolfire_triggers,longtcount);
4343

44-
staticint_SPI_pquery(QueryDesc*queryDesc,longtcount);
44+
staticint_SPI_pquery(QueryDesc*queryDesc,boolfire_triggers,longtcount);
4545

4646
staticvoid_SPI_error_callback(void*arg);
4747

@@ -316,7 +316,7 @@ SPI_execute(const char *src, bool read_only, long tcount)
316316

317317
res=_SPI_execute_plan(&plan,NULL,NULL,
318318
InvalidSnapshot,InvalidSnapshot,
319-
read_only,tcount);
319+
read_only,true,tcount);
320320

321321
_SPI_end_call(true);
322322
returnres;
@@ -349,7 +349,7 @@ SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls,
349349
res=_SPI_execute_plan(plan,
350350
Values,Nulls,
351351
InvalidSnapshot,InvalidSnapshot,
352-
read_only,tcount);
352+
read_only,true,tcount);
353353

354354
_SPI_end_call(true);
355355
returnres;
@@ -364,9 +364,12 @@ SPI_execp(SPIPlanPtr plan, Datum *Values, const char *Nulls, long tcount)
364364

365365
/*
366366
* SPI_execute_snapshot -- identical to SPI_execute_plan, except that we allow
367-
* the caller to specify exactly which snapshots to use. This is currently
368-
* not documented in spi.sgml because it is only intended for use by RI
369-
* triggers.
367+
* the caller to specify exactly which snapshots to use. Also, the caller
368+
* may specify that AFTER triggers should be queued as part of the outer
369+
* query rather than being fired immediately at the end of the command.
370+
*
371+
* This is currently not documented in spi.sgml because it is only intended
372+
* for use by RI triggers.
370373
*
371374
* Passing snapshot == InvalidSnapshot will select the normal behavior of
372375
* fetching a new snapshot for each query.
@@ -375,7 +378,7 @@ int
375378
SPI_execute_snapshot(SPIPlanPtrplan,
376379
Datum*Values,constchar*Nulls,
377380
Snapshotsnapshot,Snapshotcrosscheck_snapshot,
378-
boolread_only,longtcount)
381+
boolread_only,boolfire_triggers,longtcount)
379382
{
380383
intres;
381384

@@ -392,7 +395,7 @@ SPI_execute_snapshot(SPIPlanPtr plan,
392395
res=_SPI_execute_plan(plan,
393396
Values,Nulls,
394397
snapshot,crosscheck_snapshot,
395-
read_only,tcount);
398+
read_only,fire_triggers,tcount);
396399

397400
_SPI_end_call(true);
398401
returnres;
@@ -1428,12 +1431,14 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan)
14281431
*behavior of taking a new snapshot for each query.
14291432
* crosscheck_snapshot: for RI use, all others pass InvalidSnapshot
14301433
* read_only: TRUE for read-only execution (no CommandCounterIncrement)
1434+
* fire_triggers: TRUE to fire AFTER triggers at end of query (normal case);
1435+
*FALSE means any AFTER triggers are postponed to end of outer query
14311436
* tcount: execution tuple-count limit, or 0 for none
14321437
*/
14331438
staticint
14341439
_SPI_execute_plan(SPIPlanPtrplan,Datum*Values,constchar*Nulls,
14351440
Snapshotsnapshot,Snapshotcrosscheck_snapshot,
1436-
boolread_only,longtcount)
1441+
boolread_only,boolfire_triggers,longtcount)
14371442
{
14381443
volatileintmy_res=0;
14391444
volatileuint32my_processed=0;
@@ -1589,7 +1594,8 @@ _SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls,
15891594
crosscheck_snapshot,
15901595
dest,
15911596
paramLI, false);
1592-
res=_SPI_pquery(qdesc,canSetTag ?tcount :0);
1597+
res=_SPI_pquery(qdesc,fire_triggers,
1598+
canSetTag ?tcount :0);
15931599
FreeQueryDesc(qdesc);
15941600
}
15951601
else
@@ -1680,7 +1686,7 @@ _SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls,
16801686
}
16811687

16821688
staticint
1683-
_SPI_pquery(QueryDesc*queryDesc,longtcount)
1689+
_SPI_pquery(QueryDesc*queryDesc,boolfire_triggers,longtcount)
16841690
{
16851691
intoperation=queryDesc->operation;
16861692
intres;
@@ -1726,7 +1732,8 @@ _SPI_pquery(QueryDesc *queryDesc, long tcount)
17261732
ResetUsage();
17271733
#endif
17281734

1729-
AfterTriggerBeginQuery();
1735+
if (fire_triggers)
1736+
AfterTriggerBeginQuery();
17301737

17311738
ExecutorStart(queryDesc,0);
17321739

@@ -1743,7 +1750,8 @@ _SPI_pquery(QueryDesc *queryDesc, long tcount)
17431750
}
17441751

17451752
/* Take care of any queued AFTER triggers */
1746-
AfterTriggerEndQuery(queryDesc->estate);
1753+
if (fire_triggers)
1754+
AfterTriggerEndQuery(queryDesc->estate);
17471755

17481756
ExecutorEnd(queryDesc);
17491757

‎src/backend/utils/adt/ri_triggers.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*
1616
* Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
1717
*
18-
* $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.95 2007/06/05 21:31:06 tgl Exp $
18+
* $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.96 2007/08/15 19:15:46 tgl Exp $
1919
*
2020
* ----------
2121
*/
@@ -2774,7 +2774,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
27742774
NULL,NULL,
27752775
CopySnapshot(GetLatestSnapshot()),
27762776
InvalidSnapshot,
2777-
true,1);
2777+
true,false,1);
27782778

27792779
/* Check result */
27802780
if (spi_result!=SPI_OK_SELECT)
@@ -3308,7 +3308,7 @@ ri_PerformCheck(RI_QueryKey *qkey, SPIPlanPtr qplan,
33083308
spi_result=SPI_execute_snapshot(qplan,
33093309
vals,nulls,
33103310
test_snapshot,crosscheck_snapshot,
3311-
false,limit);
3311+
false,false,limit);
33123312

33133313
/* Restore UID */
33143314
SetUserId(save_uid);

‎src/include/executor/spi.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.62 2007/07/25 12:22:53 mha Exp $
9+
* $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.63 2007/08/15 19:15:46 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -104,7 +104,7 @@ extern intSPI_execute_snapshot(SPIPlanPtr plan,
104104
Datum*Values,constchar*Nulls,
105105
Snapshotsnapshot,
106106
Snapshotcrosscheck_snapshot,
107-
boolread_only,longtcount);
107+
boolread_only,boolfire_triggers,longtcount);
108108
externSPIPlanPtrSPI_prepare(constchar*src,intnargs,Oid*argtypes);
109109
externSPIPlanPtrSPI_prepare_cursor(constchar*src,intnargs,Oid*argtypes,
110110
intcursorOptions);

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

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,6 @@ SELECT * from FKTABLE;
646646
UPDATE PKTABLE set ptest2=5 where ptest2=2;
647647
ERROR: insert or update on table "fktable" violates foreign key constraint "constrname3"
648648
DETAIL: Key (ftest1,ftest2,ftest3)=(1,-1,3) is not present in table "pktable".
649-
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""
650649
-- Try to update something that will set default
651650
UPDATE PKTABLE set ptest1=0, ptest2=5, ptest3=10 where ptest2=2;
652651
UPDATE PKTABLE set ptest2=10 where ptest2=4;
@@ -1230,3 +1229,71 @@ ROLLBACK TO savept1;
12301229
COMMIT;
12311230
ERROR: insert or update on table "fktable" violates foreign key constraint "fktable_fk_fkey"
12321231
DETAIL: Key (fk)=(20) is not present in table "pktable".
1232+
-- test order of firing of FK triggers when several RI-induced changes need to
1233+
-- be made to the same row. This was broken by subtransaction-related
1234+
-- changes in 8.0.
1235+
CREATE TEMP TABLE users (
1236+
id INT PRIMARY KEY,
1237+
name VARCHAR NOT NULL
1238+
);
1239+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "users_pkey" for table "users"
1240+
INSERT INTO users VALUES (1, 'Jozko');
1241+
INSERT INTO users VALUES (2, 'Ferko');
1242+
INSERT INTO users VALUES (3, 'Samko');
1243+
CREATE TEMP TABLE tasks (
1244+
id INT PRIMARY KEY,
1245+
owner INT REFERENCES users ON UPDATE CASCADE ON DELETE SET NULL,
1246+
worker INT REFERENCES users ON UPDATE CASCADE ON DELETE SET NULL,
1247+
checked_by INT REFERENCES users ON UPDATE CASCADE ON DELETE SET NULL
1248+
);
1249+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "tasks_pkey" for table "tasks"
1250+
INSERT INTO tasks VALUES (1,1,NULL,NULL);
1251+
INSERT INTO tasks VALUES (2,2,2,NULL);
1252+
INSERT INTO tasks VALUES (3,3,3,3);
1253+
SELECT * FROM tasks;
1254+
id | owner | worker | checked_by
1255+
----+-------+--------+------------
1256+
1 | 1 | |
1257+
2 | 2 | 2 |
1258+
3 | 3 | 3 | 3
1259+
(3 rows)
1260+
1261+
UPDATE users SET id = 4 WHERE id = 3;
1262+
SELECT * FROM tasks;
1263+
id | owner | worker | checked_by
1264+
----+-------+--------+------------
1265+
1 | 1 | |
1266+
2 | 2 | 2 |
1267+
3 | 4 | 4 | 4
1268+
(3 rows)
1269+
1270+
DELETE FROM users WHERE id = 4;
1271+
SELECT * FROM tasks;
1272+
id | owner | worker | checked_by
1273+
----+-------+--------+------------
1274+
1 | 1 | |
1275+
2 | 2 | 2 |
1276+
3 | | |
1277+
(3 rows)
1278+
1279+
-- could fail with only 2 changes to make, if row was already updated
1280+
BEGIN;
1281+
UPDATE tasks set id=id WHERE id=2;
1282+
SELECT * FROM tasks;
1283+
id | owner | worker | checked_by
1284+
----+-------+--------+------------
1285+
1 | 1 | |
1286+
3 | | |
1287+
2 | 2 | 2 |
1288+
(3 rows)
1289+
1290+
DELETE FROM users WHERE id = 2;
1291+
SELECT * FROM tasks;
1292+
id | owner | worker | checked_by
1293+
----+-------+--------+------------
1294+
1 | 1 | |
1295+
3 | | |
1296+
2 | | |
1297+
(3 rows)
1298+
1299+
COMMIT;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp