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

Commiteb81b65

Browse files
committed
The previous fix in CVS HEAD and 8.4 for handling the case where a cursor
being used in a PL/pgSQL FOR loop is closed was inadequate, as Tom Lanepointed out. The bug affects FOR statement variants too, because you canclose an implicitly created cursor too by guessing the "<unnamed portal X>"name created for it.To fix that, "pin" the portal to prevent it from being dropped while it'sbeing used in a PL/pgSQL FOR loop. Backpatch all the way to 7.4 which isthe oldest supported version.
1 parent2330d9c commiteb81b65

File tree

3 files changed

+64
-43
lines changed

3 files changed

+64
-43
lines changed

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

Lines changed: 49 additions & 5 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.118 2010/02/26 02:01:14 momjian Exp $
15+
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.119 2010/07/05 09:27:17 heikki Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -376,6 +376,28 @@ PortalCreateHoldStore(Portal portal)
376376
MemoryContextSwitchTo(oldcxt);
377377
}
378378

379+
/*
380+
* PinPortal
381+
*Protect a portal from dropping.
382+
*/
383+
void
384+
PinPortal(Portalportal)
385+
{
386+
if (portal->portalPinned)
387+
elog(ERROR,"portal already pinned");
388+
389+
portal->portalPinned= true;
390+
}
391+
392+
void
393+
UnpinPortal(Portalportal)
394+
{
395+
if (!portal->portalPinned)
396+
elog(ERROR,"portal not pinned");
397+
398+
portal->portalPinned= false;
399+
}
400+
379401
/*
380402
* PortalDrop
381403
*Destroy the portal.
@@ -385,9 +407,16 @@ PortalDrop(Portal portal, bool isTopCommit)
385407
{
386408
AssertArg(PortalIsValid(portal));
387409

388-
/* Not sure if this case can validly happen or not... */
389-
if (portal->status==PORTAL_ACTIVE)
390-
elog(ERROR,"cannot drop active portal");
410+
/*
411+
* Don't allow dropping a pinned portal, it's still needed by whoever
412+
* pinned it. Not sure if the PORTAL_ACTIVE case can validly happen or
413+
* not...
414+
*/
415+
if (portal->portalPinned||
416+
portal->status==PORTAL_ACTIVE)
417+
ereport(ERROR,
418+
(errcode(ERRCODE_INVALID_CURSOR_STATE),
419+
errmsg("cannot drop active portal \"%s\"",portal->name)));
391420

392421
/*
393422
* Remove portal from hash table. Because we do this first, we will not
@@ -630,6 +659,13 @@ AtCommit_Portals(void)
630659
continue;
631660
}
632661

662+
/*
663+
* There should be no pinned portals anymore. Complain if someone
664+
* leaked one.
665+
*/
666+
if (portal->portalPinned)
667+
elog(ERROR,"cannot commit while a portal is pinned");
668+
633669
/*
634670
* Do nothing to cursors held over from a previous transaction
635671
* (including holdable ones just frozen by CommitHoldablePortals).
@@ -738,7 +774,15 @@ AtCleanup_Portals(void)
738774
continue;
739775
}
740776

741-
/* Else zap it. */
777+
/*
778+
* If a portal is still pinned, forcibly unpin it. PortalDrop will
779+
* not let us drop the portal otherwise. Whoever pinned the portal
780+
* was interrupted by the abort too and won't try to use it anymore.
781+
*/
782+
if (portal->portalPinned)
783+
portal->portalPinned= false;
784+
785+
/* Zap it. */
742786
PortalDrop(portal, false);
743787
}
744788
}

‎src/include/utils/portal.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
* Portions Copyright (c) 1996-2010, 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.82 2010/01/02 16:58:10 momjian Exp $
42+
* $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.83 2010/07/05 09:27:17 heikki Exp $
4343
*
4444
*-------------------------------------------------------------------------
4545
*/
@@ -133,6 +133,7 @@ typedef struct PortalData
133133

134134
/* Status data */
135135
PortalStatusstatus;/* see above */
136+
boolportalPinned;/* a pinned portal can't be dropped */
136137

