• Tom Lane's avatar
    Simplify code by getting rid of SPI_push, SPI_pop, SPI_restore_connection. · 1833f1a1
    Tom Lane authored
    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>
    1833f1a1
spi.h 5.83 KB