Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 745e6ed

Browse files
committed
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.
1 parent 953c2c9 commit 745e6ed

File tree

2 files changed

+28
-7
lines changed

2 files changed

+28
-7
lines changed

doc/src/sgml/spi.sgml

+9-6
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/spi.sgml,v 1.59 2007/09/14 04:18:27 momjian Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/spi.sgml,v 1.60 2008/02/12 04:09:44 tgl Exp $ -->
22

33
<chapter id="spi">
44
<title>Server Programming Interface</title>
@@ -1077,9 +1077,12 @@ bool SPI_is_cursor_plan(SPIPlanPtr <parameter>plan</parameter>)
10771077
<title>Return Value</title>
10781078
<para>
10791079
<symbol>true</symbol> or <symbol>false</symbol> to indicate if the
1080-
<parameter>plan</parameter> can produce a cursor or not.
1081-
If the <parameter>plan</parameter> is <symbol>NULL</symbol> or invalid,
1082-
<varname>SPI_result</varname> is set to <symbol>SPI_ERROR_ARGUMENT</symbol>
1080+
<parameter>plan</parameter> can produce a cursor or not, with
1081+
<varname>SPI_result</varname> set to zero.
1082+
If it is not possible to determine the answer (for example,
1083+
if the <parameter>plan</parameter> is <symbol>NULL</symbol> or invalid,
1084+
or if called when not connected to SPI), then
1085+
<varname>SPI_result</varname> is set to a suitable error code
10831086
and <symbol>false</symbol> is returned.
10841087
</para>
10851088
</refsect1>
@@ -1442,8 +1445,8 @@ Portal SPI_cursor_open(const char * <parameter>name</parameter>, SPIPlanPtr <par
14421445
<title>Return Value</title>
14431446

14441447
<para>
1445-
pointer to portal containing the cursor, or <symbol>NULL</symbol>
1446-
on error
1448+
Pointer to portal containing the cursor. Note there is no error
1449+
return convention; any error will be reported via <function>elog</>.
14471450
</para>
14481451
</refsect1>
14491452
</refentry>

src/backend/executor/spi.c

+19-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.187 2008/01/01 19:45:49 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.188 2008/02/12 04:09:44 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -886,6 +886,10 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan,
886886
Assert(list_length(plan->plancache_list) == 1);
887887
plansource = (CachedPlanSource *) linitial(plan->plancache_list);
888888

889+
/* Push the SPI stack */
890+
if (_SPI_begin_call(false) < 0)
891+
elog(ERROR, "SPI_cursor_open called while not connected");
892+
889893
/* Reset SPI result (note we deliberately don't touch lastoid) */
890894
SPI_processed = 0;
891895
SPI_tuptable = NULL;
@@ -1041,6 +1045,9 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan,
10411045

10421046
Assert(portal->strategy != PORTAL_MULTI_QUERY);
10431047

1048+
/* Pop the SPI stack */
1049+
_SPI_end_call(false);
1050+
10441051
/* Return the created portal */
10451052
return portal;
10461053
}
@@ -1180,16 +1187,27 @@ SPI_is_cursor_plan(SPIPlanPtr plan)
11801187
}
11811188

11821189
if (list_length(plan->plancache_list) != 1)
1190+
{
1191+
SPI_result = 0;
11831192
return false; /* not exactly 1 pre-rewrite command */
1193+
}
11841194
plansource = (CachedPlanSource *) linitial(plan->plancache_list);
11851195

1196+
/* Need _SPI_begin_call in case replanning invokes SPI-using functions */
1197+
SPI_result = _SPI_begin_call(false);
1198+
if (SPI_result < 0)
1199+
return false;
1200+
11861201
if (plan->saved)
11871202
{
11881203
/* Make sure the plan is up to date */
11891204
cplan = RevalidateCachedPlan(plansource, true);
11901205
ReleaseCachedPlan(cplan, true);
11911206
}
11921207

1208+
_SPI_end_call(false);
1209+
SPI_result = 0;
1210+
11931211
/* Does it return tuples? */
11941212
if (plansource->resultDesc)
11951213
return true;

0 commit comments

Comments
 (0)