From 745e6edaaeaccb92a2a5efe44b49c8bcc7d0ce01 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 12 Feb 2008 04:09:44 +0000
Subject: [PATCH] Fix SPI_cursor_open() and SPI_is_cursor_plan() to push the
SPI stack before doing anything interesting, such as calling
RevalidateCachedPlan(). The necessity of this is demonstrated by an example
from Willem Buitendyk: during a replan, the planner might try to evaluate
SPI-using functions, and so we'd better be in a clean SPI context.
A small downside of this fix is that these two functions will now fail
outright if called when not inside a SPI-using procedure (ie, a
SPI_connect/SPI_finish pair). The documentation never promised or suggested
that that would work, though; and they are normally used in concert with
other functions, mainly SPI_prepare, that always have failed in such a case.
So the odds of breaking something seem pretty low.
In passing, make SPI_is_cursor_plan's error handling convention clearer,
and fix documentation's erroneous claim that SPI_cursor_open would
return NULL on error.
Before 8.3 these functions could not invoke replanning, so there is probably
no need for back-patching.
---
doc/src/sgml/spi.sgml | 15 +++++++++------
src/backend/executor/spi.c | 20 +++++++++++++++++++-
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index fca48caec04..d488c203d3d 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/spi.sgml,v 1.59 2007/09/14 04:18:27 momjian Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/spi.sgml,v 1.60 2008/02/12 04:09:44 tgl Exp $ -->
<chapter id="spi">
<title>Server Programming Interface</title>
@@ -1077,9 +1077,12 @@ bool SPI_is_cursor_plan(SPIPlanPtr <parameter>plan</parameter>)
<title>Return Value</title>
<para>
<symbol>true</symbol> or <symbol>false</symbol> to indicate if the
- <parameter>plan</parameter> can produce a cursor or not.
- If the <parameter>plan</parameter> is <symbol>NULL</symbol> or invalid,
- <varname>SPI_result</varname> is set to <symbol>SPI_ERROR_ARGUMENT</symbol>
+ <parameter>plan</parameter> can produce a cursor or not, with
+ <varname>SPI_result</varname> set to zero.
+ If it is not possible to determine the answer (for example,
+ if the <parameter>plan</parameter> is <symbol>NULL</symbol> or invalid,
+ or if called when not connected to SPI), then
+ <varname>SPI_result</varname> is set to a suitable error code
and <symbol>false</symbol> is returned.
</para>
</refsect1>
@@ -1442,8 +1445,8 @@ Portal SPI_cursor_open(const char * <parameter>name</parameter>, SPIPlanPtr <par
<title>Return Value</title>
<para>
- pointer to portal containing the cursor, or <symbol>NULL</symbol>
- on error
+ Pointer to portal containing the cursor. Note there is no error
+ return convention; any error will be reported via <function>elog</>.
</para>
</refsect1>
</refentry>
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index ae083f7225d..a23c4d017a8 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.187 2008/01/01 19:45:49 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.188 2008/02/12 04:09:44 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -886,6 +886,10 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan,
Assert(list_length(plan->plancache_list) == 1);
plansource = (CachedPlanSource *) linitial(plan->plancache_list);
+ /* Push the SPI stack */
+ if (_SPI_begin_call(false) < 0)
+ elog(ERROR, "SPI_cursor_open called while not connected");
+
/* Reset SPI result (note we deliberately don't touch lastoid) */
SPI_processed = 0;
SPI_tuptable = NULL;
@@ -1041,6 +1045,9 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan,
Assert(portal->strategy != PORTAL_MULTI_QUERY);
+ /* Pop the SPI stack */
+ _SPI_end_call(false);
+
/* Return the created portal */
return portal;
}
@@ -1180,9 +1187,17 @@ SPI_is_cursor_plan(SPIPlanPtr plan)
}
if (list_length(plan->plancache_list) != 1)
+ {
+ SPI_result = 0;
return false; /* not exactly 1 pre-rewrite command */
+ }
plansource = (CachedPlanSource *) linitial(plan->plancache_list);
+ /* Need _SPI_begin_call in case replanning invokes SPI-using functions */
+ SPI_result = _SPI_begin_call(false);
+ if (SPI_result < 0)
+ return false;
+
if (plan->saved)
{
/* Make sure the plan is up to date */
@@ -1190,6 +1205,9 @@ SPI_is_cursor_plan(SPIPlanPtr plan)
ReleaseCachedPlan(cplan, true);
}
+ _SPI_end_call(false);
+ SPI_result = 0;
+
/* Does it return tuples? */
if (plansource->resultDesc)
return true;
--
GitLab