137138
/* If not NULL, Executor is active; call ExecutorEnd eventually: */
138139
QueryDesc*queryDesc;/* info needed for executor invocation */
@@ -199,6 +200,8 @@ extern void AtSubAbort_Portals(SubTransactionId mySubid,
199200
externvoidAtSubCleanup_Portals(SubTransactionIdmySubid);
200201
externPortalCreatePortal(constchar*name,boolallowDup,booldupSilent);
201202
externPortalCreateNewPortal(void);
203+
externvoidPinPortal(Portalportal);
204+
externvoidUnpinPortal(Portalportal);
202205
externvoidPortalDrop(Portalportal,boolisTopCommit);
203206
externPortalGetPortalByName(constchar*name);
204207
externvoidPortalDefineQuery(Portalportal,

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 11 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.259 2010/06/21 09:47:29 heikki Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.260 2010/07/05 09:27:18 heikki Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -2002,20 +2002,11 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
20022002
rc=exec_for_query(estate, (PLpgSQL_stmt_forq*)stmt,portal, false);
20032003

20042004
/* ----------
2005-
* Close portal. The statements executed in the loop might've closed the
2006-
* cursor already, rendering our portal pointer invalid, so we mustn't
2007-
* trust the pointer.
2005+
* Close portal, and restore cursor variable if it was initially NULL.
20082006
* ----------
20092007
*/
2010-
portal=SPI_cursor_find(portalname);
2011-
if (portal==NULL)
2012-
ereport(ERROR,
2013-
(errcode(ERRCODE_UNDEFINED_CURSOR),
2014-
errmsg("cursor \"%s\" closed unexpectedly",
2015-
portalname)));
20162008
SPI_cursor_close(portal);
20172009

2018-
/* Restore cursor variable if it was initially NULL. */
20192010
if (curname==NULL)
20202011
{
20212012
free_var(curvar);
@@ -4278,13 +4269,6 @@ exec_run_select(PLpgSQL_execstate *estate,
42784269
* exec_for_query --- execute body of FOR loop for each row from a portal
42794270
*
42804271
* Used by exec_stmt_fors, exec_stmt_forc and exec_stmt_dynfors
4281-
*
4282-
* If the portal is for a cursor that's visible to user code, the statements
4283-
* we execute might move or close the cursor. You must pass prefetch_ok=false
4284-
* in that case to disable optimizations that rely on the portal staying
4285-
* unchanged over execution of the user statements.
4286-
* NB: With prefetch_ok=false, the portal pointer might point to garbage
4287-
* after the call. Caller beware!
42884272
*/
42894273
staticint
42904274
exec_for_query(PLpgSQL_execstate*estate,PLpgSQL_stmt_forq*stmt,
@@ -4296,10 +4280,6 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
42964280
boolfound= false;
42974281
intrc=PLPGSQL_RC_OK;
42984282
intn;
4299-
constchar*portalname;
4300-
4301-
/* Remember portal name so that we can re-find it */
4302-
portalname=portal->name;
43034283

43044284
/*
43054285
* Determine if we assign to a record or a row
@@ -4311,6 +4291,12 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
43114291
else
43124292
elog(ERROR,"unsupported target");
43134293

4294+
/*
4295+
* Make sure the portal doesn't get closed by the user statements
4296+
* we execute.
4297+
*/
4298+
PinPortal(portal);
4299+
43144300
/*
43154301
* Fetch the initial tuple(s).If prefetching is allowed then we grab a
43164302
* few more rows to avoid multiple trips through executor startup
@@ -4408,22 +4394,8 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
44084394

44094395
/*
44104396
* Fetch more tuples. If prefetching is allowed, grab 50 at a time.
4411-
* Otherwise the statements executed in the loop might've moved or
4412-
* even closed the cursor, so check that the cursor is still open,
4413-
* and fetch only one row at a time.
44144397
*/
4415-
if (prefetch_ok)
4416-
SPI_cursor_fetch(portal, true,50);
4417-
else
4418-
{
4419-
portal=SPI_cursor_find(portalname);
4420-
if (portal==NULL)
4421-
ereport(ERROR,
4422-
(errcode(ERRCODE_UNDEFINED_CURSOR),
4423-
errmsg("cursor \"%s\" closed unexpectedly",
4424-
portalname)));
4425-
SPI_cursor_fetch(portal, true,1);
4426-
}
4398+
SPI_cursor_fetch(portal, true,prefetch_ok ?50 :1);
44274399
tuptab=SPI_tuptable;
44284400
n=SPI_processed;
44294401
}
@@ -4435,6 +4407,8 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
44354407
*/
44364408
SPI_freetuptable(tuptab);
44374409

4410+
UnpinPortal(portal);
4411+
44384412
/*
44394413
* Set the FOUND variable to indicate the result of executing the loop
44404414
* (namely, whether we looped one or more times). This must be set last so

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp