On 22/06/10 00:47, Heikki Linnakangas wrote:
Maybe it would be easier to somehow protect the portal then, and throw
an error if you try to close it. We could just mark the portal as
PORTAL_ACTIVE while we run the user statements, but that would also
forbid fetching or moving it. I'm thinking of a new "pinned" state,
which is like PORTAL_READY except that the portal can't be dropped like
in PORTAL_ACTIVE state.
Like this.
(I'll need to revert the broken commit before applying this)
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 8ad4915..9f15772 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -723,6 +723,7 @@ PortalRun(Portal portal, long count, bool isTopLevel,
ResourceOwner saveResourceOwner;
MemoryContext savePortalContext;
MemoryContext saveMemoryContext;
+ PortalStatus readyStatus;
AssertArg(PortalIsValid(portal));
@@ -742,10 +743,12 @@ PortalRun(Portal portal, long count, bool isTopLevel,
/*
* Check for improper portal use, and mark portal active.
*/
- if (portal->status != PORTAL_READY)
+ if (portal->status != PORTAL_READY &&
+ portal->status != PORTAL_READY_NOCLOSE)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("portal \"%s\" cannot be run", portal->name)));
+ readyStatus = portal->status;
portal->status = PORTAL_ACTIVE;
/*
@@ -810,7 +813,7 @@ PortalRun(Portal portal, long count, bool isTopLevel,
}
/* Mark portal not active */
- portal->status = PORTAL_READY;
+ portal->status = readyStatus;
/*
* Since it's a forward fetch, say DONE iff atEnd is now true.
@@ -1359,16 +1362,19 @@ PortalRunFetch(Portal portal,
ResourceOwner saveResourceOwner;
MemoryContext savePortalContext;
MemoryContext oldContext;
+ PortalStatus readyStatus;
AssertArg(PortalIsValid(portal));
/*
* Check for improper portal use, and mark portal active.
*/
- if (portal->status != PORTAL_READY)
+ if (portal->status != PORTAL_READY &&
+ portal->status != PORTAL_READY_NOCLOSE)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("portal \"%s\" cannot be run", portal->name)));
+ readyStatus = portal->status;
portal->status = PORTAL_ACTIVE;
/*
@@ -1430,7 +1436,7 @@ PortalRunFetch(Portal portal,
MemoryContextSwitchTo(oldContext);
/* Mark portal not active */
- portal->status = PORTAL_READY;
+ portal->status = readyStatus;
ActivePortal = saveActivePortal;
CurrentResourceOwner = saveResourceOwner;
diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index ac62d45..28e854d 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -377,6 +377,27 @@ PortalCreateHoldStore(Portal portal)
}
/*
+ * PinPortal
+ * Protect a portal from dropping
+ */
+void
+PinPortal(Portal portal)
+{
+ if (portal->status != PORTAL_READY)
+ elog(ERROR, "cannot pin portal with status %u", portal->status);
+
+ portal->status = PORTAL_READY_NOCLOSE;
+}
+
+void
+UnpinPortal(Portal portal)
+{
+ if (portal->status != PORTAL_READY_NOCLOSE)
+ elog(ERROR, "portal not pinned");
+ portal->status = PORTAL_READY;
+}
+
+/*
* PortalDrop
* Destroy the portal.
*/
@@ -386,8 +407,11 @@ 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");
+ if (portal->status == PORTAL_ACTIVE ||
+ portal->status == PORTAL_READY_NOCLOSE)
+ 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
@@ -490,7 +514,8 @@ PortalHashTableDeleteAll(void)
{
Portal portal = hentry->portal;
- if (portal->status != PORTAL_ACTIVE)
+ if (portal->status != PORTAL_ACTIVE &&
+ portal->status != PORTAL_READY_NOCLOSE)
PortalDrop(portal, false);
}
}
@@ -631,6 +656,13 @@ AtCommit_Portals(void)
}
/*
+ * There should be no pinned portals anymore. Complain if someone
+ * leaked one.
+ */
+ if (portal->status == PORTAL_READY_NOCLOSE)
+ 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).
*/
@@ -684,7 +716,8 @@ AtAbort_Portals(void)
* created in the failed transaction. See comments in
* AtSubAbort_Portals.
*/
- if (portal->status == PORTAL_READY)
+ if (portal->status == PORTAL_READY ||
+ portal->status == PORTAL_READY_NOCLOSE)
portal->status = PORTAL_FAILED;
/* let portalcmds.c clean up the state it knows about */
@@ -807,6 +840,7 @@ AtSubAbort_Portals(SubTransactionId mySubid,
* have such references and hence may be able to continue.)
*/
if (portal->status == PORTAL_READY ||
+ portal->status == PORTAL_READY_NOCLOSE ||
portal->status == PORTAL_ACTIVE)
portal->status = PORTAL_FAILED;
diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index ce2f38a..b5dc3af 100644
--- a/src/include/utils/portal.h
+++ b/src/include/utils/portal.h
@@ -88,15 +88,17 @@ typedef enum PortalStrategy
} PortalStrategy;
/*
- * A portal is always in one of these states. It is possible to transit
- * from ACTIVE back to READY if the query is not run to completion;
- * otherwise we never back up in status.
+ * A portal is always in one of these states. It is possible to transit
+ * from ACTIVE back to previous READY/READY_NOCLOSE state if the query is
+ * not run to completion. Also, pinning/unpinning a portal moves it
+ * between READY and READY_NOCLOSE. Otherwise we never back up in status.
*/
typedef enum PortalStatus
{
PORTAL_NEW, /* freshly created */
PORTAL_DEFINED, /* PortalDefineQuery done */
PORTAL_READY, /* PortalStart complete, can run it */
+ PORTAL_READY_NOCLOSE, /* pinned -- like READY, but can't delete it */
PORTAL_ACTIVE, /* portal is running (can't delete it) */
PORTAL_DONE, /* portal is finished (don't re-run it) */
PORTAL_FAILED /* portal got error (can't re-run it) */
@@ -199,6 +201,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 0f52eae..2ceccc5 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -4290,6 +4290,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.
@@ -4399,6 +4405,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
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
