diff options
| -rw-r--r-- | src/backend/utils/mmgr/portalmem.c | 54 | ||||
| -rw-r--r-- | src/include/utils/portal.h | 5 | ||||
| -rw-r--r-- | src/pl/plpgsql/src/pl_exec.c | 48 |
3 files changed, 64 insertions, 43 deletions
diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c index f7b26cdd97..64b29906d9 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -12,7 +12,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.118 2010/02/26 02:01:14 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.119 2010/07/05 09:27:17 heikki Exp $ * *------------------------------------------------------------------------- */ @@ -377,6 +377,28 @@ PortalCreateHoldStore(Portal portal) } /* + * PinPortal + * Protect a portal from dropping. + */ +void +PinPortal(Portal portal) +{ + if (portal->portalPinned) + elog(ERROR, "portal already pinned"); + + portal->portalPinned = true; +} + +void +UnpinPortal(Portal portal) +{ + if (!portal->portalPinned) + elog(ERROR, "portal not pinned"); + + portal->portalPinned = false; +} + +/* * PortalDrop * Destroy the portal. */ @@ -385,9 +407,16 @@ PortalDrop(Portal portal, bool isTopCommit) { AssertArg(PortalIsValid(portal)); - /* Not sure if this case can validly happen or not... */ - if (portal->status == PORTAL_ACTIVE) - elog(ERROR, "cannot drop active portal"); + /* + * Don't allow dropping a pinned portal, it's still needed by whoever + * pinned it. Not sure if the PORTAL_ACTIVE case can validly happen or + * not... + */ + if (portal->portalPinned || + portal->status == PORTAL_ACTIVE) + ereport(ERROR, + (errcode(ERRCODE_INVALID_CURSOR_STATE), + errmsg("cannot drop active portal \"%s\"", portal->name))); /* * Remove portal from hash table. Because we do this first, we will not @@ -631,6 +660,13 @@ AtCommit_Portals(void) } /* + * There should be no pinned portals anymore. Complain if someone + * leaked one. + */ + if (portal->portalPinned) + elog(ERROR, "cannot commit while a portal is pinned"); + + /* * Do nothing to cursors held over from a previous transaction * (including holdable ones just frozen by CommitHoldablePortals). */ @@ -738,7 +774,15 @@ AtCleanup_Portals(void) continue; } - /* Else zap it. */ + /* + * If a portal is still pinned, forcibly unpin it. PortalDrop will + * not let us drop the portal otherwise. Whoever pinned the portal + * was interrupted by the abort too and won't try to use it anymore. + */ + if (portal->portalPinned) + portal->portalPinned = false; + + /* Zap it. */ PortalDrop(portal, false); } } diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h index 75101a782b..30214e68f3 100644 --- a/src/include/utils/portal.h +++ b/src/include/utils/portal.h @@ -39,7 +39,7 @@ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.82 2010/01/02 16:58:10 momjian Exp $ + * $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.83 2010/07/05 09:27:17 heikki Exp $ * *------------------------------------------------------------------------- */ @@ -133,6 +133,7 @@ typedef struct PortalData /* Status data */ PortalStatus status; /* see above */ + bool portalPinned; /* a pinned portal can't be dropped */ /* If not NULL, Executor is active; call ExecutorEnd eventually: */ QueryDesc *queryDesc; /* info needed for executor invocation */ @@ -199,6 +200,8 @@ extern void AtSubAbort_Portals(SubTransactionId mySubid, extern void AtSubCleanup_Portals(SubTransactionId mySubid); extern Portal CreatePortal(const char *name, bool allowDup, bool dupSilent); extern Portal CreateNewPortal(void); +extern void PinPortal(Portal portal); +extern void UnpinPortal(Portal portal); extern void PortalDrop(Portal portal, bool isTopCommit); extern Portal GetPortalByName(const char *name); extern void PortalDefineQuery(Portal portal, diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 3342e79150..d1af4a6e5f 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.259 2010/06/21 09:47:29 heikki Exp $ + * $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.260 2010/07/05 09:27:18 heikki Exp $ * *------------------------------------------------------------------------- */ @@ -2002,20 +2002,11 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt) rc = exec_for_query(estate, (PLpgSQL_stmt_forq *) stmt, portal, false); /* ---------- - * Close portal. The statements executed in the loop might've closed the - * cursor already, rendering our portal pointer invalid, so we mustn't - * trust the pointer. + * Close portal, and restore cursor variable if it was initially NULL. * ---------- */ - portal = SPI_cursor_find(portalname); - if (portal == NULL) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_CURSOR), - errmsg("cursor \"%s\" closed unexpectedly", - portalname))); SPI_cursor_close(portal); - /* Restore cursor variable if it was initially NULL. */ if (curname == NULL) { free_var(curvar); @@ -4278,13 +4269,6 @@ exec_run_select(PLpgSQL_execstate *estate, * exec_for_query --- execute body of FOR loop for each row from a portal * * Used by exec_stmt_fors, exec_stmt_forc and exec_stmt_dynfors - * - * If the portal is for a cursor that's visible to user code, the statements - * we execute might move or close the cursor. You must pass prefetch_ok=false - * in that case to disable optimizations that rely on the portal staying - * unchanged over execution of the user statements. - * NB: With prefetch_ok=false, the portal pointer might point to garbage - * after the call. Caller beware! */ static int exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt, @@ -4296,10 +4280,6 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt, bool found = false; int rc = PLPGSQL_RC_OK; int n; - const char *portalname; - - /* Remember portal name so that we can re-find it */ - portalname = portal->name; /* * Determine if we assign to a record or a row @@ -4312,6 +4292,12 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt, elog(ERROR, "unsupported target"); /* + * Make sure the portal doesn't get closed by the user statements + * we execute. + */ + PinPortal(portal); + + /* * Fetch the initial tuple(s). If prefetching is allowed then we grab a * few more rows to avoid multiple trips through executor startup * overhead. @@ -4408,22 +4394,8 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt, /* * Fetch more tuples. If prefetching is allowed, grab 50 at a time. - * Otherwise the statements executed in the loop might've moved or - * even closed the cursor, so check that the cursor is still open, - * and fetch only one row at a time. */ - if (prefetch_ok) - SPI_cursor_fetch(portal, true, 50); - else - { - portal = SPI_cursor_find(portalname); - if (portal == NULL) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_CURSOR), - errmsg("cursor \"%s\" closed unexpectedly", - portalname))); - SPI_cursor_fetch(portal, true, 1); - } + SPI_cursor_fetch(portal, true, prefetch_ok ? 50 : 1); tuptab = SPI_tuptable; n = SPI_processed; } @@ -4435,6 +4407,8 @@ loop_exit: */ SPI_freetuptable(tuptab); + UnpinPortal(portal); + /* * Set the FOUND variable to indicate the result of executing the loop * (namely, whether we looped one or more times). This must be set last so |