Re: plpython SPI cursors

On 11-11-23 01:58 PM, Jan Urbański wrote: <blockquote cite="mid:4ECD426F.5060702@wulczer.org" type="cite">On 20/11/11
19:14,Steve Singer wrote: <br /><blockquote type="cite">On 11-10-15 07:28 PM, Jan Urbański wrote: <br /><blockquote
type="cite">Hi,<br /><br /> attached is a patch implementing the usage of SPI cursors in PL/Python. <br /> Currently
whentrying to process a large table in PL/Python you have <br /> slurp it all into memory (that's what plpy.execute
does).<br /><br /> J <br /></blockquote><br /> I found a few bugs (see my testing section below) that will need fixing
<br/> + a few questions about the code <br /></blockquote><br /> Responding now to all questions and attaching a
revisedpatch based on your comments. <br /><br /></blockquote><br /> I've looked over the revised version of the patch
andit now seems fine.<br /><br /> Ready for committer.<br /><br /><br /><br /><blockquote
cite="mid:4ECD426F.5060702@wulczer.org"type="cite"><blockquote type="cite">Do we like the name plpy.cursor or would we
rathercall it something like <br /> plpy.execute_cursor(...) or plpy.cursor_open(...) or <br /> plpy.create_cursor(...)
<br/> Since we will be mostly stuck with the API once we release 9.2 this is <br /> worth <br /> some opinions on. I
likecursor() but if anyone disagrees now is the time. <br /></blockquote><br /> We use plpy.subtransaction() to create
Subxactobjects, so I though plpy.cursor() would be most appropriate. <br /><br /><blockquote type="cite">This patch
doesnot provide a wrapper around SPI_cursor_move. The patch <br /> is useful without that and I don't see anything that
preculdessomeone else <br /> adding that later if they see a need. <br /></blockquote><br /> My idea is to add keyword
argumentsto plpy.cursor() that will allow you to decide whether you want a scrollable cursor and after that provide a
move()method. <br /><br /><blockquote type="cite">The patch includes documentation updates that describes the new
feature.<br /> The Database Access page doesn't provide a API style list of database <br /> access <br /> functions
likethe plperl <br /><a class="moz-txt-link-freetext"
href="http://www.postgresql.org/docs/9.1/interactive/plperl-builtins.html">http://www.postgresql.org/docs/9.1/interactive/plperl-builtins.html</a>
page<br /> does. I think the organization of the perl page is <br /> clearer than the python one and we should think
abouta doing some <br /> documentaiton refactoring. That should be done as a seperate patch and <br /> shouldn't be a
barrierto committing this one. <br /></blockquote><br /> Yeah, the PL/Python docs are a bit chaotic right now. I
haven'tyet summoned force to overhaul them. <br /><br /><blockquote type="cite">in PLy_cursor_plan line 4080 <br /> +
PG_TRY();<br /> + { <br /> + Portal portal; <br /> + char *volatile nulls; <br /> + volatile int j; <br
/></blockquote><br/><blockquote type="cite">I am probably not seeing a code path or misunderstanding something <br />
aboutthe setjmp/longjump usages but I don't see why nulls and j need to be <br /> volatile here. <br /></blockquote><br
/>It looked like you could drop volatile there (and in PLy_spi_execute_plan, where this is copied from (did I mention
there'squite some code duplication in PL/Python?)) but digging in git I found this commit: <br /><br /><a
class="moz-txt-link-freetext"
href="http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=2789b7278c11785750dd9d2837856510ffc67000">http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=2789b7278c11785750dd9d2837856510ffc67000</a><br
/><br/> that added the original volatile qualification, so I guess there's a reason. <br /><br /><blockquote
type="cite">line444 <br /> PLy_cursor(PyObject *self, PyObject *args) <br /> + { <br /> + char *query; <br /> +
PyObject*plan; <br /> + PyObject *planargs = NULL; <br /> + <br /> + if (PyArg_ParseTuple(args, "s", &query)) <br
/>+ return PLy_cursor_query(query); <br /> + <br /><br /> Should query be freed with PyMem_free() <br
/></blockquote><br/> No, PyArg_ParseTuple returns a string on the stack, I check that repeatedly creating a cursor with
aplan argument does not leak memory and that adding PyMem_Free there promptly leads to a segfault. <br /><br /><br
/><blockquotetype="cite">I tested both python 2.6 and 3 on a Linux system <br /><br /> [test cases demonstrating bugs]
<br/></blockquote><br /> Turns out it's a really bad idea to store pointers to Portal structures, because they get
invalidatedby the subtransaction abort hooks. <br /><br /> I switched to storing the cursor name and looking it up in
theappropriate hash table every time it's used. The examples you sent (which I included as regression tests) now cause
aValueError to be raised with a message stating that the cursor has been created in an aborted subtransaction. <br
/><br/> Not sure about the wording of the error message, though. <br /><br /> Thanks again for the review! <br /><br />
Cheers,<br /> Jan <br /><pre wrap="">
 
<fieldset class="mimeAttachmentHeader"></fieldset>

</pre></blockquote><br />