Let's get rid of SPI_push/SPI_pop

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Let's get rid of SPI_push/SPI_pop
Date: 2016-11-07 01:16:43
Message-ID: 20808.1478481403@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Franck Verrot 2016-11-07 01:26:15 Re: Mention column name in error messages
Previous Message Tsunakawa, Takayuki 2016-11-07 00:49:42 Re: Re: BUG #13755: pgwin32_is_service not checking if SECURITY_SERVICE_SID is disabled