From deac9488d3715408665cac707c4994e96af8b535 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 7 Jan 2009 20:38:56 +0000 Subject: [PATCH] Insert conditional SPI_push/SPI_pop calls into InputFunctionCall, 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. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 27 ++++++++++++++++- src/backend/utils/fmgr/fmgr.c | 55 +++++++++++++++++++++++++++++------ src/include/executor/spi.h | 4 ++- src/pl/plpgsql/src/pl_exec.c | 31 ++------------------ 4 files changed, 77 insertions(+), 40 deletions(-) diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 5391dcb6e9..aa562c2ea9 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -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) diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c index 994b65866d..34777b7606 100644 --- a/src/backend/utils/fmgr/fmgr.c +++ b/src/backend/utils/fmgr/fmgr.c @@ -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; } /* diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h index c28730c425..66a77ce477 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -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, diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 04eb52bdc3..4dab908a16 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -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(); } }