Simplify code by getting rid of SPI_push, SPI_pop, SPI_restore_connection.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 8 Nov 2016 22:39:45 +0000 (17:39 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 8 Nov 2016 22:39:57 +0000 (17:39 -0500)
The idea behind SPI_push was to allow transitioning back into an
"unconnected" state when a SPI-using procedure calls unrelated code that
might or might not invoke SPI.  That sounds good, but in practice the only
thing it does for us is to catch cases where a called SPI-using function
forgets to call SPI_connect --- which is a highly improbable failure mode,
since it would be exposed immediately by direct testing of said function.
As against that, we've had multiple bugs induced by forgetting to call
SPI_push/SPI_pop around code that might invoke SPI-using functions; these
are much harder to catch and indeed have gone undetected for years in some
cases.  And we've had to band-aid around some problems of this ilk by
introducing conditional push/pop pairs in some places, which really kind
of defeats the purpose altogether; if we can't draw bright lines between
connected and unconnected code, what's the point?

Hence, get rid of SPI_push[_conditional], SPI_pop[_conditional], and the
underlying state variable _SPI_curid.  It turns out SPI_restore_connection
can go away too, which is a nice side benefit since it was never more than
a kluge.  Provide no-op macros for the deleted functions so as to avoid an
API break for external modules.

A side effect of this removal is that SPI_palloc and allied functions no
longer permit being called when unconnected; they'll throw an error
instead.  The apparent usefulness of the previous behavior was a mirage
as well, because it was depended on by only a few places (which I fixed in
preceding commits), and it posed a risk of allocations being unexpectedly
long-lived if someone forgot a SPI_push call.

Discussion: <20808.1478481403@sss.pgh.pa.us>

12 files changed:
doc/src/sgml/spi.sgml
src/backend/executor/spi.c
src/backend/utils/adt/xml.c
src/backend/utils/cache/plancache.c
src/backend/utils/fmgr/fmgr.c
src/include/executor/spi.h
src/pl/plperl/plperl.c
src/pl/plpgsql/src/pl_exec.c
src/pl/plpython/plpy_exec.c
src/pl/plpython/plpy_spi.c
src/pl/plpython/plpy_subxactobject.c
src/pl/tcl/pltcl.c

index 39133c90385f68e80eaef3ce5c9b79c09d704eef..836ce0822f3edfe78e632a3b848f0d381e86f689 100644 (file)
@@ -90,21 +90,6 @@ int SPI_connect(void)
    function if you want to execute commands through SPI.  Some utility
    SPI functions can be called from unconnected procedures.
   </para>
-
-  <para>
-   If your procedure is already connected,
-   <function>SPI_connect</function> will return the error code
-   <returnvalue>SPI_ERROR_CONNECT</returnvalue>.  This could happen if
-   a procedure that has called <function>SPI_connect</function>
-   directly calls another procedure that calls
-   <function>SPI_connect</function>.  While recursive calls to the
-   <acronym>SPI</acronym> manager are permitted when an SQL command
-   called through SPI invokes another function that uses
-   <acronym>SPI</acronym>, directly nested calls to
-   <function>SPI_connect</function> and
-   <function>SPI_finish</function> are forbidden.
-   (But see <function>SPI_push</function> and <function>SPI_pop</function>.)
-  </para>
  </refsect1>
 
  <refsect1>
@@ -164,13 +149,6 @@ int SPI_finish(void)
    abort the transaction via <literal>elog(ERROR)</literal>.  In that
    case SPI will clean itself up automatically.
   </para>
-
-  <para>
-   If <function>SPI_finish</function> is called without having a valid
-   connection, it will return <symbol>SPI_ERROR_UNCONNECTED</symbol>.
-   There is no fundamental problem with this; it means that the SPI
-   manager has nothing to do.
-  </para>
  </refsect1>
 
  <refsect1>
@@ -200,86 +178,6 @@ int SPI_finish(void)
 
 <!-- *********************************************** -->
 
-<refentry id="spi-spi-push">
- <indexterm><primary>SPI_push</primary></indexterm>
-
- <refmeta>
-  <refentrytitle>SPI_push</refentrytitle>
-  <manvolnum>3</manvolnum>
- </refmeta>
-
- <refnamediv>
-  <refname>SPI_push</refname>
-  <refpurpose>push SPI stack to allow recursive SPI usage</refpurpose>
- </refnamediv>
-
- <refsynopsisdiv>
-<synopsis>
-void SPI_push(void)
-</synopsis>
- </refsynopsisdiv>
-
- <refsect1>
-  <title>Description</title>
-
-  <para>
-   <function>SPI_push</function> should be called before executing another
-   procedure that might itself wish to use SPI.
-   After <function>SPI_push</function>, SPI is no longer in a
-   <quote>connected</> state, and SPI function calls will be rejected unless
-   a fresh <function>SPI_connect</function> is done.  This ensures a clean
-   separation between your procedure's SPI state and that of another procedure
-   you call.  After the other procedure returns, call
-   <function>SPI_pop</function> to restore access to your own SPI state.
-  </para>
-
-  <para>
-   Note that <function>SPI_execute</function> and related functions
-   automatically do the equivalent of <function>SPI_push</function> before
-   passing control back to the SQL execution engine, so it is not necessary
-   for you to worry about this when using those functions.
-   Only when you are directly calling arbitrary code that might contain
-   <function>SPI_connect</function> calls do you need to issue
-   <function>SPI_push</function> and <function>SPI_pop</function>.
-  </para>
- </refsect1>
-
-</refentry>
-
-<!-- *********************************************** -->
-
-<refentry id="spi-spi-pop">
- <indexterm><primary>SPI_pop</primary></indexterm>
-
- <refmeta>
-  <refentrytitle>SPI_pop</refentrytitle>
-  <manvolnum>3</manvolnum>
- </refmeta>
-
- <refnamediv>
-  <refname>SPI_pop</refname>
-  <refpurpose>pop SPI stack to return from recursive SPI usage</refpurpose>
- </refnamediv>
-
- <refsynopsisdiv>
-<synopsis>
-void SPI_pop(void)
-</synopsis>
- </refsynopsisdiv>
-
- <refsect1>
-  <title>Description</title>
-
-  <para>
-   <function>SPI_pop</function> pops the previous environment from the
-   SPI call stack.  See <function>SPI_push</function>.
-  </para>
- </refsect1>
-
-</refentry>
-
-<!-- *********************************************** -->
-
 <refentry id="spi-spi-execute">
  <indexterm><primary>SPI_execute</primary></indexterm>
 
@@ -3361,17 +3259,8 @@ char * SPI_getnspname(Relation <parameter>rel</parameter>)
    <quote>upper executor context</quote>, that is, the memory context
    that was current when <function>SPI_connect</function> was called,
    which is precisely the right context for a value returned from your
-   procedure.
-  </para>
-
-  <para>
-   If <function>SPI_palloc</function> is called while the procedure is
-   not connected to SPI, then it acts the same as a normal
-   <function>palloc</function>.  Before a procedure connects to the
-   SPI manager, the current memory context is the upper executor
-   context, so all allocations made by the procedure via
-   <function>palloc</function> or by SPI utility functions are made in
-   this context.
+   procedure.  Several of the other utility procedures described in
+   this section also return objects created in the upper executor context.
   </para>
 
   <para>
@@ -3379,25 +3268,14 @@ char * SPI_getnspname(Relation <parameter>rel</parameter>)
    context of the procedure, which is created by
    <function>SPI_connect</function>, is made the current context.  All
    allocations made by <function>palloc</function>,
-   <function>repalloc</function>, or SPI utility functions (except for
-   <function>SPI_copytuple</function>,
-   <function>SPI_returntuple</function>,
-   <function>SPI_modifytuple</function>,
-   <function>SPI_palloc</function>, and
-   <function>SPI_datumTransfer</function>) are made in this context.  When a
+   <function>repalloc</function>, or SPI utility functions (except as
+   described in this section) are made in this context.  When a
    procedure disconnects from the SPI manager (via
    <function>SPI_finish</function>) the current context is restored to
    the upper executor context, and all allocations made in the
    procedure memory context are freed and cannot be used any more.
   </para>
 
-  <para>
-   All functions described in this section can be used by both
-   connected and unconnected procedures.  In an unconnected procedure,
-   they act the same as the underlying ordinary server functions
-   (<function>palloc</>, etc.).
-  </para>
-
 <!-- *********************************************** -->
 
 <refentry id="spi-spi-palloc">
@@ -3426,6 +3304,11 @@ void * SPI_palloc(Size <parameter>size</parameter>)
    <function>SPI_palloc</function> allocates memory in the upper
    executor context.
   </para>
+
+  <para>
+   This function can only be used while connected to SPI.
+   Otherwise, it throws an error.
+  </para>
  </refsect1>
 
  <refsect1>
@@ -3605,6 +3488,12 @@ HeapTuple SPI_copytuple(HeapTuple <parameter>row</parameter>)
    row from a trigger.  In a function declared to return a composite
    type, use <function>SPI_returntuple</function> instead.
   </para>
+
+  <para>
+   This function can only be used while connected to SPI.
+   Otherwise, it returns NULL and sets <varname>SPI_result</varname> to
+   <symbol>SPI_ERROR_UNCONNECTED</symbol>.
+  </para>
  </refsect1>
 
  <refsect1>
@@ -3626,8 +3515,8 @@ HeapTuple SPI_copytuple(HeapTuple <parameter>row</parameter>)
   <title>Return Value</title>
 
   <para>
-   the copied row; <symbol>NULL</symbol> only if
-   <parameter>tuple</parameter> is <symbol>NULL</symbol>
+   the copied row, or <symbol>NULL</symbol> on error
+   (see <varname>SPI_result</varname> for an error indication)
   </para>
  </refsect1>
 </refentry>
@@ -3663,6 +3552,12 @@ HeapTupleHeader SPI_returntuple(HeapTuple <parameter>row</parameter>, TupleDesc
    before returning.
   </para>
 
+  <para>
+   This function can only be used while connected to SPI.
+   Otherwise, it returns NULL and sets <varname>SPI_result</varname> to
+   <symbol>SPI_ERROR_UNCONNECTED</symbol>.
+  </para>
+
   <para>
    Note that this should be used for functions that are declared to return
    composite types.  It is not used for triggers; use
@@ -3699,10 +3594,9 @@ HeapTupleHeader SPI_returntuple(HeapTuple <parameter>row</parameter>, TupleDesc
   <title>Return Value</title>
 
   <para>
-   <type>HeapTupleHeader</type> pointing to copied row;
-   <symbol>NULL</symbol> only if
-   <parameter>row</parameter> or <parameter>rowdesc</parameter> is
-   <symbol>NULL</symbol>
+   <type>HeapTupleHeader</type> pointing to copied row,
+   or <symbol>NULL</symbol> on error
+   (see <varname>SPI_result</varname> for an error indication)
   </para>
  </refsect1>
 </refentry>
@@ -3736,6 +3630,13 @@ HeapTuple SPI_modifytuple(Relation <parameter>rel</parameter>, HeapTuple <parame
    <function>SPI_modifytuple</function> creates a new row by
    substituting new values for selected columns, copying the original
    row's columns at other positions.  The input row is not modified.
+   The new row is returned in the upper executor context.
+  </para>
+
+  <para>
+   This function can only be used while connected to SPI.
+   Otherwise, it returns NULL and sets <varname>SPI_result</varname> to
+   <symbol>SPI_ERROR_UNCONNECTED</symbol>.
   </para>
  </refsect1>
 
@@ -3821,8 +3722,8 @@ HeapTuple SPI_modifytuple(Relation <parameter>rel</parameter>, HeapTuple <parame
 
   <para>
    new row with modifications, allocated in the upper executor
-   context; <symbol>NULL</symbol> only if <parameter>row</parameter>
-   is <symbol>NULL</symbol>
+   context, or <symbol>NULL</symbol> on error
+   (see <varname>SPI_result</varname> for an error indication)
   </para>
 
   <para>
@@ -3845,11 +3746,20 @@ HeapTuple SPI_modifytuple(Relation <parameter>rel</parameter>, HeapTuple <parame
      <listitem>
       <para>
        if <parameter>colnum</> contains an invalid column number (less
-       than or equal to 0 or greater than the number of column in
+       than or equal to 0 or greater than the number of columns in
        <parameter>row</>)
       </para>
      </listitem>
     </varlistentry>
+
+    <varlistentry>
+     <term><symbol>SPI_ERROR_UNCONNECTED</symbol></term>
+     <listitem>
+      <para>
+       if SPI is not active
+      </para>
+     </listitem>
+    </varlistentry>
    </variablelist>
   </para>
  </refsect1>
index 8e650bc4123d10d0b81521664fecdb914114001a..80fc4c47253b246c86f09406e26b4364786f302d 100644 (file)
@@ -44,8 +44,7 @@ int                   SPI_result;
 static _SPI_connection *_SPI_stack = NULL;
 static _SPI_connection *_SPI_current = NULL;
 static int     _SPI_stack_depth = 0;           /* allocated size of _SPI_stack */
-static int     _SPI_connected = -1;
-static int     _SPI_curid = -1;
+static int     _SPI_connected = -1;    /* current stack index */
 
 static Portal SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
                                                 ParamListInfo paramLI, bool read_only);
@@ -86,13 +85,7 @@ SPI_connect(void)
 {
        int                     newdepth;
 
-       /*
-        * When procedure called by Executor _SPI_curid expected to be equal to
-        * _SPI_connected
-        */
-       if (_SPI_curid != _SPI_connected)
-               return SPI_ERROR_CONNECT;
-
+       /* Enlarge stack if necessary */
        if (_SPI_stack == NULL)
        {
                if (_SPI_connected != -1 || _SPI_stack_depth != 0)
@@ -117,9 +110,7 @@ SPI_connect(void)
                }
        }
 
-       /*
-        * We're entering procedure where _SPI_curid == _SPI_connected - 1
-        */
+       /* Enter new stack level */
        _SPI_connected++;
        Assert(_SPI_connected >= 0 && _SPI_connected < _SPI_stack_depth);
 
@@ -178,14 +169,9 @@ SPI_finish(void)
        SPI_lastoid = InvalidOid;
        SPI_tuptable = NULL;
 
-       /*
-        * After _SPI_begin_call _SPI_connected == _SPI_curid. Now we are closing
-        * connection to SPI and returning to upper Executor and so _SPI_connected
-        * must be equal to _SPI_curid.
-        */
+       /* Exit stack level */
        _SPI_connected--;
-       _SPI_curid--;
-       if (_SPI_connected == -1)
+       if (_SPI_connected < 0)
                _SPI_current = NULL;
        else
                _SPI_current = &(_SPI_stack[_SPI_connected]);
@@ -212,7 +198,7 @@ AtEOXact_SPI(bool isCommit)
 
        _SPI_current = _SPI_stack = NULL;
        _SPI_stack_depth = 0;
-       _SPI_connected = _SPI_curid = -1;
+       _SPI_connected = -1;
        SPI_processed = 0;
        SPI_lastoid = InvalidOid;
        SPI_tuptable = NULL;
@@ -258,8 +244,7 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
                 * be already gone.
                 */
                _SPI_connected--;
-               _SPI_curid = _SPI_connected;
-               if (_SPI_connected == -1)
+               if (_SPI_connected < 0)
                        _SPI_current = NULL;
                else
                        _SPI_current = &(_SPI_stack[_SPI_connected]);
@@ -313,53 +298,6 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
 }
 
 
-/* Pushes SPI stack to allow recursive SPI calls */
-void
-SPI_push(void)
-{
-       _SPI_curid++;
-}
-
-/* Pops SPI stack to allow recursive SPI calls */
-void
-SPI_pop(void)
-{
-       _SPI_curid--;
-}
-
-/* Conditional push: push only if we're inside a SPI procedure */
-bool
-SPI_push_conditional(void)
-{
-       bool            pushed = (_SPI_curid != _SPI_connected);
-
-       if (pushed)
-       {
-               _SPI_curid++;
-               /* We should now be in a state where SPI_connect would succeed */
-               Assert(_SPI_curid == _SPI_connected);
-       }
-       return pushed;
-}
-
-/* Conditional pop: pop only if SPI_push_conditional pushed */
-void
-SPI_pop_conditional(bool pushed)
-{
-       /* We should be in a state where SPI_connect would succeed */
-       Assert(_SPI_curid == _SPI_connected);
-       if (pushed)
-               _SPI_curid--;
-}
-
-/* Restore state of SPI stack after aborting a subtransaction */
-void
-SPI_restore_connection(void)
-{
-       Assert(_SPI_connected >= 0);
-       _SPI_curid = _SPI_connected - 1;
-}
-
 /* Parse, plan, and execute a query string */
 int
 SPI_execute(const char *src, bool read_only, long tcount)
@@ -691,7 +629,7 @@ SPI_freeplan(SPIPlanPtr plan)
 HeapTuple
 SPI_copytuple(HeapTuple tuple)
 {
-       MemoryContext oldcxt = NULL;
+       MemoryContext oldcxt;
        HeapTuple       ctuple;
 
        if (tuple == NULL)
@@ -700,17 +638,17 @@ SPI_copytuple(HeapTuple tuple)
                return NULL;
        }
 
-       if (_SPI_curid + 1 == _SPI_connected)           /* connected */
+       if (_SPI_current == NULL)
        {
-               if (_SPI_current != &(_SPI_stack[_SPI_curid + 1]))
-                       elog(ERROR, "SPI stack corrupted");
-               oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
+               SPI_result = SPI_ERROR_UNCONNECTED;
+               return NULL;
        }
 
+       oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
+
        ctuple = heap_copytuple(tuple);
 
-       if (oldcxt)
-               MemoryContextSwitchTo(oldcxt);
+       MemoryContextSwitchTo(oldcxt);
 
        return ctuple;
 }
@@ -718,7 +656,7 @@ SPI_copytuple(HeapTuple tuple)
 HeapTupleHeader
 SPI_returntuple(HeapTuple tuple, TupleDesc tupdesc)
 {
-       MemoryContext oldcxt = NULL;
+       MemoryContext oldcxt;
        HeapTupleHeader dtup;
 
        if (tuple == NULL || tupdesc == NULL)
@@ -727,22 +665,22 @@ SPI_returntuple(HeapTuple tuple, TupleDesc tupdesc)
                return NULL;
        }
 
+       if (_SPI_current == NULL)
+       {
+               SPI_result = SPI_ERROR_UNCONNECTED;
+               return NULL;
+       }
+
        /* For RECORD results, make sure a typmod has been assigned */
        if (tupdesc->tdtypeid == RECORDOID &&
                tupdesc->tdtypmod < 0)
                assign_record_type_typmod(tupdesc);
 
-       if (_SPI_curid + 1 == _SPI_connected)           /* connected */
-       {
-               if (_SPI_current != &(_SPI_stack[_SPI_curid + 1]))
-                       elog(ERROR, "SPI stack corrupted");
-               oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
-       }
+       oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
 
        dtup = DatumGetHeapTupleHeader(heap_copy_tuple_as_datum(tuple, tupdesc));
 
-       if (oldcxt)
-               MemoryContextSwitchTo(oldcxt);
+       MemoryContextSwitchTo(oldcxt);
 
        return dtup;
 }
@@ -751,7 +689,7 @@ HeapTuple
 SPI_modifytuple(Relation rel, HeapTuple tuple, int natts, int *attnum,
                                Datum *Values, const char *Nulls)
 {
-       MemoryContext oldcxt = NULL;
+       MemoryContext oldcxt;
        HeapTuple       mtuple;
        int                     numberOfAttributes;
        Datum      *v;
@@ -764,13 +702,16 @@ SPI_modifytuple(Relation rel, HeapTuple tuple, int natts, int *attnum,
                return NULL;
        }
 
-       if (_SPI_curid + 1 == _SPI_connected)           /* connected */
+       if (_SPI_current == NULL)
        {
-               if (_SPI_current != &(_SPI_stack[_SPI_curid + 1]))
-                       elog(ERROR, "SPI stack corrupted");
-               oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
+               SPI_result = SPI_ERROR_UNCONNECTED;
+               return NULL;
        }
+
+       oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
+
        SPI_result = 0;
+
        numberOfAttributes = rel->rd_att->natts;
        v = (Datum *) palloc(numberOfAttributes * sizeof(Datum));
        n = (bool *) palloc(numberOfAttributes * sizeof(bool));
@@ -810,8 +751,7 @@ SPI_modifytuple(Relation rel, HeapTuple tuple, int natts, int *attnum,
        pfree(v);
        pfree(n);
 
-       if (oldcxt)
-               MemoryContextSwitchTo(oldcxt);
+       MemoryContextSwitchTo(oldcxt);
 
        return mtuple;
 }
@@ -980,22 +920,10 @@ SPI_getnspname(Relation rel)
 void *
 SPI_palloc(Size size)
 {
-       MemoryContext oldcxt = NULL;
-       void       *pointer;
-
-       if (_SPI_curid + 1 == _SPI_connected)           /* connected */
-       {
-               if (_SPI_current != &(_SPI_stack[_SPI_curid + 1]))
-                       elog(ERROR, "SPI stack corrupted");
-               oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
-       }
-
-       pointer = palloc(size);
-
-       if (oldcxt)
-               MemoryContextSwitchTo(oldcxt);
+       if (_SPI_current == NULL)
+               elog(ERROR, "SPI_palloc called while not connected to SPI");
 
-       return pointer;
+       return MemoryContextAlloc(_SPI_current->savedcxt, size);
 }
 
 void *
@@ -1015,20 +943,17 @@ SPI_pfree(void *pointer)
 Datum
 SPI_datumTransfer(Datum value, bool typByVal, int typLen)
 {
-       MemoryContext oldcxt = NULL;
+       MemoryContext oldcxt;
        Datum           result;
 
-       if (_SPI_curid + 1 == _SPI_connected)           /* connected */
-       {
-               if (_SPI_current != &(_SPI_stack[_SPI_curid + 1]))
-                       elog(ERROR, "SPI stack corrupted");
-               oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
-       }
+       if (_SPI_current == NULL)
+               elog(ERROR, "SPI_datumTransfer called while not connected to SPI");
+
+       oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
 
        result = datumTransfer(value, typByVal, typLen);
 
-       if (oldcxt)
-               MemoryContextSwitchTo(oldcxt);
+       MemoryContextSwitchTo(oldcxt);
 
        return result;
 }
@@ -1050,17 +975,12 @@ SPI_freetuptable(SPITupleTable *tuptable)
                return;
 
        /*
-        * Since this function might be called during error recovery, it seems
-        * best not to insist that the caller be actively connected.  We just
-        * search the topmost SPI context, connected or not.
+        * Search only the topmost SPI context for a matching tuple table.
         */
-       if (_SPI_connected >= 0)
+       if (_SPI_current != NULL)
        {
                slist_mutable_iter siter;
 
-               if (_SPI_current != &(_SPI_stack[_SPI_connected]))
-                       elog(ERROR, "SPI stack corrupted");
-
                /* find tuptable in active list, then remove it */
                slist_foreach_modify(siter, &_SPI_current->tuptables)
                {
@@ -1168,13 +1088,9 @@ SPI_cursor_open_with_args(const char *name,
 
        /* We needn't copy the plan; SPI_cursor_open_internal will do so */
 
-       /* Adjust stack so that SPI_cursor_open_internal doesn't complain */
-       _SPI_curid--;
-
        result = SPI_cursor_open_internal(name, &plan, paramLI, read_only);
 
        /* And clean up */
-       _SPI_curid++;
        _SPI_end_call(true);
 
        return result;
@@ -1723,14 +1639,8 @@ spi_dest_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
        MemoryContext oldcxt;
        MemoryContext tuptabcxt;
 
-       /*
-        * When called by Executor _SPI_curid expected to be equal to
-        * _SPI_connected
-        */
-       if (_SPI_curid != _SPI_connected || _SPI_connected < 0)
-               elog(ERROR, "improper call to spi_dest_startup");
-       if (_SPI_current != &(_SPI_stack[_SPI_curid]))
-               elog(ERROR, "SPI stack corrupted");
+       if (_SPI_current == NULL)
+               elog(ERROR, "spi_dest_startup called while not connected to SPI");
 
        if (_SPI_current->tuptable != NULL)
                elog(ERROR, "improper call to spi_dest_startup");
@@ -1775,14 +1685,8 @@ spi_printtup(TupleTableSlot *slot, DestReceiver *self)
        SPITupleTable *tuptable;
        MemoryContext oldcxt;
 
-       /*
-        * When called by Executor _SPI_curid expected to be equal to
-        * _SPI_connected
-        */
-       if (_SPI_curid != _SPI_connected || _SPI_connected < 0)
-               elog(ERROR, "improper call to spi_printtup");
-       if (_SPI_current != &(_SPI_stack[_SPI_curid]))
-               elog(ERROR, "SPI stack corrupted");
+       if (_SPI_current == NULL)
+               elog(ERROR, "spi_printtup called while not connected to SPI");
 
        tuptable = _SPI_current->tuptable;
        if (tuptable == NULL)
@@ -2534,11 +2438,8 @@ _SPI_procmem(void)
 static int
 _SPI_begin_call(bool execmem)
 {
-       if (_SPI_curid + 1 != _SPI_connected)
+       if (_SPI_current == NULL)
                return SPI_ERROR_UNCONNECTED;
-       _SPI_curid++;
-       if (_SPI_current != &(_SPI_stack[_SPI_curid]))
-               elog(ERROR, "SPI stack corrupted");
 
        if (execmem)                            /* switch to the Executor memory context */
                _SPI_execmem();
@@ -2554,11 +2455,6 @@ _SPI_begin_call(bool execmem)
 static int
 _SPI_end_call(bool procmem)
 {
-       /*
-        * We're returning to procedure where _SPI_curid == _SPI_connected - 1
-        */
-       _SPI_curid--;
-
        if (procmem)                            /* switch to the procedure memory context */
        {
                _SPI_procmem();
index b144920ec65d02d00312b5e2621b6f1d7b6013e3..057c3bfd7c11c82935a54f9d388d30e08eebe21c 100644 (file)
@@ -2644,8 +2644,6 @@ schema_to_xml_internal(Oid nspid, const char *xmlschema, bool nulls,
 
        relid_list = schema_get_xml_visible_tables(nspid);
 
-       SPI_push();
-
        foreach(cell, relid_list)
        {
                Oid                     relid = lfirst_oid(cell);
@@ -2658,7 +2656,6 @@ schema_to_xml_internal(Oid nspid, const char *xmlschema, bool nulls,
                appendStringInfoChar(result, '\n');
        }
 
-       SPI_pop();
        SPI_finish();
 
        xmldata_root_element_end(result, xmlsn);
@@ -2822,8 +2819,6 @@ database_to_xml_internal(const char *xmlschema, bool nulls,
 
        nspid_list = database_get_xml_visible_schemas();
 
-       SPI_push();
-
        foreach(cell, nspid_list)
        {
                Oid                     nspid = lfirst_oid(cell);
@@ -2836,7 +2831,6 @@ database_to_xml_internal(const char *xmlschema, bool nulls,
                appendStringInfoChar(result, '\n');
        }
 
-       SPI_pop();
        SPI_finish();
 
        xmldata_root_element_end(result, xmlcn);
index c96a86500adc46920c07470f8eef15016c6cee7c..884cdab7024eadf5d5bf16ec95819af00c91e835 100644 (file)
@@ -53,7 +53,6 @@
 #include "access/transam.h"
 #include "catalog/namespace.h"
 #include "executor/executor.h"
-#include "executor/spi.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/cost.h"
@@ -878,7 +877,6 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
        CachedPlan *plan;
        List       *plist;
        bool            snapshot_set;
-       bool            spi_pushed;
        bool            is_transient;
        MemoryContext plan_context;
        MemoryContext oldcxt = CurrentMemoryContext;
@@ -926,22 +924,11 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
                snapshot_set = true;
        }
 
-       /*
-        * The planner may try to call SPI-using functions, which causes a problem
-        * if we're already inside one.  Rather than expect all SPI-using code to
-        * do SPI_push whenever a replan could happen, it seems best to take care
-        * of the case here.
-        */
-       spi_pushed = SPI_push_conditional();
-
        /*
         * Generate the plan.
         */
        plist = pg_plan_queries(qlist, plansource->cursor_options, boundParams);
 
-       /* Clean up SPI state */
-       SPI_pop_conditional(spi_pushed);
-
        /* Release snapshot if we got one */
        if (snapshot_set)
                PopActiveSnapshot();
index 46a55ba7b953b4786dd2ba07b57019cc86031e33..3340b17d908a2808eada989d05fbb97c56b91c01 100644 (file)
@@ -19,7 +19,6 @@
 #include "catalog/pg_language.h"
 #include "catalog/pg_proc.h"
 #include "executor/functions.h"
-#include "executor/spi.h"
 #include "lib/stringinfo.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
@@ -1878,25 +1877,16 @@ OidFunctionCall9Coll(Oid functionId, Oid collation, Datum arg1, Datum arg2,
  * the caller should assume the result is NULL, but we'll call the input
  * function anyway if it's not strict.  So this is almost but not quite
  * the same as FunctionCall3.
- *
- * One important difference from the bare function call is that we will
- * push any active SPI context, allowing SPI-using I/O functions to be
- * called from other SPI functions without extra notation.  This is a hack,
- * but the alternative of expecting all SPI functions to do SPI_push/SPI_pop
- * around I/O calls seems worse.
  */
 Datum
 InputFunctionCall(FmgrInfo *flinfo, char *str, Oid typioparam, int32 typmod)
 {
        FunctionCallInfoData fcinfo;
        Datum           result;
-       bool            pushed;
 
        if (str == NULL && flinfo->fn_strict)
                return (Datum) 0;               /* just return null result */
 
-       pushed = SPI_push_conditional();
-
        InitFunctionCallInfoData(fcinfo, flinfo, 3, InvalidOid, NULL, NULL);
 
        fcinfo.arg[0] = CStringGetDatum(str);
@@ -1922,8 +1912,6 @@ InputFunctionCall(FmgrInfo *flinfo, char *str, Oid typioparam, int32 typmod)
                                 fcinfo.flinfo->fn_oid);
        }
 
-       SPI_pop_conditional(pushed);
-
        return result;
 }
 
@@ -1932,22 +1920,12 @@ InputFunctionCall(FmgrInfo *flinfo, char *str, Oid typioparam, int32 typmod)
  *
  * Do not call this on NULL datums.
  *
- * This is almost just window dressing for FunctionCall1, but it includes
- * SPI context pushing for the same reasons as InputFunctionCall.
+ * This is currently little more than window dressing for FunctionCall1.
  */
 char *
 OutputFunctionCall(FmgrInfo *flinfo, Datum val)
 {
-       char       *result;
-       bool            pushed;
-
-       pushed = SPI_push_conditional();
-
-       result = DatumGetCString(FunctionCall1(flinfo, val));
-
-       SPI_pop_conditional(pushed);
-
-       return result;
+       return DatumGetCString(FunctionCall1(flinfo, val));
 }
 
 /*
@@ -1956,8 +1934,7 @@ OutputFunctionCall(FmgrInfo *flinfo, Datum val)
  * "buf" may be NULL to indicate we are reading a NULL.  In this case
  * the caller should assume the result is NULL, but we'll call the receive
  * function anyway if it's not strict.  So this is almost but not quite
- * the same as FunctionCall3.  Also, this includes SPI context pushing for
- * the same reasons as InputFunctionCall.
+ * the same as FunctionCall3.
  */
 Datum
 ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
@@ -1965,13 +1942,10 @@ ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
 {
        FunctionCallInfoData fcinfo;
        Datum           result;
-       bool            pushed;
 
        if (buf == NULL && flinfo->fn_strict)
                return (Datum) 0;               /* just return null result */
 
-       pushed = SPI_push_conditional();
-
        InitFunctionCallInfoData(fcinfo, flinfo, 3, InvalidOid, NULL, NULL);
 
        fcinfo.arg[0] = PointerGetDatum(buf);
@@ -1997,8 +1971,6 @@ ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
                                 fcinfo.flinfo->fn_oid);
        }
 
-       SPI_pop_conditional(pushed);
-
        return result;
 }
 
@@ -2009,22 +1981,12 @@ ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
  *
  * This is little more than window dressing for FunctionCall1, but it does
  * guarantee a non-toasted result, which strictly speaking the underlying
- * function doesn't.  Also, this includes SPI context pushing for the same
- * reasons as InputFunctionCall.
+ * function doesn't.
  */
 bytea *
 SendFunctionCall(FmgrInfo *flinfo, Datum val)
 {
-       bytea      *result;
-       bool            pushed;
-
-       pushed = SPI_push_conditional();
-
-       result = DatumGetByteaP(FunctionCall1(flinfo, val));
-
-       SPI_pop_conditional(pushed);
-
-       return result;
+       return DatumGetByteaP(FunctionCall1(flinfo, val));
 }
 
 /*
index 1792fb12172c4e1c4d2107a7157ebb569ee7f240..76ba394a2b658ab304b71612841bf32ce51d1d45 100644 (file)
@@ -59,6 +59,13 @@ typedef struct _SPI_plan *SPIPlanPtr;
 #define SPI_OK_UPDATE_RETURNING 13
 #define SPI_OK_REWRITTEN               14
 
+/* These used to be functions, now just no-ops for backwards compatibility */
+#define SPI_push()     ((void) 0)
+#define SPI_pop()      ((void) 0)
+#define SPI_push_conditional() false
+#define SPI_pop_conditional(pushed) ((void) 0)
+#define SPI_restore_connection()       ((void) 0)
+
 extern PGDLLIMPORT uint64 SPI_processed;
 extern PGDLLIMPORT Oid SPI_lastoid;
 extern PGDLLIMPORT SPITupleTable *SPI_tuptable;
@@ -66,11 +73,6 @@ extern PGDLLIMPORT int SPI_result;
 
 extern int     SPI_connect(void);
 extern int     SPI_finish(void);
-extern void SPI_push(void);
-extern void SPI_pop(void);
-extern bool SPI_push_conditional(void);
-extern void SPI_pop_conditional(bool pushed);
-extern void SPI_restore_connection(void);
 extern int     SPI_execute(const char *src, bool read_only, long tcount);
 extern int SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls,
                                 bool read_only, long tcount);
index 461986cda31ea0b4e1eb7ad5e08889bc8e2a079d..9a2d0527f81bb0ed0166fb699cefd311fa842757 100644 (file)
@@ -3057,12 +3057,6 @@ plperl_spi_exec(char *query, int limit)
                ReleaseCurrentSubTransaction();
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
-
-               /*
-                * AtEOSubXact_SPI() should not have popped any SPI context, but just
-                * in case it did, make sure we remain connected.
-                */
-               SPI_restore_connection();
        }
        PG_CATCH();
        {
@@ -3078,13 +3072,6 @@ plperl_spi_exec(char *query, int limit)
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
 
-               /*
-                * If AtEOSubXact_SPI() popped any SPI context of the subxact, it will
-                * have left us in a disconnected state.  We need this hack to return
-                * to connected state.
-                */
-               SPI_restore_connection();
-
                /* Punt the error to Perl */
                croak_cstr(edata->message);
 
@@ -3296,12 +3283,6 @@ plperl_spi_query(char *query)
                ReleaseCurrentSubTransaction();
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
-
-               /*
-                * AtEOSubXact_SPI() should not have popped any SPI context, but just
-                * in case it did, make sure we remain connected.
-                */
-               SPI_restore_connection();
        }
        PG_CATCH();
        {
@@ -3317,13 +3298,6 @@ plperl_spi_query(char *query)
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
 
-               /*
-                * If AtEOSubXact_SPI() popped any SPI context of the subxact, it will
-                * have left us in a disconnected state.  We need this hack to return
-                * to connected state.
-                */
-               SPI_restore_connection();
-
                /* Punt the error to Perl */
                croak_cstr(edata->message);
 
@@ -3382,12 +3356,6 @@ plperl_spi_fetchrow(char *cursor)
                ReleaseCurrentSubTransaction();
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
-
-               /*
-                * AtEOSubXact_SPI() should not have popped any SPI context, but just
-                * in case it did, make sure we remain connected.
-                */
-               SPI_restore_connection();
        }
        PG_CATCH();
        {
@@ -3403,13 +3371,6 @@ plperl_spi_fetchrow(char *cursor)
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
 
-               /*
-                * If AtEOSubXact_SPI() popped any SPI context of the subxact, it will
-                * have left us in a disconnected state.  We need this hack to return
-                * to connected state.
-                */
-               SPI_restore_connection();
-
                /* Punt the error to Perl */
                croak_cstr(edata->message);
 
@@ -3543,12 +3504,6 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
                ReleaseCurrentSubTransaction();
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
-
-               /*
-                * AtEOSubXact_SPI() should not have popped any SPI context, but just
-                * in case it did, make sure we remain connected.
-                */
-               SPI_restore_connection();
        }
        PG_CATCH();
        {
@@ -3574,13 +3529,6 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
 
-               /*
-                * If AtEOSubXact_SPI() popped any SPI context of the subxact, it will
-                * have left us in a disconnected state.  We need this hack to return
-                * to connected state.
-                */
-               SPI_restore_connection();
-
                /* Punt the error to Perl */
                croak_cstr(edata->message);
 
@@ -3694,12 +3642,6 @@ plperl_spi_exec_prepared(char *query, HV *attr, int argc, SV **argv)
                ReleaseCurrentSubTransaction();
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
-
-               /*
-                * AtEOSubXact_SPI() should not have popped any SPI context, but just
-                * in case it did, make sure we remain connected.
-                */
-               SPI_restore_connection();
        }
        PG_CATCH();
        {
@@ -3715,13 +3657,6 @@ plperl_spi_exec_prepared(char *query, HV *attr, int argc, SV **argv)
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
 
-               /*
-                * If AtEOSubXact_SPI() popped any SPI context of the subxact, it will
-                * have left us in a disconnected state.  We need this hack to return
-                * to connected state.
-                */
-               SPI_restore_connection();
-
                /* Punt the error to Perl */
                croak_cstr(edata->message);
 
@@ -3823,12 +3758,6 @@ plperl_spi_query_prepared(char *query, int argc, SV **argv)
                ReleaseCurrentSubTransaction();
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
-
-               /*
-                * AtEOSubXact_SPI() should not have popped any SPI context, but just
-                * in case it did, make sure we remain connected.
-                */
-               SPI_restore_connection();
        }
        PG_CATCH();
        {
@@ -3844,13 +3773,6 @@ plperl_spi_query_prepared(char *query, int argc, SV **argv)
                MemoryContextSwitchTo(oldcontext);
                CurrentResourceOwner = oldowner;
 
-               /*
-                * If AtEOSubXact_SPI() popped any SPI context of the subxact, it will
-                * have left us in a disconnected state.  We need this hack to return
-                * to connected state.
-                */
-               SPI_restore_connection();
-
                /* Punt the error to Perl */
                croak_cstr(edata->message);
 
index 91e1f8dd3fd8918c4ef6ee5e37a2911199ec874e..77e7440002334102e1c836019315bba37500a469 100644 (file)
@@ -1337,12 +1337,6 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
                         * automatically cleaned up during subxact exit.)
                         */
                        estate->eval_econtext = old_eval_econtext;
-
-                       /*
-                        * AtEOSubXact_SPI() should not have popped any SPI context, but
-                        * just in case it did, make sure we remain connected.
-                        */
-                       SPI_restore_connection();
                }
                PG_CATCH();
                {
@@ -1384,13 +1378,6 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
                        /* Revert to outer eval_econtext */
                        estate->eval_econtext = old_eval_econtext;
 
-                       /*
-                        * If AtEOSubXact_SPI() popped any SPI context of the subxact, it
-                        * will have left us in a disconnected state.  We need this hack
-                        * to return to connected state.
-                        */
-                       SPI_restore_connection();
-
                        /*
                         * Must clean up the econtext too.  However, any tuple table made
                         * in the subxact will have been thrown away by SPI during subxact
@@ -5587,8 +5574,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
         * Without this, stable functions within the expression would fail to see
         * updates made so far by our own function.
         */
-       SPI_push();
-
        oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));
        if (!estate->readonly_func)
        {
@@ -5636,8 +5621,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
 
        MemoryContextSwitchTo(oldcontext);
 
-       SPI_pop();
-
        /*
         * Now we can release our refcount on the cached plan.
         */
@@ -6281,8 +6264,6 @@ exec_cast_value(PLpgSQL_execstate *estate,
                        ExprContext *econtext = estate->eval_econtext;
                        MemoryContext oldcontext;
 
-                       SPI_push();
-
                        oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));
 
                        econtext->caseValue_datum = value;
@@ -6296,8 +6277,6 @@ exec_cast_value(PLpgSQL_execstate *estate,
                        cast_entry->cast_in_use = false;
 
                        MemoryContextSwitchTo(oldcontext);
-
-                       SPI_pop();
                }
        }
 
index fa5b25a5fade10047fba63df98a433cbedc96caf..697a0e1cc03396d279de20dc664379cdae565350 100644 (file)
@@ -1103,8 +1103,6 @@ PLy_abort_open_subtransactions(int save_subxact_level)
 
                RollbackAndReleaseCurrentSubTransaction();
 
-               SPI_restore_connection();
-
                subtransactiondata = (PLySubtransactionData *) linitial(explicit_subtransactions);
                explicit_subtransactions = list_delete_first(explicit_subtransactions);
 
index b082d017ea942df6d530d846dfa170fa7ffdc57b..07ab6a087e531ccfaab13301d618cf0a8c8c7670 100644 (file)
@@ -516,12 +516,6 @@ PLy_spi_subtransaction_commit(MemoryContext oldcontext, ResourceOwner oldowner)
        ReleaseCurrentSubTransaction();
        MemoryContextSwitchTo(oldcontext);
        CurrentResourceOwner = oldowner;
-
-       /*
-        * AtEOSubXact_SPI() should not have popped any SPI context, but just in
-        * case it did, make sure we remain connected.
-        */
-       SPI_restore_connection();
 }
 
 void
@@ -541,13 +535,6 @@ PLy_spi_subtransaction_abort(MemoryContext oldcontext, ResourceOwner oldowner)
        MemoryContextSwitchTo(oldcontext);
        CurrentResourceOwner = oldowner;
 
-       /*
-        * If AtEOSubXact_SPI() popped any SPI context of the subxact, it will
-        * have left us in a disconnected state.  We need this hack to return to
-        * connected state.
-        */
-       SPI_restore_connection();
-
        /* Look up the correct exception */
        entry = hash_search(PLy_spi_exceptions, &(edata->sqlerrcode),
                                                HASH_FIND, NULL);
index 81fb3a3a4ab8761a2f79a467223afa8dcfd5aad0..9f1caa87d9411e6f56cbc3f5a48a4b105346f4fd 100644 (file)
@@ -7,7 +7,6 @@
 #include "postgres.h"
 
 #include "access/xact.h"
-#include "executor/spi.h"
 #include "utils/memutils.h"
 
 #include "plpython.h"
@@ -213,12 +212,6 @@ PLy_subtransaction_exit(PyObject *self, PyObject *args)
        CurrentResourceOwner = subxactdata->oldowner;
        pfree(subxactdata);
 
-       /*
-        * AtEOSubXact_SPI() should not have popped any SPI context, but just in
-        * case it did, make sure we remain connected.
-        */
-       SPI_restore_connection();
-
        Py_INCREF(Py_None);
        return Py_None;
 }
index 20809102efbe7400158dc791b3910a093c533dfe..b0d9e419bb92439f0c49e31eaa3f06dc6b890999 100644 (file)
@@ -2182,11 +2182,9 @@ pltcl_returnnext(ClientData cdata, Tcl_Interp *interp,
                {
                        HeapTuple       tuple;
 
-                       SPI_push();
                        tuple = pltcl_build_tuple_result(interp, rowObjv, rowObjc,
                                                                                         call_state);
                        tuplestore_puttuple(call_state->tuple_store, tuple);
-                       SPI_pop();
                }
        }
        else
@@ -2249,12 +2247,6 @@ pltcl_subtrans_commit(MemoryContext oldcontext, ResourceOwner oldowner)
        ReleaseCurrentSubTransaction();
        MemoryContextSwitchTo(oldcontext);
        CurrentResourceOwner = oldowner;
-
-       /*
-        * AtEOSubXact_SPI() should not have popped any SPI context, but just in
-        * case it did, make sure we remain connected.
-        */
-       SPI_restore_connection();
 }
 
 static void
@@ -2273,13 +2265,6 @@ pltcl_subtrans_abort(Tcl_Interp *interp,
        MemoryContextSwitchTo(oldcontext);
        CurrentResourceOwner = oldowner;
 
-       /*
-        * If AtEOSubXact_SPI() popped any SPI context of the subxact, it will
-        * have left us in a disconnected state.  We need this hack to return to
-        * connected state.
-        */
-       SPI_restore_connection();
-
        /* Pass the error data to Tcl */
        pltcl_construct_errorCode(interp, edata);
        UTF_BEGIN;
@@ -3029,9 +3014,6 @@ pltcl_build_tuple_argument(HeapTuple tuple, TupleDesc tupdesc)
  * mess, there's no way to prevent the datatype input functions it calls
  * from leaking.  Run it in a short-lived context, unless we're about to
  * exit the procedure anyway.
- *
- * Also, caller is responsible for doing SPI_push/SPI_pop if calling from
- * inside SPI environment.
  **********************************************************************/
 static HeapTuple
 pltcl_build_tuple_result(Tcl_Interp *interp, Tcl_Obj **kvObjv, int kvObjc,