On 12/12/17 10:34, Peter Eisentraut wrote:
> But I also wonder whether we shouldn't automatically pin/unpin portals
> in SPI_cursor_open() and SPI_cursor_close(). This makes sense if you
> consider "pinned" to mean "internally generated". I don't think there
> is a scenario in which user code should directly operate on a portal
> created by SPI.
Here is a patch for this option.
The above sentence was not quite correct. Only unnamed portals should
be pinned automatically. Named portals are of course possible to be
passed around as refcursors for example.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From f9aaa47cf4e46aac973e532a790ac2099d017523 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 15 Dec 2017 15:24:10 -0500
Subject: [PATCH] Move portal pinning from PL/pgSQL to SPI
PL/pgSQL "pins" internally generated (unnamed) portals so that user code
cannot close them by guessing their names. This logic is also useful in
other languages and really for any code. So move that logic into SPI.
An unnamed portal obtained through SPI_cursor_open() and related
functions is now automatically pinned, and SPI_cursor_close()
automatically unpins a portal that is pinned.
---
src/backend/executor/spi.c | 9 +++++++++
src/pl/plpgsql/src/pl_exec.c | 8 --------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index f3da2ddd08..1f0a07ce0b 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1175,6 +1175,12 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr
plan,
{
/* Use a random nonconflicting name */
portal = CreateNewPortal();
+
+ /*
+ * Make sure the portal doesn't get closed by the user
statements we
+ * execute.
+ */
+ PinPortal(portal);
}
else
{
@@ -1413,6 +1419,9 @@ SPI_cursor_close(Portal portal)
if (!PortalIsValid(portal))
elog(ERROR, "invalid portal in SPI cursor operation");
+ if (portal->portalPinned)
+ UnpinPortal(portal);
+
PortalDrop(portal, false);
}
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index fa4d573e50..243396abd1 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -5330,12 +5330,6 @@ exec_for_query(PLpgSQL_execstate *estate,
PLpgSQL_stmt_forq *stmt,
/* Fetch loop variable's datum entry */
var = (PLpgSQL_variable *) estate->datums[stmt->var->dno];
- /*
- * 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
@@ -5450,8 +5444,6 @@ exec_for_query(PLpgSQL_execstate *estate,
PLpgSQL_stmt_forq *stmt,
*/
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
--
2.15.1
