Re: plpython SPI cursors
От | Steve Singer |
---|---|
Тема | Re: plpython SPI cursors |
Дата | |
Msg-id | BLU0-SMTP2363BD56F6BB1AD3E704448ECC0@phx.gbl обсуждение исходный текст |
Ответ на | Re: plpython SPI cursors (Jan Urbański <wulczer@wulczer.org>) |
Ответы |
Re: plpython SPI cursors
|
Список | pgsql-hackers |
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 />
В списке pgsql-hackers по дате отправления: