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.

diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index 3ee4b06e71978e039a6dc069b059571a54261aa7..985920900710bc27239a95b1ec60e441ef715112 100644 (file)
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -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 f4c1a5642ac42467be232ec5560560c53049ab73..76769dbcbe1de5337fab0c833cdf1bd5f453d624 100644 (file)
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -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;