From 1833f1a1c3b0e12b3ea40d49bf11898eedae5248 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 8 Nov 2016 17:39:45 -0500 Subject: Simplify code by getting rid of SPI_push, SPI_pop, SPI_restore_connection. 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> --- src/include/executor/spi.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'src/include/executor') diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h index 1792fb12172..76ba394a2b6 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -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); -- cgit v1.2.3