Let's get rid of SPI_push/SPI_pop - Mailing list pgsql-hackers

From Tom Lane
Subject Let's get rid of SPI_push/SPI_pop
Date
Msg-id 20808.1478481403@sss.pgh.pa.us
Whole thread Raw
Responses Re: Let's get rid of SPI_push/SPI_pop
List pgsql-hackers
The intent of SPI_push/SPI_pop seems to be to draw a boundary line between
nested layers of SPI callers.  Which is fine, but the SPI_connect and
SPI_finish calls of the inner layer would suffice for that.  AFAICS,
the only thing that SPI_push/SPI_pop buy for us is the ability to detect
a missing SPI_connect or SPI_finish in an inner function layer.  And
that seems pretty useless, because any such bug in a function would be
immediately detected in simple testing that calls it without any outer
level of SPI calls.

As against that, we have the risk of forgotten SPI_push/SPI_pop calls that
go undetected for years, as just seen in commit fc8b81a29.  We've had that
type of bug before too, cf 0d4899e44.  And then there's the fact that we
put conditional SPI_push/SPI_pop calls into various places, eg deac9488d,
which it seems to me largely destroys whatever debugging value the concept
did have.

So I think we should just delete these functions and adjust SPI_connect
and SPI_finish so that they just push/pop a context level unconditionally.
(Which will make them simpler, not more complicated.)

We can provide do-nothing macros by these names to avoid an API break
for third-party extensions.

Comments, objections?
        regards, tom lane



pgsql-hackers by date:

Previous
From: "Tsunakawa, Takayuki"
Date:
Subject: Re: Re: BUG #13755: pgwin32_is_service not checking if SECURITY_SERVICE_SID is disabled
Next
From: Franck Verrot
Date:
Subject: Re: Mention column name in error messages