I wrote: > Pavel Stehule <pavel.steh...@gmail.com> writes: >> 2016-11-07 2:16 GMT+01:00 Tom Lane <t...@sss.pgh.pa.us>: >>> 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.)
>> cannot be there some performance impacts? > Any impact will be for the better, since we're removing logic. > I haven't gone through it in detail, but it might be as simple > as removing _SPI_curid and code that manipulates it. After studying spi.c more closely, I find that it can't be quite as simple as that. There are five functions that look at _SPI_curid to determine where they will palloc their results: SPI_copytuple SPI_returntuple SPI_modifytuple SPI_palloc SPI_datumTransfer Specifically, when used in a normal SPI execution context, they will palloc in the "savedcxt", the one that had been current when SPI_connect was called; this behavior is used for returning values out of a SPI-using function. However, when not in a SPI context, they just allocate in CurrentMemoryContext. So their behavior is different after SPI_push than before it. The majority of call sites use these functions while connected to SPI, so that it wouldn't change things if we redefined these functions to insist on being in SPI context and always use the "savedcxt". However, there are about half a dozen places that use SPI_modifytuple while not connected. Some of them never do connect at all, and some use it after disconnecting. I don't think that this discovery constitutes a reason not to get rid of SPI_push/SPI_pop. What it shows is that forgetting to call them actually has another, subtler way of causing bugs. If you forget, and call some code that is using one of these functions this way and expecting to produce a result in CurrentMemoryContext, it won't produce the result there but in the context of the caller of your SPI-using procedure. That probably means the memory will get leaked until you return, which could be quite a long time (consider a long-running plpgsql function that repeatedly calls something that acts like this). So I think that we should still get rid of SPI_push/SPI_pop, and to that end, redefine these five functions to throw error outside SPI context. To simplify adapting the places that use SPI_modifytuple outside SPI context, we could provide a function with a similar API that allocates in CurrentMemoryContext. I have not found any caller that really needs a run-time determination of which memory context to use. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers