I wrote:
> I tried fixing this
> http://archives.postgresql.org/pgsql-general/2009-01/msg00030.php
> by inserting SPI_push/SPI_pop calls around plperl's use of
> InputFunctionCall and OutputFunctionCall ...
> I also thought about attacking the problem by having InputFunctionCall
> and OutputFunctionCall automatically do SPI_push/SPI_pop if they are
> called within an active SPI context. I don't like this approach too
> much because it seems likely to mask bugs as often as fix them. (In
> particular I'd be afraid to back-patch such a change.) It might be the
> cleanest solution overall, though, particularly when you consider that
> we've probably got similar issues in pltcl, plpython, and add-on PLs.
I've done a trial patch along the second line, and on the whole I think
it's probably far safer than sprinkling the system with SPI_push/SPI_pop
calls. Comments?
regards, tom lane
Index: src/backend/executor/spi.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/spi.c,v
retrieving revision 1.204
diff -c -r1.204 spi.c
*** src/backend/executor/spi.c 2 Jan 2009 20:42:00 -0000 1.204
--- src/backend/executor/spi.c 6 Jan 2009 18:40:54 -0000
***************
*** 296,301 ****
--- 296,326 ----
_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: src/backend/utils/fmgr/fmgr.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/fmgr/fmgr.c,v
retrieving revision 1.124
diff -c -r1.124 fmgr.c
*** src/backend/utils/fmgr/fmgr.c 1 Jan 2009 17:23:51 -0000 1.124
--- src/backend/utils/fmgr/fmgr.c 6 Jan 2009 18:40:55 -0000
***************
*** 1846,1861 ****
--- 1851,1875 ----
* 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,1886 ****
--- 1895,1902 ----
fcinfo.flinfo->fn_oid);
}
+ SPI_pop_conditional(pushed);
+
return result;
}
***************
*** 1889,1901 ****
*
* 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.
*/
char *
OutputFunctionCall(FmgrInfo *flinfo, Datum val)
{
! return DatumGetCString(FunctionCall1(flinfo, val));
}
/*
--- 1905,1926 ----
*
* 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.
*/
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;
}
/*
***************
*** 1904,1910 ****
* "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.
*/
Datum
ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
--- 1929,1936 ----
* "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.
*/
Datum
ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
***************
*** 1912,1921 ****
--- 1938,1950 ----
{
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,1946 ****
--- 1970,1977 ----
fcinfo.flinfo->fn_oid);
}
+ SPI_pop_conditional(pushed);
+
return result;
}
***************
*** 1949,1962 ****
*
* 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.
*/
bytea *
SendFunctionCall(FmgrInfo *flinfo, Datum val)
{
! return DatumGetByteaP(FunctionCall1(flinfo, val));
}
/*
--- 1980,2003 ----
*
* Do not call this on NULL datums.
*
! * 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)
{
! bytea *result;
! bool pushed;
!
! pushed = SPI_push_conditional();
!
! result = DatumGetByteaP(FunctionCall1(flinfo, val));
!
! SPI_pop_conditional(pushed);
!
! return result;
}
/*
Index: src/include/executor/spi.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/executor/spi.h,v
retrieving revision 1.68
diff -c -r1.68 spi.h
*** src/include/executor/spi.h 1 Jan 2009 17:23:59 -0000 1.68
--- src/include/executor/spi.h 6 Jan 2009 18:40:55 -0000
***************
*** 93,98 ****
--- 93,100 ----
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: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.226
diff -c -r1.226 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c 1 Jan 2009 17:24:03 -0000 1.226
--- src/pl/plpgsql/src/pl_exec.c 6 Jan 2009 18:40:55 -0000
***************
*** 4689,4715 ****
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;
}
/* ----------
--- 4689,4699 ----
static char *
convert_value_to_string(Datum value, Oid valtype)
{
Oid typoutput;
bool typIsVarlena;
getTypeOutputInfo(valtype, &typoutput, &typIsVarlena);
! return OidOutputFunctionCall(typoutput, value);
}
/* ----------
***************
*** 4735,4759 ****
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();
}
}
--- 4719,4732 ----
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
