Insert conditional SPI_push/SPI_pop calls into InputFunctionCall,
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 7 Jan 2009 20:38:56 +0000 (20:38 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 7 Jan 2009 20:38:56 +0000 (20:38 +0000)
OutputFunctionCall, and friends.  This allows SPI-using functions to invoke
datatype I/O without concern for the possibility that a SPI-using function
will be called (which could be either the I/O function itself, or a function
used in a domain check constraint).  It's a tad ugly, but not nearly as ugly
as what'd be needed to make this work via retail insertion of push/pop
operations in all the PLs.

This reverts my patch of 2007-01-30 that inserted some retail SPI_push/pop
calls into plpgsql; that approach only fixed plpgsql, and not any other PLs.
But the other PLs have the issue too, as illustrated by a recent gripe from
Christian Schröder.

Back-patch to 8.2, which is as far back as this solution will work.  It's
also as far back as we need to worry about the domain-constraint case, since
earlier versions did not attempt to check domain constraints within datatype
input.  I'm not aware of any old I/O functions that use SPI themselves, so
this should be sufficient for a back-patch.

src/backend/executor/spi.c
src/backend/utils/fmgr/fmgr.c
src/include/executor/spi.h
src/pl/plpgsql/src/pl_exec.c

index 5391dcb6e9b483ad3fe2e7ba89dcac0159198337..aa562c2ea9a2e6a36ded2d295c540ba05f2304eb 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.205 2009/01/07 13:44:36 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.206 2009/01/07 20:38:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -304,6 +304,31 @@ 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)
index 994b65866d86591406d811681526c62ec48565ee..34777b7606bd139e1af930c44c297836cf85de62 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.124 2009/01/01 17:23:51 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.125 2009/01/07 20:38:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -19,6 +19,7 @@
 #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"
@@ -1846,16 +1847,25 @@ OidFunctionCall9(Oid functionId, 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, NULL, NULL);
 
        fcinfo.arg[0] = CStringGetDatum(str);
@@ -1881,6 +1891,8 @@ InputFunctionCall(FmgrInfo *flinfo, char *str, Oid typioparam, int32 typmod)
                                 fcinfo.flinfo->fn_oid);
        }
 
+       SPI_pop_conditional(pushed);
+
        return result;
 }
 
@@ -1889,13 +1901,22 @@ InputFunctionCall(FmgrInfo *flinfo, char *str, Oid typioparam, int32 typmod)
  *
  * Do not call this on NULL datums.
  *
- * This is mere window dressing for FunctionCall1, but its use is recommended
- * anyway so that code invoking output functions can be identified easily.
+ * This is almost just window dressing for FunctionCall1, but it includes
+ * SPI context pushing for the same reasons as InputFunctionCall.
  */
 char *
 OutputFunctionCall(FmgrInfo *flinfo, Datum val)
 {
-       return DatumGetCString(FunctionCall1(flinfo, val));
+       char       *result;
+       bool            pushed;
+
+       pushed = SPI_push_conditional();
+
+       result = DatumGetCString(FunctionCall1(flinfo, val));
+
+       SPI_pop_conditional(pushed);
+
+       return result;
 }
 
 /*
@@ -1904,7 +1925,8 @@ 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.
+ * the same as FunctionCall3.  Also, this includes SPI context pushing for
+ * the same reasons as InputFunctionCall.
  */
 Datum
 ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
@@ -1912,10 +1934,13 @@ 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, NULL, NULL);
 
        fcinfo.arg[0] = PointerGetDatum(buf);
@@ -1941,6 +1966,8 @@ ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
                                 fcinfo.flinfo->fn_oid);
        }
 
+       SPI_pop_conditional(pushed);
+
        return result;
 }
 
@@ -1949,14 +1976,24 @@ ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
  *
  * Do not call this on NULL datums.
  *
- * This is little more than window dressing for FunctionCall1, but its use is
- * recommended anyway so that code invoking output functions can be identified
- * easily.     Note however that it does guarantee a non-toasted result.
+ * 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.
  */
 bytea *
 SendFunctionCall(FmgrInfo *flinfo, Datum val)
 {
-       return DatumGetByteaP(FunctionCall1(flinfo, val));
+       bytea      *result;
+       bool            pushed;
+
+       pushed = SPI_push_conditional();
+
+       result = DatumGetByteaP(FunctionCall1(flinfo, val));
+
+       SPI_pop_conditional(pushed);
+
+       return result;
 }
 
 /*
index c28730c425bad306f5bcdd3396d3f1b5c08ffde3..66a77ce47789dbdb1b3fce1ab2a22a91d18d34a1 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.69 2009/01/07 13:44:37 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.70 2009/01/07 20:38:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -66,6 +66,8 @@ 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,
index 04eb52bdc3fc68922a162693c6635f4a7e7cd3e9..4dab908a16ae3c75681e5d73aad1b058ee383c7c 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.227 2009/01/07 13:44:37 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.228 2009/01/07 20:38:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -4690,27 +4690,11 @@ make_tuple_from_row(PLpgSQL_execstate *estate,
 static char *
 convert_value_to_string(Datum value, Oid valtype)
 {
-       char       *str;
        Oid                     typoutput;
        bool            typIsVarlena;
 
        getTypeOutputInfo(valtype, &typoutput, &typIsVarlena);
-
-       /*
-        * We do SPI_push to allow the datatype output function to use SPI.
-        * However we do not mess around with CommandCounterIncrement or advancing
-        * the snapshot, which means that a stable output function would not see
-        * updates made so far by our own function.  The use-case for such
-        * scenarios seems too narrow to justify the cycles that would be
-        * expended.
-        */
-       SPI_push();
-
-       str = OidOutputFunctionCall(typoutput, value);
-
-       SPI_pop();
-
-       return str;
+       return OidOutputFunctionCall(typoutput, value);
 }
 
 /* ----------
@@ -4736,25 +4720,14 @@ exec_cast_value(Datum value, Oid valtype,
                        char       *extval;
 
                        extval = convert_value_to_string(value, valtype);
-
-                       /* Allow input function to use SPI ... see notes above */
-                       SPI_push();
-
                        value = InputFunctionCall(reqinput, extval,
                                                                          reqtypioparam, reqtypmod);
-
-                       SPI_pop();
-
                        pfree(extval);
                }
                else
                {
-                       SPI_push();
-
                        value = InputFunctionCall(reqinput, NULL,
                                                                          reqtypioparam, reqtypmod);
-
-                       SPI_pop();
                }
        }