Обсуждение: portal pinning
While working on transaction control in procedures, I noticed some inconsistencies in how portal pinning is used. This mechanism was introduced in eb81b6509f4c9109ecf8839d8c482cc597270687 to prevent user code from closing cursors that PL/pgSQL has created internally, mainly for FOR loops. Otherwise, user code could just write CLOSE '<unnamed portal X>' to mess with the language internals. It seems to me that PL/Perl and PL/Python should also use that mechanism, because the same problem could happen there. (PL/Tcl does not expose any cursor functionality AFAICT.) Currently, in PL/Perl, if an internally generated cursor is closed, PL/Perl just thinks the cursor has been exhausted and silently does nothing. PL/Python comes back with a slightly bizarre error message "closing a cursor in an aborted subtransaction", which might apply in some situations but not in all. Attached is a sample patch that adds portal pinning to PL/Perl and PL/Python. But I also wonder whether we shouldn't automatically pin/unpin portals in SPI_cursor_open() and SPI_cursor_close(). This makes sense if you consider "pinned" to mean "internally generated". I don't think there is a scenario in which user code should directly operate on a portal created by SPI. Comments? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On 12/12/17 10:34, Peter Eisentraut wrote: > But I also wonder whether we shouldn't automatically pin/unpin portals > in SPI_cursor_open() and SPI_cursor_close(). This makes sense if you > consider "pinned" to mean "internally generated". I don't think there > is a scenario in which user code should directly operate on a portal > created by SPI. Here is a patch for this option. The above sentence was not quite correct. Only unnamed portals should be pinned automatically. Named portals are of course possible to be passed around as refcursors for example. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On 12/15/2017 03:36 PM, Peter Eisentraut wrote: > On 12/12/17 10:34, Peter Eisentraut wrote: >> But I also wonder whether we shouldn't automatically pin/unpin portals >> in SPI_cursor_open() and SPI_cursor_close(). This makes sense if you >> consider "pinned" to mean "internally generated". I don't think there >> is a scenario in which user code should directly operate on a portal >> created by SPI. > Here is a patch for this option. > > The above sentence was not quite correct. Only unnamed portals should > be pinned automatically. Named portals are of course possible to be > passed around as refcursors for example. > This seems like a good idea, and the code change is tiny and clean. I don't know of any third party PLs or other libraries might be pinning the portals already on their own. How would they be affected if they did? Anyway, marking ready for committer. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 1/8/18 15:27, Andrew Dunstan wrote: > This seems like a good idea, and the code change is tiny and clean. I > don't know of any third party PLs or other libraries might be pinning > the portals already on their own. How would they be affected if they did? They would get an error if they tried to pin it a second time. So this would require a small source-level adjustment. But I doubt this is actually the case anywhere, seeing that we are not even using this consistently in core. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 1/8/18 20:28, Peter Eisentraut wrote: > On 1/8/18 15:27, Andrew Dunstan wrote: >> This seems like a good idea, and the code change is tiny and clean. I >> don't know of any third party PLs or other libraries might be pinning >> the portals already on their own. How would they be affected if they did? > > They would get an error if they tried to pin it a second time. So this > would require a small source-level adjustment. But I doubt this is > actually the case anywhere, seeing that we are not even using this > consistently in core. committed -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
> committed
Here's CI log: https://travis-ci.org/pgjdbc/pgjdbc/jobs/327327402
The errors are:
testMetaData[typeName = REF_CURSOR, cursorType = 2,012](org.postgresql.test.jdbc2.RefCursorTest) Time elapsed: 0.032 sec
<<< ERROR! org.postgresql.util.PSQLException: ERROR: cannot drop pinned portal "<unnamed portal 1>"
It looks like "refcursor" expressions are somehow broken.
The test code is to execute testspg__getRefcursor procedure
that is defined as follows
CREATE OR REPLACE FUNCTION testspg__getRefcursor () RETURNS refcursor AS '
declare v_resset refcursor; begin open v_resset for select id from testrs order by id;
return v_resset; end;' LANGUAGE plpgsql;
Would you please check that?
Vladimir
On 1/10/18 13:53, Vladimir Sitnikov wrote: >> committed > > I'm afraid it causes regressions for pgjdbc. > Here's CI log: https://travis-ci.org/pgjdbc/pgjdbc/jobs/327327402 > > The errors are: > testMetaData[typeName = REF_CURSOR, cursorType = > 2,012](org.postgresql.test.jdbc2.RefCursorTest) Time elapsed: 0.032 sec > <<< ERROR! org.postgresql.util.PSQLException: ERROR: cannot drop pinned > portal "<unnamed portal 1>" > > It looks like "refcursor" expressions are somehow broken. Hmm, it seems like we are missing some test coverage in core for that. I guess I'll have to revert this patch and go with the first approach of putting it directly into PL/Perl and PL/Python. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services