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

Commitc3294f1

Browse files
committed
Fix interaction between materializing holdable cursors and firing
deferred triggers: either one can create more work for the other,so we have to loop till it's all gone. Per example from andrew@supernews.Add a regression test to help spot trouble in this area in future.
1 parent0c400f1 commitc3294f1

File tree

7 files changed

+187
-70
lines changed

7 files changed

+187
-70
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.198 2005/03/28 01:50:33 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.199 2005/04/11 19:51:14 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -1439,16 +1439,32 @@ CommitTransaction(void)
14391439
/*
14401440
* Do pre-commit processing (most of this stuff requires database
14411441
* access, and in fact could still cause an error...)
1442+
*
1443+
* It is possible for CommitHoldablePortals to invoke functions that
1444+
* queue deferred triggers, and it's also possible that triggers create
1445+
* holdable cursors. So we have to loop until there's nothing left to
1446+
* do.
14421447
*/
1448+
for (;;)
1449+
{
1450+
/*
1451+
* Fire all currently pending deferred triggers.
1452+
*/
1453+
AfterTriggerFireDeferred();
14431454

1444-
/*
1445-
* Tell the trigger manager that this transaction is about to be
1446-
* committed. He'll invoke all trigger deferred until XACT before we
1447-
* really start on committing the transaction.
1448-
*/
1449-
AfterTriggerEndXact();
1455+
/*
1456+
* Convert any open holdable cursors into static portals. If there
1457+
* weren't any, we are done ... otherwise loop back to check if they
1458+
* queued deferred triggers. Lather, rinse, repeat.
1459+
*/
1460+
if (!CommitHoldablePortals())
1461+
break;
1462+
}
1463+
1464+
/* Now we can shut down the deferred-trigger manager */
1465+
AfterTriggerEndXact(true);
14501466

1451-
/* Close open cursors */
1467+
/* Closeanyopen regular cursors */
14521468
AtCommit_Portals();
14531469

14541470
/*
@@ -1649,7 +1665,7 @@ AbortTransaction(void)
16491665
/*
16501666
* do abort processing
16511667
*/
1652-
AfterTriggerAbortXact();
1668+
AfterTriggerEndXact(false);
16531669
AtAbort_Portals();
16541670
AtEOXact_LargeObject(false);/* 'false' means it's abort */
16551671
AtAbort_Notify();

‎src/backend/commands/trigger.c

Lines changed: 20 additions & 24 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.183 2005/03/29 03:01:30 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.184 2005/04/11 19:51:15 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -2441,14 +2441,18 @@ AfterTriggerEndQuery(EState *estate)
24412441

24422442

24432443
/* ----------
2444-
*AfterTriggerEndXact()
2444+
*AfterTriggerFireDeferred()
24452445
*
24462446
*Called just before the current transaction is committed. At this
2447-
*time we invoke all DEFERRED triggers and tidy up.
2447+
*time we invoke all pending DEFERRED triggers.
2448+
*
2449+
*It is possible for other modules to queue additional deferred triggers
2450+
*during pre-commit processing; therefore xact.c may have to call this
2451+
*multiple times.
24482452
* ----------
24492453
*/
24502454
void
2451-
AfterTriggerEndXact(void)
2455+
AfterTriggerFireDeferred(void)
24522456
{
24532457
AfterTriggerEventList*events;
24542458

@@ -2463,49 +2467,41 @@ AfterTriggerEndXact(void)
24632467
* for them to use. (Since PortalRunUtility doesn't set a snap for
24642468
* COMMIT, we can't assume ActiveSnapshot is valid on entry.)
24652469
*/
2466-
if (afterTriggers->events.head!=NULL)
2470+
events=&afterTriggers->events;
2471+
if (events->head!=NULL)
24672472
ActiveSnapshot=CopySnapshot(GetTransactionSnapshot());
24682473

24692474
/*
24702475
* Run all the remaining triggers. Loop until they are all gone,
24712476
* just in case some trigger queues more for us to do.
24722477
*/
2473-
events=&afterTriggers->events;
24742478
while (afterTriggerMarkEvents(events,NULL, false))
24752479
{
24762480
CommandIdfiring_id=afterTriggers->firing_counter++;
24772481

24782482
afterTriggerInvokeEvents(events,firing_id,NULL, true);
24792483
}
24802484

2481-
/*
2482-
* Forget everything we know about AFTER triggers.
2483-
*
2484-
* Since all the info is in TopTransactionContext or children thereof, we
2485-
* need do nothing special to reclaim memory.
2486-
*/
2487-
afterTriggers=NULL;
2485+
Assert(events->head==NULL);
24882486
}
24892487

24902488

24912489
/* ----------
2492-
* AfterTriggerAbortXact()
2490+
* AfterTriggerEndXact()
2491+
*
2492+
*The current transaction is finishing.
24932493
*
2494-
*The current transaction has entered the abort state.
2495-
*All outstanding triggers are canceled so we simply throw
2494+
*Any unfired triggers are canceled so we simply throw
24962495
*away anything we know.
2496+
*
2497+
*Note: it is possible for this to be called repeatedly in case of
2498+
*error during transaction abort; therefore, do not complain if
2499+
*already closed down.
24972500
* ----------
24982501
*/
24992502
void
2500-
AfterTriggerAbortXact(void)
2503+
AfterTriggerEndXact(boolisCommit)
25012504
{
2502-
/*
2503-
* Ignore call if we aren't in a transaction. (Need this to survive
2504-
* repeat call in case of error during transaction abort.)
2505-
*/
2506-
if (afterTriggers==NULL)
2507-
return;
2508-
25092505
/*
25102506
* Forget everything we know about AFTER triggers.
25112507
*

‎src/backend/utils/mmgr/portalmem.c

Lines changed: 59 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* Portions Copyright (c) 1994, Regents of the University of California
1313
*
1414
* IDENTIFICATION
15-
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.77 2005/01/26 23:20:21 tgl Exp $
15+
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.78 2005/04/11 19:51:15 tgl Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -416,18 +416,14 @@ DropDependentPortals(MemoryContext queryContext)
416416
*
417417
* Any holdable cursors created in this transaction need to be converted to
418418
* materialized form, since we are going to close down the executor and
419-
* release locks. Remove all other portals created in this transaction.
420-
* Portals remaining from prior transactions should be left untouched.
419+
* release locks. Other portals are not touched yet.
421420
*
422-
* XXX This assumes that portals can be deleted in a random order, ie,
423-
* no portal has a reference to any other (at least not one that will be
424-
* exercised during deletion).I think this is okay at the moment, but
425-
* we've had bugs of that ilk in the past. Keep a close eye on cursor
426-
* references...
421+
* Returns TRUE if any holdable cursors were processed, FALSE if not.
427422
*/
428-
void
429-
AtCommit_Portals(void)
423+
bool
424+
CommitHoldablePortals(void)
430425
{
426+
boolresult= false;
431427
HASH_SEQ_STATUSstatus;
432428
PortalHashEnt*hentry;
433429

@@ -437,27 +433,9 @@ AtCommit_Portals(void)
437433
{
438434
Portalportal=hentry->portal;
439435

440-
/*
441-
* Do not touch active portals --- this can only happen in the
442-
* case of a multi-transaction utility command, such as VACUUM.
443-
*
444-
* Note however that any resource owner attached to such a portal is
445-
* still going to go away, so don't leave a dangling pointer.
446-
*/
447-
if (portal->status==PORTAL_ACTIVE)
448-
{
449-
portal->resowner=NULL;
450-
continue;
451-
}
452-
453-
/*
454-
* Do nothing else to cursors held over from a previous
455-
* transaction.
456-
*/
457-
if (portal->createSubid==InvalidSubTransactionId)
458-
continue;
459-
436+
/* Is it a holdable portal created in the current xact? */
460437
if ((portal->cursorOptions&CURSOR_OPT_HOLD)&&
438+
portal->createSubid!=InvalidSubTransactionId&&
461439
portal->status==PORTAL_READY)
462440
{
463441
/*
@@ -484,12 +462,60 @@ AtCommit_Portals(void)
484462
* as not belonging to this transaction.
485463
*/
486464
portal->createSubid=InvalidSubTransactionId;
465+
466+
result= true;
487467
}
488-
else
468+
}
469+
470+
returnresult;
471+
}
472+
473+
/*
474+
* Pre-commit processing for portals.
475+
*
476+
* Remove all non-holdable portals created in this transaction.
477+
* Portals remaining from prior transactions should be left untouched.
478+
*
479+
* XXX This assumes that portals can be deleted in a random order, ie,
480+
* no portal has a reference to any other (at least not one that will be
481+
* exercised during deletion).I think this is okay at the moment, but
482+
* we've had bugs of that ilk in the past. Keep a close eye on cursor
483+
* references...
484+
*/
485+
void
486+
AtCommit_Portals(void)
487+
{
488+
HASH_SEQ_STATUSstatus;
489+
PortalHashEnt*hentry;
490+
491+
hash_seq_init(&status,PortalHashTable);
492+
493+
while ((hentry= (PortalHashEnt*)hash_seq_search(&status))!=NULL)
494+
{
495+
Portalportal=hentry->portal;
496+
497+
/*
498+
* Do not touch active portals --- this can only happen in the
499+
* case of a multi-transaction utility command, such as VACUUM.
500+
*
501+
* Note however that any resource owner attached to such a portal is
502+
* still going to go away, so don't leave a dangling pointer.
503+
*/
504+
if (portal->status==PORTAL_ACTIVE)
489505
{
490-
/* Zap all non-holdable portals */
491-
PortalDrop(portal, true);
506+
portal->resowner=NULL;
507+
continue;
492508
}
509+
510+
/*
511+
* Do nothing to cursors held over from a previous transaction
512+
* (including holdable ones just frozen by CommitHoldablePortals).
513+
*/
514+
if (portal->createSubid==InvalidSubTransactionId)
515+
continue;
516+
517+
/* Zap all non-holdable portals */
518+
PortalDrop(portal, true);
493519
}
494520
}
495521

‎src/include/commands/trigger.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.52 2005/03/25 21:57:59 tgl Exp $
9+
* $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.53 2005/04/11 19:51:15 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -157,8 +157,8 @@ extern void ExecARUpdateTriggers(EState *estate,
157157
externvoidAfterTriggerBeginXact(void);
158158
externvoidAfterTriggerBeginQuery(void);
159159
externvoidAfterTriggerEndQuery(EState*estate);
160-
externvoidAfterTriggerEndXact(void);
161-
externvoidAfterTriggerAbortXact(void);
160+
externvoidAfterTriggerFireDeferred(void);
161+
externvoidAfterTriggerEndXact(boolisCommit);
162162
externvoidAfterTriggerBeginSubXact(void);
163163
externvoidAfterTriggerEndSubXact(boolisCommit);
164164

‎src/include/utils/portal.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
4040
* Portions Copyright (c) 1994, Regents of the University of California
4141
*
42-
* $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.54 2004/12/31 22:03:46 pgsql Exp $
42+
* $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.55 2005/04/11 19:51:16 tgl Exp $
4343
*
4444
*-------------------------------------------------------------------------
4545
*/
@@ -182,6 +182,7 @@ typedef struct PortalData
182182

183183
/* Prototypes for functions in utils/mmgr/portalmem.c */
184184
externvoidEnablePortalManager(void);
185+
externboolCommitHoldablePortals(void);
185186
externvoidAtCommit_Portals(void);
186187
externvoidAtAbort_Portals(void);
187188
externvoidAtCleanup_Portals(void);

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,3 +773,38 @@ FETCH ALL FROM c;
773773
(15 rows)
774774

775775
ROLLBACK;
776+
--
777+
-- Test behavior of both volatile and stable functions inside a cursor;
778+
-- in particular we want to see what happens during commit of a holdable
779+
-- cursor
780+
--
781+
create temp table tt1(f1 int);
782+
create function count_tt1_v() returns int8 as
783+
'select count(*) from tt1' language sql volatile;
784+
create function count_tt1_s() returns int8 as
785+
'select count(*) from tt1' language sql stable;
786+
begin;
787+
insert into tt1 values(1);
788+
declare c1 cursor for select count_tt1_v(), count_tt1_s();
789+
insert into tt1 values(2);
790+
fetch all from c1;
791+
count_tt1_v | count_tt1_s
792+
-------------+-------------
793+
2 | 1
794+
(1 row)
795+
796+
rollback;
797+
begin;
798+
insert into tt1 values(1);
799+
declare c2 cursor with hold for select count_tt1_v(), count_tt1_s();
800+
insert into tt1 values(2);
801+
commit;
802+
delete from tt1;
803+
fetch all from c2;
804+
count_tt1_v | count_tt1_s
805+
-------------+-------------
806+
2 | 1
807+
(1 row)
808+
809+
drop function count_tt1_v();
810+
drop function count_tt1_s();

0 commit comments

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