Re: [HACKERS] plpython is broken for recursive use
On 10/16/2015 10:03 PM, Tom Lane wrote: I wrote: This seems like a very Rube-Goldbergian way of setting up a local namespace for the user-defined code. I think perhaps what we should do is: 1. Compile the user-supplied code directly into a code object, without wrapping it in a "def". (Hence, PLy_procedure_munge_source goes away entirely, which would be nice.) Forget about generating a code object containing a call, too. After further study, it appears this approach won't work because it breaks "yield" --- AFAICT, Python only allows "yield" inside a "def". At this point I think what we need is to find a way of passing the function parameters honestly, that is, as actual parameters in the manufactured call. I've not looked into how that might be done. +1 if it can be done I haven't looked very closely at plpython for a long time, but anything else seems ugly. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython is broken for recursive use
I wrote: > This seems like a very Rube-Goldbergian way of setting up a local > namespace for the user-defined code. I think perhaps what we should do > is: > 1. Compile the user-supplied code directly into a code object, without > wrapping it in a "def". (Hence, PLy_procedure_munge_source goes away > entirely, which would be nice.) Forget about generating a code object > containing a call, too. After further study, it appears this approach won't work because it breaks "yield" --- AFAICT, Python only allows "yield" inside a "def". At this point I think what we need is to find a way of passing the function parameters honestly, that is, as actual parameters in the manufactured call. I've not looked into how that might be done. 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
Re: [HACKERS] plpython is broken for recursive use
I wrote: > Anyway, the real problem here is the decision to pass procedure arguments > by assigning them to keys in the global dict. That is just brain-dead, > both because it means that recursive calls can't possibly work and because > it creates the bizarre scoping behavior mentioned in > http://www.postgresql.org/docs/devel/static/plpython-funcs.html > I suppose we cannot change that in back branches for fear of creating > subtle compatibility problems, but surely we can do better going forward? I looked into this a little more. The Python function we use to execute plpython code, PyEval_EvalCode, has arguments to supply both a global and a local namespace dict. Right now we just pass proc->globals for both, which sounds and is weird. However, what we're actually executing in that context is just an argument-less function call, eg "__plpython_procedure_foo_nnn()". So I believe that no use of the passed "local" namespace actually happens: the user-supplied code executes with proc->globals as its global namespace and some transient dict created by the function call action as a local namespace. This seems like a very Rube-Goldbergian way of setting up a local namespace for the user-defined code. I think perhaps what we should do is: 1. Compile the user-supplied code directly into a code object, without wrapping it in a "def". (Hence, PLy_procedure_munge_source goes away entirely, which would be nice.) Forget about generating a code object containing a call, too. 2. During function call startup, create a dict to be the local namespace for that call. Shove the argument values into entries in that dict, not the global one. 3. Run the user-supplied code directly with PyEval_EvalCode, passing proc->globals as its global namespace and the transient dict as its local namespace. This would fix the problem with recursive calls and probably be a lot cleaner as well. It would not be 100% backwards compatible, because the technique shown in http://www.postgresql.org/docs/devel/static/plpython-funcs.html of declaring an argument "global" would not work, nor be necessary, anymore. That does not seem like a bad thing, but it would be a reason not to back-patch. Thoughts? 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
Re: [HACKERS] plpython is broken for recursive use
Andrew Dunstan writes: > On 10/15/2015 05:16 PM, Josh Berkus wrote: >> On 10/15/2015 01:10 PM, Tom Lane wrote: >>> I think this means that we should get rid of proc->globals and instead >>> manufacture a new globals dict locally in each call to PLy_exec_function >>> or PLy_exec_trigger. >> Don't people currently specifically treat the state of the globals dict >> as a feature? That is, make use of the fact that you can store >> session-persistent data in it? > That was the thinking behind plperl's %_SHARED, and I assume this is > similar. I poked around in the code a bit more, and I now see that the procedure's "globals" dictionary actually is global, in the sense that it's not the most closely nested namespace when the procedure's code runs. It's not so surprising that if you write "global foo" then foo will have a value that persists across calls. But then it is fair to ask what the heck is the point of the "SD" dict, which has got *exactly* the same lifespan as the procedure's globals dictionary --- if you want to share a value across multiple executions of the procedure, it does not matter whether you make it within SD or just declare it "global". So why'd we bother with SD? Anyway, the real problem here is the decision to pass procedure arguments by assigning them to keys in the global dict. That is just brain-dead, both because it means that recursive calls can't possibly work and because it creates the bizarre scoping behavior mentioned in http://www.postgresql.org/docs/devel/static/plpython-funcs.html I suppose we cannot change that in back branches for fear of creating subtle compatibility problems, but surely we can do better going forward? Another interesting point is that if the procedure cache entry is rebuilt for any reason whatever, such as an ALTER on the function definition or even just an sinval flush, you lose whatever may have been in either SD or the procedure's "global" namespace. That seems like a rather surprising implementation behavior. I'd have expected plpython to make some effort to make "SD" have actual session lifespan, not just "maybe it'll survive and maybe it won't". 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
Re: [HACKERS] plpython is broken for recursive use
On 10/15/2015 02:16 PM, Josh Berkus wrote: On 10/15/2015 01:10 PM, Tom Lane wrote: I think this means that we should get rid of proc->globals and instead manufacture a new globals dict locally in each call to PLy_exec_function or PLy_exec_trigger. For SETOF functions it would be necessary to keep the globals dict reference somewhere in the FunctionCallInfo struct, probably. Not sure about cleaning up after an error that occurs between SETOF callbacks --- we might need plpython to grow an at-abort callback to do decref's on unreleased dicts. Don't people currently specifically treat the state of the globals dict as a feature? That is, make use of the fact that you can store session-persistent data in it? Yes, just like the plperl feature. jD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Centered full stack support, consulting and development. New rule for social situations: "If you think to yourself not even JD would say this..." Stop and shut your mouth. It's going to be bad. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython is broken for recursive use
On 10/15/2015 05:16 PM, Josh Berkus wrote: On 10/15/2015 01:10 PM, Tom Lane wrote: I think this means that we should get rid of proc->globals and instead manufacture a new globals dict locally in each call to PLy_exec_function or PLy_exec_trigger. For SETOF functions it would be necessary to keep the globals dict reference somewhere in the FunctionCallInfo struct, probably. Not sure about cleaning up after an error that occurs between SETOF callbacks --- we might need plpython to grow an at-abort callback to do decref's on unreleased dicts. Don't people currently specifically treat the state of the globals dict as a feature? That is, make use of the fact that you can store session-persistent data in it? That was the thinking behind plperl's %_SHARED, and I assume this is similar. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython is broken for recursive use
On 10/15/2015 01:10 PM, Tom Lane wrote: > I think this means that we should get rid of proc->globals and instead > manufacture a new globals dict locally in each call to PLy_exec_function > or PLy_exec_trigger. For SETOF functions it would be necessary to keep > the globals dict reference somewhere in the FunctionCallInfo struct, > probably. Not sure about cleaning up after an error that occurs between > SETOF callbacks --- we might need plpython to grow an at-abort callback to > do decref's on unreleased dicts. Don't people currently specifically treat the state of the globals dict as a feature? That is, make use of the fact that you can store session-persistent data in it? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] plpython is broken for recursive use
I looked into bug #13683,
http://www.postgresql.org/message-id/20151015135804.3019.31...@wrigleys.postgresql.org
It looks to me like plpython basically doesn't work at all for re-entrant
calls to plpython functions, because all executions of a given function
share the same "globals" dict, which doesn't seem sane. Shouldn't each
execution have its own?
I believe the specific sequence of events shown here is
1. On the way into the recursion, each level does PLy_function_build_args
which sets "i" as a key in the proc's globals dict.
2. On the way out, each level does PLy_function_delete_args which deletes
"i" from the dict. At levels after the innermost, this results in setting
"KeyError: 'i'" as the active Python error, because that key is already
gone. We fail to notice the error indicator, though.
3. The pending error indicator causes the attempt to "def" the second
function to fail.
If I change the test case to
create or replace function a(i integer) returns integer as
$$
if i > 1:
return i
j = plpy.execute("select a(%s)" % (i+1))[0]['a']
return i+j
$$ language plpythonu;
select a(0);
then I get
ERROR: spiexceptions.ExternalRoutineException: NameError: global name 'i' is
not defined
CONTEXT: Traceback (most recent call last):
PL/Python function "a", line 4, in
j = plpy.execute("select a(%s)" % (i+1))[0]['a']
PL/Python function "a"
which shows that a()'s own access to "i" is broken too, though I confess
I'm not sure why it's failing at line 4 rather than 5.
I think this means that we should get rid of proc->globals and instead
manufacture a new globals dict locally in each call to PLy_exec_function
or PLy_exec_trigger. For SETOF functions it would be necessary to keep
the globals dict reference somewhere in the FunctionCallInfo struct,
probably. Not sure about cleaning up after an error that occurs between
SETOF callbacks --- we might need plpython to grow an at-abort callback to
do decref's on unreleased dicts.
Thoughts?
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
Re: PL/Lua (was: [HACKERS] plpython implementation)
On 07/02/2013 01:54 AM, Luis Carvalho wrote: Peter Eisentraut wrote: On Mon, 2013-07-01 at 18:15 -0400, Luis Carvalho wrote: The project is maintained -- I don't know how to say when something is well-maintained, but small frequency of code updates is not one of my criteria; The bug tracker contains bugs about build problems with PG 8.4, 9.2, and 9.3, which have not been addressed. Done (it took me a while to see the bug tracker in pgfoundry...) BTW, thanks for the patch; I'll release a new version of PL/Lua once PG 9.3 is out. It might be worth looking at the feature set of PL/v8 which currently seems to be larger than PL/Perl, PL/Python and PL/tcl. Including having the possibility to implement window functions. http://pgxn.org/dist/plv8/doc/plv8.html#Window.function.API Nice job with PL/Lua, Andreas -- Andreas Karlsson -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: PL/Lua (was: [HACKERS] plpython implementation)
Peter Eisentraut wrote:
> On Mon, 2013-07-01 at 18:15 -0400, Luis Carvalho wrote:
> > The project is maintained -- I don't know how to say when something is
> > well-maintained, but small frequency of code updates is not one of my
> > criteria;
>
> The bug tracker contains bugs about build problems with PG 8.4, 9.2, and
> 9.3, which have not been addressed.
Done (it took me a while to see the bug tracker in pgfoundry...) BTW, thanks
for the patch; I'll release a new version of PL/Lua once PG 9.3 is out.
Cheers,
Luis
--
Computers are useless. They can only give you answers.
-- Pablo Picasso
--
Luis Carvalho (Kozure)
lua -e 'print((("lexcarva...@no.gmail.spam.com"):gsub("(%u+%.)","")))'
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: PL/Lua (was: [HACKERS] plpython implementation)
On Mon, 2013-07-01 at 18:15 -0400, Luis Carvalho wrote: > The project is maintained -- I don't know how to say when something is > well-maintained, but small frequency of code updates is not one of my > criteria; The bug tracker contains bugs about build problems with PG 8.4, 9.2, and 9.3, which have not been addressed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
PL/Lua (was: [HACKERS] plpython implementation)
Hi all,
Claudio Freire wrote:
> On Mon, Jul 1, 2013 at 2:29 AM, james wrote:
> > On 01/07/2013 02:43, Claudio Freire wrote:
> >>
> >> In essence, you'd have to use another implementation. CPython guys
> >> have left it very clear they don't intend to "fix" that, as they don't
> >> consider it a bug. It's just how it is.
> >
> > Given how useful it is to have a scripting language that can be used outside
> > of the database as well as inside it, would it be reasonable to consider
> > 'promoting' pllua?
> >
> > My understanding is that it (lua) is much cleaner under the hood (than
> > CPython).
> > Although I do recognise that Python as a whole has always had more traction.
>
> Well, that, or you can use another implementation. There are many, and
> PyPy should be seriously considered given its JIT and how much faster
> it is for raw computation power, which is what a DB is most likely
> going to care about. I bet PyPy's sandboxing is a lot better as well.
I think that 'promoting' PL/Lua would be too early, but it'd be a great
addition. The latest version, for instance, can run LuaJIT which has a FFI
(check the example in "Anonymous Blocks" at PL/Lua's docs.) I think there are
two main problems: finding maintainers in the core, and lack of popularity to
warrant its promotion (the two problems are related, of course.)
Peter Eisentraut wrote:
> On 7/1/13 1:29 AM, james wrote:
> > Given how useful it is to have a scripting language that can be used
> > outside
> > of the database as well as inside it, would it be reasonable to consider
> > 'promoting' pllua?
>
> You can start promoting pllua by making it work with current PostgreSQL
> versions. It hasn't been updated in 5 years, and doesn't build cleanly
> last I checked.
>
> Having a well-maintained and fully featured pllua available would surely
> be welcome by many.
Thanks for the feedback. Actually, PL/Lua's latest version (1.0) was out one
month ago,
http://pgfoundry.org/frs/?group_id=1000314
but the previous version took around 4 years. I was waiting for bug reports,
since I deemed PL/Lua to be fairly featured, but I have now declared it
"stable".
The project is maintained -- I don't know how to say when something is
well-maintained, but small frequency of code updates is not one of my
criteria; Lua, for instance, took six years between versions 5.2 and 5.1.
BTW, just out of curiosity, when was the last time PL/Tcl was updated?
I think that the project is also fully featured, but I'd appreciate any
comments on the contrary (that is, feature requests.) I might be mistaken, but
PL/Lua has all the features that PL/Python, PL/Perl, and PL/Tcl have, but, for
example, features a trusted flavor when PL/Python does not, and has proper
type mappings, which PL/Perl does not (everything is translated to text.)
PL/Lua 1.0 adds anonymous blocks and a TRUNCATE trigger, and it should run on
PostgreSQL 9.2. It can be used with Lua 5.1, 5.2, and LuaJIT 2.0 (if you want
speed and an easy C interface through a FFI, you should try LuaJIT!)
I'd like to take this opportunity to kindly ask the PostgreSQL doc maintainers
to include PL/Lua in the table at Appendix H.3:
Name: PL/Lua
Language: Lua
Website: http://pgfoundry.org/projects/pllua/
Cheers,
Luis
--
Computers are useless. They can only give you answers.
-- Pablo Picasso
--
Luis Carvalho (Kozure)
lua -e 'print((("lexcarva...@no.gmail.spam.com"):gsub("(%u+%.)","")))'
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On 7/1/13 1:29 AM, james wrote: > Given how useful it is to have a scripting language that can be used > outside > of the database as well as inside it, would it be reasonable to consider > 'promoting' pllua? You can start promoting pllua by making it work with current PostgreSQL versions. It hasn't been updated in 5 years, and doesn't build cleanly last I checked. Having a well-maintained and fully featured pllua available would surely be welcome by many. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On 2013-06-30 22:43:52 -0300, Claudio Freire wrote: > Not only that, the CPython interpreter is rather fuzzy about the > division between interpreters. You can initialize multiple > interpreters, but they share a lot of state, so you can never fully > separate them. You'd have some state from the untrusted interpreter > spill over into the trusted one within the same session, which is not > ideal at all (and in fact can be exploited). > > In essence, you'd have to use another implementation. CPython guys > have left it very clear they don't intend to "fix" that, as they don't > consider it a bug. It's just how it is. Doesn't zope's RestrictedPython have a history of working reasonably well? Now, you sure pay a price for that, but ... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On 07/01/2013 07:53 AM, Claudio Freire wrote: > On Mon, Jul 1, 2013 at 2:29 AM, james wrote: >> On 01/07/2013 02:43, Claudio Freire wrote: >>> In essence, you'd have to use another implementation. CPython guys >>> have left it very clear they don't intend to "fix" that, as they don't >>> consider it a bug. It's just how it is. >> Given how useful it is to have a scripting language that can be used outside >> of the database as well as inside it, would it be reasonable to consider >> 'promoting' pllua? >> >> My understanding is that it (lua) is much cleaner under the hood (than >> CPython). >> Although I do recognise that Python as a whole has always had more traction. > Well, that, or you can use another implementation. There are many, and > PyPy should be seriously considered given its JIT and how much faster > it is for raw computation power, which is what a DB is most likely > going to care about. OTOH, pypy startup time is bigger than CPython. It is also generally slower at running small on-call functions before JIT kicks in. > I bet PyPy's sandboxing is a lot better as well. pypy sandbox implementation seems to be a sound one, as it delegates all "unsafe" operations to outside controller at bytecode level. The outside controller usually being a standard CPython wrapper. Of course this makes any such operations slower, but this is the price to pay for sandboxing. > Making a postgres-interphasing pypy fork I guess would be a nice > project, it's as "simple" as implementing all of plpy's API in RPython > and translating a C module out of it. I have some ideas about allowing new pl-s to be written in pl/pythonu If any of you interested in this are at Europython come talk to me about this after my presentations ;) > No, I'm not volunteering ;-) Neither am I, at least not yet -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On Mon, Jul 1, 2013 at 2:29 AM, james wrote: > On 01/07/2013 02:43, Claudio Freire wrote: >> >> In essence, you'd have to use another implementation. CPython guys >> have left it very clear they don't intend to "fix" that, as they don't >> consider it a bug. It's just how it is. > > Given how useful it is to have a scripting language that can be used outside > of the database as well as inside it, would it be reasonable to consider > 'promoting' pllua? > > My understanding is that it (lua) is much cleaner under the hood (than > CPython). > Although I do recognise that Python as a whole has always had more traction. Well, that, or you can use another implementation. There are many, and PyPy should be seriously considered given its JIT and how much faster it is for raw computation power, which is what a DB is most likely going to care about. I bet PyPy's sandboxing is a lot better as well. Making a postgres-interphasing pypy fork I guess would be a nice project, it's as "simple" as implementing all of plpy's API in RPython and translating a C module out of it. No, I'm not volunteering ;-) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On 01/07/2013 02:43, Claudio Freire wrote: In essence, you'd have to use another implementation. CPython guys have left it very clear they don't intend to "fix" that, as they don't consider it a bug. It's just how it is. Given how useful it is to have a scripting language that can be used outside of the database as well as inside it, would it be reasonable to consider 'promoting' pllua? My understanding is that it (lua) is much cleaner under the hood (than CPython). Although I do recognise that Python as a whole has always had more traction. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On Sun, Jun 30, 2013 at 9:45 AM, Andres Freund wrote:
> On 2013-06-30 14:42:24 +0200, Szymon Guz wrote:
>> On 30 June 2013 14:31, Martijn van Oosterhout wrote:
>>
>> > On Sun, Jun 30, 2013 at 02:18:07PM +0200, Szymon Guz wrote:
>> > > > python does not any any sort of reliable sandbox, so there is no
>> > plpython,
>> > > > only plpythonu - hence only one interpreter per backend is needed.
>> > > >
>> > > Is there any track of the discussion that there is no way to make the
>> > > sandbox? I managed to create some kind of sandbox, a simple modification
>> > > which totally disables importing modules, so I'm just wondering why it
>> > > cannot be done.
>> >
>> > http://wiki.python.org/moin/SandboxedPython
>> >
>> > This is the thread I was thinking of:
>> > http://mail.python.org/pipermail/python-dev/2009-February/086401.html
>> >
>> > If you read through it I think you will understand the difficulties.
>> >
>> thanks for links. I was thinking about something else. In fact we don't
>> need full sandbox, I think it would be enough to have safe python, if it
>> couldn't import any outside module. Wouldn't be enough?
>>
>> It seems like the sandbox modules want to limit many external operations,
>> I'm thinking about not being able to import any module, even standard ones,
>> wouldn't be enough?
>
> python
>>> open('/etc/passwd', 'r').readlines()
Not only that, the CPython interpreter is rather fuzzy about the
division between interpreters. You can initialize multiple
interpreters, but they share a lot of state, so you can never fully
separate them. You'd have some state from the untrusted interpreter
spill over into the trusted one within the same session, which is not
ideal at all (and in fact can be exploited).
In essence, you'd have to use another implementation. CPython guys
have left it very clear they don't intend to "fix" that, as they don't
consider it a bug. It's just how it is.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On 30 June 2013 14:45, Andres Freund wrote:
> On 2013-06-30 14:42:24 +0200, Szymon Guz wrote:
> > On 30 June 2013 14:31, Martijn van Oosterhout wrote:
> >
> > > On Sun, Jun 30, 2013 at 02:18:07PM +0200, Szymon Guz wrote:
> > > > > python does not any any sort of reliable sandbox, so there is no
> > > plpython,
> > > > > only plpythonu - hence only one interpreter per backend is needed.
> > > > >
> > > > Is there any track of the discussion that there is no way to make the
> > > > sandbox? I managed to create some kind of sandbox, a simple
> modification
> > > > which totally disables importing modules, so I'm just wondering why
> it
> > > > cannot be done.
> > >
> > > http://wiki.python.org/moin/SandboxedPython
> > >
> > > This is the thread I was thinking of:
> > > http://mail.python.org/pipermail/python-dev/2009-February/086401.html
> > >
> > > If you read through it I think you will understand the difficulties.
> > >
> > thanks for links. I was thinking about something else. In fact we don't
> > need full sandbox, I think it would be enough to have safe python, if it
> > couldn't import any outside module. Wouldn't be enough?
> >
> > It seems like the sandbox modules want to limit many external operations,
> > I'm thinking about not being able to import any module, even standard
> ones,
> > wouldn't be enough?
>
> python
> >> open('/etc/passwd', 'r').readlines()
>
>
thanks :)
Re: [HACKERS] plpython implementation
On 2013-06-30 14:42:24 +0200, Szymon Guz wrote:
> On 30 June 2013 14:31, Martijn van Oosterhout wrote:
>
> > On Sun, Jun 30, 2013 at 02:18:07PM +0200, Szymon Guz wrote:
> > > > python does not any any sort of reliable sandbox, so there is no
> > plpython,
> > > > only plpythonu - hence only one interpreter per backend is needed.
> > > >
> > > Is there any track of the discussion that there is no way to make the
> > > sandbox? I managed to create some kind of sandbox, a simple modification
> > > which totally disables importing modules, so I'm just wondering why it
> > > cannot be done.
> >
> > http://wiki.python.org/moin/SandboxedPython
> >
> > This is the thread I was thinking of:
> > http://mail.python.org/pipermail/python-dev/2009-February/086401.html
> >
> > If you read through it I think you will understand the difficulties.
> >
> thanks for links. I was thinking about something else. In fact we don't
> need full sandbox, I think it would be enough to have safe python, if it
> couldn't import any outside module. Wouldn't be enough?
>
> It seems like the sandbox modules want to limit many external operations,
> I'm thinking about not being able to import any module, even standard ones,
> wouldn't be enough?
python
>> open('/etc/passwd', 'r').readlines()
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On 30 June 2013 14:31, Martijn van Oosterhout wrote: > On Sun, Jun 30, 2013 at 02:18:07PM +0200, Szymon Guz wrote: > > > python does not any any sort of reliable sandbox, so there is no > plpython, > > > only plpythonu - hence only one interpreter per backend is needed. > > > > > Is there any track of the discussion that there is no way to make the > > sandbox? I managed to create some kind of sandbox, a simple modification > > which totally disables importing modules, so I'm just wondering why it > > cannot be done. > > http://wiki.python.org/moin/SandboxedPython > > This is the thread I was thinking of: > http://mail.python.org/pipermail/python-dev/2009-February/086401.html > > If you read through it I think you will understand the difficulties. > > Hi Martin, thanks for links. I was thinking about something else. In fact we don't need full sandbox, I think it would be enough to have safe python, if it couldn't import any outside module. Wouldn't be enough? It seems like the sandbox modules want to limit many external operations, I'm thinking about not being able to import any module, even standard ones, wouldn't be enough? Szymon
Re: [HACKERS] plpython implementation
On 06/30/2013 08:18 AM, Szymon Guz wrote: python does not any any sort of reliable sandbox, so there is no plpython, only plpythonu - hence only one interpreter per backend is needed. Is there any track of the discussion that there is no way to make the sandbox? I managed to create some kind of sandbox, a simple modification which totally disables importing modules, so I'm just wondering why it cannot be done. If your sandbox is simple it's almost certainly going to be broken. I suggest you use Google to research the topic. Our discussions should be in the mailing list archives. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On Sun, Jun 30, 2013 at 02:18:07PM +0200, Szymon Guz wrote: > > python does not any any sort of reliable sandbox, so there is no plpython, > > only plpythonu - hence only one interpreter per backend is needed. > > > Is there any track of the discussion that there is no way to make the > sandbox? I managed to create some kind of sandbox, a simple modification > which totally disables importing modules, so I'm just wondering why it > cannot be done. http://wiki.python.org/moin/SandboxedPython This is the thread I was thinking of: http://mail.python.org/pipermail/python-dev/2009-February/086401.html If you read through it I think you will understand the difficulties. Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > He who writes carelessly confesses thereby at the very outset that he does > not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: [HACKERS] plpython implementation
On 30 June 2013 14:13, Andrew Dunstan wrote: > > On 06/30/2013 07:49 AM, Szymon Guz wrote: > >> I'm reading through plperl and plpython implementations and I don't >> understand the way they work. >> >> Comments for plperl say that there are two interpreters (trusted and >> untrusted) for each user session, and they are stored in a hash. >> >> Plpython version looks quite different, there is no such global hash with >> interpreters, there is just a pointer to an interpreter and one global >> function _PG_init, which runs once (but per session, user, or what?). >> >> I'm just wondering how a plpython implementation should look like. We >> need another interpreter, but PG_init function is run once, should it then >> create two interpreters on init, or should we let this function do nothing >> and create a proper interpreter in the first call of plpython(u) function >> for current session? >> >> >> > > python does not any any sort of reliable sandbox, so there is no plpython, > only plpythonu - hence only one interpreter per backend is needed. > > Is there any track of the discussion that there is no way to make the sandbox? I managed to create some kind of sandbox, a simple modification which totally disables importing modules, so I'm just wondering why it cannot be done. Szymon
Re: [HACKERS] plpython implementation
On 06/30/2013 07:49 AM, Szymon Guz wrote: I'm reading through plperl and plpython implementations and I don't understand the way they work. Comments for plperl say that there are two interpreters (trusted and untrusted) for each user session, and they are stored in a hash. Plpython version looks quite different, there is no such global hash with interpreters, there is just a pointer to an interpreter and one global function _PG_init, which runs once (but per session, user, or what?). I'm just wondering how a plpython implementation should look like. We need another interpreter, but PG_init function is run once, should it then create two interpreters on init, or should we let this function do nothing and create a proper interpreter in the first call of plpython(u) function for current session? python does not any any sort of reliable sandbox, so there is no plpython, only plpythonu - hence only one interpreter per backend is needed. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython implementation
On Sun, Jun 30, 2013 at 01:49:53PM +0200, Szymon Guz wrote: > I'm reading through plperl and plpython implementations and I don't > understand the way they work. > > Comments for plperl say that there are two interpreters (trusted and > untrusted) for each user session, and they are stored in a hash. The point is that python has no version for untrusted users, since it's been accepted that there's no way to build a python sandbox for untrusted code. There was actually a small competition to make one but it failed, since then they don't bother. Perl does provide a sandbox, hence you can have two interpreters in a single backend. Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > He who writes carelessly confesses thereby at the very outset that he does > not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
[HACKERS] plpython implementation
I'm reading through plperl and plpython implementations and I don't understand the way they work. Comments for plperl say that there are two interpreters (trusted and untrusted) for each user session, and they are stored in a hash. Plpython version looks quite different, there is no such global hash with interpreters, there is just a pointer to an interpreter and one global function _PG_init, which runs once (but per session, user, or what?). I'm just wondering how a plpython implementation should look like. We need another interpreter, but PG_init function is run once, should it then create two interpreters on init, or should we let this function do nothing and create a proper interpreter in the first call of plpython(u) function for current session? thanks, Szymon
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
On 05/07/12 21:37, Heikki Linnakangas wrote: Committed. This bug was present in versions >= 9.0, so backpatched. Thanks! I used ereport() rather than elog() in the error message. Correct me if that was wrong, but the point was to avoid PLy_elog(), because that might cause recursion, and ereport() should be ok. I believe the message should be translated, as it's quite possible to get that error, at least if you use SQL_ASCII, so ereport() is more approriate than elog(). Yes, you're absolutely right. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
On 04.07.2012 15:11, Jan Urbański wrote:
On 04/07/12 13:58, Asif Naeem wrote:
I have test the patch on Win64. postgres server is working fine now for
WIN1252. Thanks.
create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('**utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
enctest | encode
--+
tróspido | 7472c3b3737069646f
(1 row)
Please do let me know If you have any other query. Thanks.
Great, this looks correct.
Can we apply this to 9.2?
Committed. This bug was present in versions >= 9.0, so backpatched.
I used ereport() rather than elog() in the error message. Correct me if
that was wrong, but the point was to avoid PLy_elog(), because that
might cause recursion, and ereport() should be ok. I believe the message
should be translated, as it's quite possible to get that error, at least
if you use SQL_ASCII, so ereport() is more approriate than elog().
Thanks!
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
On 04/07/12 13:58, Asif Naeem wrote:
Patch attached. Asif, could you try a few things on a CP1252 database?
First verify if your original test case now works and then try this:
I have test the patch on Win64. postgres server is working fine now for
WIN1252. Thanks.
create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('**utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
enctest | encode
--+
tróspido | 7472c3b3737069646f
(1 row)
Please do let me know If you have any other query. Thanks.
Great, this looks correct.
Can we apply this to 9.2?
Cheers,
Jan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
> Patch attached. Asif, could you try a few things on a CP1252 database?
First verify if your original test case now works and then try this:
>
>
I have test the patch on Win64. postgres server is working fine now for
WIN1252. Thanks.
> create function enctest() returns text as $$
> return b'tr\xc3\xb3spido'.decode('**utf-8')
> $$ language plpython3u;
>
> select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
>
create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
enctest | encode
--+
tróspido | 7472c3b3737069646f
(1 row)
Please do let me know If you have any other query. Thanks.
Best Regards,
Muhammad Asif Naeem
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
On 03/07/12 17:45, Jan Urbański wrote:
On 29/06/12 00:36, Jan Urbański wrote:
On 27/06/12 13:57, Jan Urbański wrote:
On 27/06/12 11:51, Asif Naeem wrote:
Hi,
On Windows 7 64bit, plpython is causing server crash with the following
test case i.e.
So: I'd add code to translate WINxxx into CPxxx when choosing the Python
to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the
SQL_ASCII case alone, as there were no complaints and people using
SQL_ASCII are asking for it anyway.
Since no one commented, I'll produce a patch to that effect. I believe
this should go into 9.2 given that otherwise PL/Python will basically
crash any database using the CP12xx encoding.
Patch attached. Asif, could you try a few things on a CP1252 database?
First verify if your original test case now works and then try this:
create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
Thanks,
Jan
diff --git a/src/pl/plpython/plpy_util.c b/src/pl/plpython/plpy_util.c
new file mode 100644
index 9a4901e..5fafbd1
*** a/src/pl/plpython/plpy_util.c
--- b/src/pl/plpython/plpy_util.c
*** PLyUnicode_Bytes(PyObject *unicode)
*** 66,80
/*
* Python understands almost all PostgreSQL encoding names, but it doesn't
! * know SQL_ASCII.
*/
! if (GetDatabaseEncoding() == PG_SQL_ASCII)
! serverenc = "ascii";
! else
! serverenc = GetDatabaseEncodingName();
rv = PyUnicode_AsEncodedString(unicode, serverenc, "strict");
! if (rv == NULL)
! PLy_elog(ERROR, "could not convert Python Unicode object to PostgreSQL server encoding");
return rv;
}
--- 66,125
/*
* Python understands almost all PostgreSQL encoding names, but it doesn't
! * know SQL_ASCII and calls the Windows encodings differently.
*/
! switch (GetDatabaseEncoding())
! {
! case PG_SQL_ASCII:
! serverenc = "ascii";
! break;
! case PG_WIN1250:
! serverenc = "cp1250";
! break;
! case PG_WIN1251:
! serverenc = "cp1251";
! break;
! case PG_WIN1252:
! serverenc = "cp1252";
! break;
! case PG_WIN1253:
! serverenc = "cp1253";
! break;
! case PG_WIN1254:
! serverenc = "cp1254";
! break;
! case PG_WIN1255:
! serverenc = "cp1255";
! break;
! case PG_WIN1256:
! serverenc = "cp1256";
! break;
! case PG_WIN1257:
! serverenc = "cp1257";
! break;
! case PG_WIN1258:
! serverenc = "cp1258";
! break;
! case PG_WIN866:
! serverenc = "cp866";
! break;
! case PG_WIN874:
! serverenc = "cp874";
! break;
! default:
! serverenc = GetDatabaseEncodingName();
! break;
! }
!
rv = PyUnicode_AsEncodedString(unicode, serverenc, "strict");
! if (rv == NULL) {
! /*
! * Use a plan elog instead of PLy_elog here to avoid getting in
! * recursion trouble when the traceback formatting functions try doing
! * unicode to bytes conversion.
! */
! elog(ERROR, "could not convert Python Unicode object to PostgreSQL server encoding");
! }
return rv;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
On 29/06/12 00:36, Jan Urbański wrote: On 27/06/12 13:57, Jan Urbański wrote: On 27/06/12 11:51, Asif Naeem wrote: Hi, On Windows 7 64bit, plpython is causing server crash with the following test case i.e. So: I'd add code to translate WINxxx into CPxxx when choosing the Python to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the SQL_ASCII case alone, as there were no complaints and people using SQL_ASCII are asking for it anyway. Since no one commented, I'll produce a patch to that effect. I believe this should go into 9.2 given that otherwise PL/Python will basically crash any database using the CP12xx encoding. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
Thank you. Please do let me know once fix check-in. I will test it and share feedback with you. Thanks. Best Regards, Asif Naeem On Fri, Jun 29, 2012 at 3:36 AM, Jan Urbański wrote: > On 27/06/12 13:57, Jan Urbański wrote: > >> On 27/06/12 11:51, Asif Naeem wrote: >> >>> Hi, >>> >>> On Windows 7 64bit, plpython is causing server crash with the following >>> test case i.e. >>> >>> CREATE PROCEDURAL LANGUAGE 'plpython3u'; >>> CREATE OR REPLACE FUNCTION pymax (a integer, b integer) RETURNS integer AS $$ if a> b: return a return b $$ LANGUAGE plpython3u; SELECT pymax(1, 2); >>> >> >>> I think primary reason that trigger this issue is when Function >>> PLyUnicode_Bytes() calls "PyUnicode_AsEncodedString( ,WIN1252 /*Server >>> encoding*/, ) " it fails with null. I built latest pg 9.2 source code >>> with >>> python 3.2.2.3 by using Visual Studio 2010. Thanks. >>> >> >> I'll try to reproduce this on Linux, which should be possible given the >> results of your investigation. >> > > Your analysis is correct, I managed to reproduce this by injecting > > serverenc = "win1252"; > > into PLyUnicode_Bytes. The comment in that function says that Python > understands all PostgreSQL encoding names except for SQL_ASCII, but that's > not really true. In your case GetDatabaseEncodingName() returns "WIN1252" > and Python accepts "CP125". > > I'm wondering how this should be fixed. Just by adding more special cases > in PLyUnicode_Bytes? > > Even if we add a switch statement that would convert PG_WIN1250 into > "CP1250", Python can still raise an exception when encoding (for various > reasons). How about replacing the PLy_elog there with just an elog? This > loses traceback support and the Python exception message, which could be > helpful for debugging (something like "invalid character for encoding > cp1250"). OTOH, I'm uneasy about invoking the entire PLy_elog machinery > from a function that's as low-level as PLyUnicode_Bytes. > > Lastly, we map SQL_ASCII to "ascii" which is arguably wrong. The function > is supposed to return bytes in the server encoding, and under SQL_ASCII > that probably means we can return anything (ie. use any encoding we deem > useful). Using "ascii" as the Python codec name will raise an error on > anything that has the high bit set. > > So: I'd add code to translate WINxxx into CPxxx when choosing the Python > to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the SQL_ASCII > case alone, as there were no complaints and people using SQL_ASCII are > asking for it anyway. > > Cheers, > Jan >
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
On 27/06/12 13:57, Jan Urbański wrote: On 27/06/12 11:51, Asif Naeem wrote: Hi, On Windows 7 64bit, plpython is causing server crash with the following test case i.e. CREATE PROCEDURAL LANGUAGE 'plpython3u'; CREATE OR REPLACE FUNCTION pymax (a integer, b integer) RETURNS integer AS $$ if a> b: return a return b $$ LANGUAGE plpython3u; SELECT pymax(1, 2); I think primary reason that trigger this issue is when Function PLyUnicode_Bytes() calls "PyUnicode_AsEncodedString( ,WIN1252 /*Server encoding*/, ) " it fails with null. I built latest pg 9.2 source code with python 3.2.2.3 by using Visual Studio 2010. Thanks. I'll try to reproduce this on Linux, which should be possible given the results of your investigation. Your analysis is correct, I managed to reproduce this by injecting serverenc = "win1252"; into PLyUnicode_Bytes. The comment in that function says that Python understands all PostgreSQL encoding names except for SQL_ASCII, but that's not really true. In your case GetDatabaseEncodingName() returns "WIN1252" and Python accepts "CP125". I'm wondering how this should be fixed. Just by adding more special cases in PLyUnicode_Bytes? Even if we add a switch statement that would convert PG_WIN1250 into "CP1250", Python can still raise an exception when encoding (for various reasons). How about replacing the PLy_elog there with just an elog? This loses traceback support and the Python exception message, which could be helpful for debugging (something like "invalid character for encoding cp1250"). OTOH, I'm uneasy about invoking the entire PLy_elog machinery from a function that's as low-level as PLyUnicode_Bytes. Lastly, we map SQL_ASCII to "ascii" which is arguably wrong. The function is supposed to return bytes in the server encoding, and under SQL_ASCII that probably means we can return anything (ie. use any encoding we deem useful). Using "ascii" as the Python codec name will raise an error on anything that has the high bit set. So: I'd add code to translate WINxxx into CPxxx when choosing the Python to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the SQL_ASCII case alone, as there were no complaints and people using SQL_ASCII are asking for it anyway. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
On 27/06/12 11:51, Asif Naeem wrote: Hi, On Windows 7 64bit, plpython is causing server crash with the following test case i.e. CREATE PROCEDURAL LANGUAGE 'plpython3u'; CREATE OR REPLACE FUNCTION pymax (a integer, b integer) RETURNS integer AS $$ if a> b: return a return b $$ LANGUAGE plpython3u; SELECT pymax(1, 2); I think primary reason that trigger this issue is when Function PLyUnicode_Bytes() calls "PyUnicode_AsEncodedString( ,WIN1252 /*Server encoding*/, ) " it fails with null. I built latest pg 9.2 source code with python 3.2.2.3 by using Visual Studio 2010. Thanks. I'll try to reproduce this on Linux, which should be possible given the results of your investigation. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] plpython issue with Win64 (PG 9.2)
Hi, On Windows 7 64bit, plpython is causing server crash with the following test case i.e. CREATE PROCEDURAL LANGUAGE 'plpython3u'; > CREATE OR REPLACE FUNCTION pymax (a integer, b integer) > RETURNS integer > AS $$ > if a > b: > return a > return b > $$ LANGUAGE plpython3u; > SELECT pymax(1, 2); Server exit with the following exception i.e. Unhandled exception at 0x777a3483 in postgres.exe: 0xC0FD: Stack overflow. plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int * tb_depth) Line 174 C plpython3.dll!PLy_elog(int elevel, const char * fmt, ...) Line 67 C plpython3.dll!PLyUnicode_AsString(_object * unicode) Line 96 C plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int * tb_depth) Line 176 + 0x8 bytes C plpython3.dll!PLy_elog(int elevel, const char * fmt, ...) Line 67 C plpython3.dll!PLyUnicode_AsString(_object * unicode) Line 96 C ... ... plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int * tb_depth) Line 176 + 0x8 bytes C plpython3.dll!PLy_elog(int elevel, const char * fmt, ...) Line 67 C plpython3.dll!PLyUnicode_AsString(_object * unicode) Line 96 C plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int * tb_depth) Line 176 + 0x8 bytes C Dbserver get stuck in the following call loop i.e. ... PLy_elog() -> PLy_traceback() -> PLyUnicode_AsString() -> PLyUnicode_Bytes() -> PLy_elog() ... I think primary reason that trigger this issue is when Function PLyUnicode_Bytes() calls "PyUnicode_AsEncodedString( ,WIN1252 /*Server encoding*/, ) " it fails with null. I built latest pg 9.2 source code with python 3.2.2.3 by using Visual Studio 2010. Thanks. Best Regards, Muhammad Asif Naeem
Re: [HACKERS] plpython crash (PG 92)
On lör, 2012-04-28 at 00:32 -0400, Tom Lane wrote: > I'm inclined to think that the best fix is for > PLy_spi_execute_fetch_result to copy the tupledesc into > TopMemoryContext, not the current context. This is a tad scary from a > memory leakage standpoint, but I suppose that if python fails to recover > the PLyResultObject, this isn't the only memory that's going to be > leaked. > > This area appears to be shy a regression test case or two, in any event. Fixed like that. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash (PG 92)
Peter Eisentraut writes: > On tor, 2012-04-26 at 17:32 +0500, Asif Naeem wrote: >> PFA test case. It used simple select statement to retrieve data via >> plpython. It crashes latest pg 9.2 with the following stack trace i.e. >> Apparently it is being crashed because of invalid related pointer value of >> pfree() *header->context->methods->free_p. It is reproducible with latest >> version of python i.e. Python-2.7.3 and Python-3.2.3. Thanks. > This is because of this code: I traced through this test case. The situation is: (1) PLy_spi_execute_fetch_result is executed with CurrentMemoryContext being the SPI Proc context for the current SPI invocation, so that's where CreateTupleDescCopy creates the tupledesc that's placed into the PLyResultObject. (2) As we fall out of the SETOF function with the first result row, PLy_exec_function does SPI_finish() (plpy_exec.c:146). That causes the SPI contexts to go away. The PLyResultObject's tupdesc pointer is now pointing at freed memory, and in a cassert build, that memory has been actively wiped. (3) The main executor calls back to plpython for the next value-per-call result. Control goes to the PyIter_Next call at plpy_exec.c:108, which decides that we're done iterating (since this example only returns one row), and evidently it tries to deallocate the PLyResultObject immediately. Whether that happens immediately or later, though, you're screwed because FreeTupleDesc will be invoked on garbage. I'm inclined to think that the best fix is for PLy_spi_execute_fetch_result to copy the tupledesc into TopMemoryContext, not the current context. This is a tad scary from a memory leakage standpoint, but I suppose that if python fails to recover the PLyResultObject, this isn't the only memory that's going to be leaked. This area appears to be shy a regression test case or two, in any event. 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
Re: [HACKERS] plpython crash (PG 92)
Peter Eisentraut writes: > I must have been confused about the tuple descriptor APIs. ob->tupdesc > is created using CreateTupleDescCopy(), which copies the refcount of the > original tuple descriptor, Um, surely not. That would be nonsensical, and anyway a look at the code shows it isn't doing that. > thus causing a failure when the (seemingly still referenced) tupdesc > is freed. Is this behavior correct and useful? I think there must be some mistake in your analysis. 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
Re: [HACKERS] plpython crash (PG 92)
On tor, 2012-04-26 at 17:32 +0500, Asif Naeem wrote:
> PFA test case. It used simple select statement to retrieve data via
> plpython. It crashes latest pg 9.2 with the following stack trace i.e.
> Apparently it is being crashed because of invalid related pointer value of
> pfree() *header->context->methods->free_p. It is reproducible with latest
> version of python i.e. Python-2.7.3 and Python-3.2.3. Thanks.
This is because of this code:
static void
PLy_result_dealloc(PyObject *arg)
{
PLyResultObject *ob = (PLyResultObject *) arg;
Py_XDECREF(ob->nrows);
Py_XDECREF(ob->rows);
Py_XDECREF(ob->status);
if (ob->tupdesc)
{
FreeTupleDesc(ob->tupdesc); // <-- dies here
ob->tupdesc = NULL;
}
arg->ob_type->tp_free(arg);
}
I must have been confused about the tuple descriptor APIs. ob->tupdesc
is created using CreateTupleDescCopy(), which copies the refcount of the
original tuple descriptor, thus causing a failure when the (seemingly
still referenced) tupdesc is freed. Is this behavior correct and
useful?
The dominant coding practice appears to be to not explicitly free copied
tuple descriptors, so maybe that should be done here as well.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython triggers are broken for composite-type columns
On mån, 2012-04-23 at 02:25 +0200, Jan Urbański wrote: > It turned out not to be as straightforward as I though :( Yeah, been there ... > > The I/O code in PL/Python is a bit of a mess and that's something that > I'd like to address somewhere in the 9.3 development cycle. Right now > making the conversion function recursive is not possible without some > deep surgery (or kludgery...) so I limited myself to producing > regression-fixing patches for 9.2 and 9.1 (attached). Committed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash (PG 92)
FYI, I have observed this crash on Linux64. Thanks. Best Regards, Muhammad Asif Naeem On Thu, Apr 26, 2012 at 5:32 PM, Asif Naeem wrote: > Hi, > > PFA test case. It used simple select statement to retrieve data via > plpython. It crashes latest pg 9.2 with the following stack trace i.e. > > #0 0x0073021f in pfree () >> #1 0x7fa74b632f7a in PLy_result_dealloc () from >> /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so >> #2 0x7fa74b2c710b in iter_iternext (iterator=0x1ad7150) at >> Objects/iterobject.c:74 >> #3 0x7fa74b2934db in PyIter_Next (iter=0x1b3c5f0) at >> Objects/abstract.c:3107 >> #4 0x7fa74b630245 in PLy_exec_function () from >> /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so >> #5 0x7fa74b630c57 in plpython_call_handler () from >> /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so >> #6 0x00583907 in ExecMakeFunctionResult () >> #7 0x0057f146 in ExecProject () >> #8 0x00596740 in ExecResult () >> #9 0x0057e708 in ExecProcNode () >> #10 0x0057d582 in standard_ExecutorRun () >> #11 0x0064f477 in PortalRunSelect () >> #12 0x00650778 in PortalRun () >> #13 0x0064ceca in exec_simple_query () >> #14 0x0064ddc7 in PostgresMain () >> #15 0x0060bdd9 in ServerLoop () >> #16 0x0060e9d7 in PostmasterMain () >> #17 0x005ad360 in main () > > > Apparently it is being crashed because of invalid related pointer value of > pfree() *header->context->methods->free_p. It is reproducible with latest > version of python i.e. Python-2.7.3 and Python-3.2.3. Thanks. > > Best Regards, > Muhammad Asif Naeem >
[HACKERS] plpython crash (PG 92)
Hi, PFA test case. It used simple select statement to retrieve data via plpython. It crashes latest pg 9.2 with the following stack trace i.e. #0 0x0073021f in pfree () > #1 0x7fa74b632f7a in PLy_result_dealloc () from > /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so > #2 0x7fa74b2c710b in iter_iternext (iterator=0x1ad7150) at > Objects/iterobject.c:74 > #3 0x7fa74b2934db in PyIter_Next (iter=0x1b3c5f0) at > Objects/abstract.c:3107 > #4 0x7fa74b630245 in PLy_exec_function () from > /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so > #5 0x7fa74b630c57 in plpython_call_handler () from > /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so > #6 0x00583907 in ExecMakeFunctionResult () > #7 0x0057f146 in ExecProject () > #8 0x00596740 in ExecResult () > #9 0x0057e708 in ExecProcNode () > #10 0x0057d582 in standard_ExecutorRun () > #11 0x0064f477 in PortalRunSelect () > #12 0x00650778 in PortalRun () > #13 0x0064ceca in exec_simple_query () > #14 0x0064ddc7 in PostgresMain () > #15 0x0060bdd9 in ServerLoop () > #16 0x0060e9d7 in PostmasterMain () > #17 0x005ad360 in main () Apparently it is being crashed because of invalid related pointer value of pfree() *header->context->methods->free_p. It is reproducible with latest version of python i.e. Python-2.7.3 and Python-3.2.3. Thanks. Best Regards, Muhammad Asif Naeem plpython_crash.sql Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython triggers are broken for composite-type columns
On 10/04/12 21:47, Jan Urbański wrote:
On 10/04/12 21:27, Tom Lane wrote:
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes:
Yes, that would be ideal, even though not backwards-compatible.
Back-patching is out of the question, but do we want to change trigger
functions to receive dictionaries in NEW?
Hm, I was not thinking of this as being trigger-specific, but more a
general principle that composite columns of tuples ought to be handled
in a recursive fashion.
Sure, that would be the way.
If so, should this be 9.2 material, or just a TODO?
If it can be done quickly and with not much risk, I'd vote for
squeezing it into 9.2, because it seems to me to be a clear bug that the
two directions are not handled consistently. If you don't have time for
it now or you don't think it would be a small/safe patch, we'd better
just put it on TODO.
It turned out not to be as straightforward as I though :(
The I/O code in PL/Python is a bit of a mess and that's something that
I'd like to address somewhere in the 9.3 development cycle. Right now
making the conversion function recursive is not possible without some
deep surgery (or kludgery...) so I limited myself to producing
regression-fixing patches for 9.2 and 9.1 (attached).
Cheers,
Jan
>From 513e8c484f32599a8753035fad638c8712339480 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?=
Date: Mon, 23 Apr 2012 02:07:46 +0200
Subject: [PATCH] Accept strings in PL/Python functions returning composite
types.
Before 9.1, PL/Python functions returning composite types could return
a string and it would be parsed using record_in. The 9.1 changes made
PL/Python only expect dictionaries, tuples or objects supporting
getattr as output of composite functions, resulting in a regression
and a confusing error message, as the strings were interpreted as
sequences and the code for transforming lists Postgres tuples was
being used.
The reason why it's important is that trigger functions on tables with
composite columns get the composite row passed in as a string (from
record_out). This makes it impossible to implement passthrough
behaviour for these columns, as PL/Python no longer accepts strings
for composite values. A better solution would be to fix the code that
transforms composite inputs into Python objecst to produce
dictionaries that would then be correctly interpreted by the
Python->Postgres counterpart code. This would be too invasive to
backpatch in 9.1 and it is too late in the 9.2 cycle to do it in
HEAD. It should get revisited in 9.3, though.
Reported as bug #6559 by Kirill Simonov.
---
src/pl/plpython/expected/plpython_record.out | 16
src/pl/plpython/expected/plpython_trigger.out | 37 +
src/pl/plpython/plpython.c| 94 ---
src/pl/plpython/regression.diffs | 99 +
src/pl/plpython/regression.out| 20 +
src/pl/plpython/sql/plpython_record.sql | 10 +++
src/pl/plpython/sql/plpython_trigger.sql | 36 +
7 files changed, 268 insertions(+), 44 deletions(-)
create mode 100644 src/pl/plpython/regression.diffs
create mode 100644 src/pl/plpython/regression.out
diff --git a/src/pl/plpython/expected/plpython_record.out b/src/pl/plpython/expected/plpython_record.out
index 0bcc46c..4583307 100644
--- a/src/pl/plpython/expected/plpython_record.out
+++ b/src/pl/plpython/expected/plpython_record.out
@@ -38,6 +38,8 @@ elif typ == 'obj':
type_record.first = first
type_record.second = second
return type_record
+elif typ == 'str':
+ return "('%s',%r)" % (first, second)
$$ LANGUAGE plpythonu;
CREATE FUNCTION test_in_out_params(first in text, second out text) AS $$
return first + '_in_to_out';
@@ -290,6 +292,12 @@ SELECT * FROM test_type_record_as('obj', null, null, true);
|
(1 row)
+SELECT * FROM test_type_record_as('str', 'one', 1, false);
+ first | second
+---+
+ 'one' | 1
+(1 row)
+
SELECT * FROM test_in_out_params('test_in');
second
---
@@ -355,3 +363,11 @@ ERROR: attribute "second" does not exist in Python object
HINT: To return null in a column, let the returned object have an attribute named after column with value None.
CONTEXT: while creating return value
PL/Python function "test_type_record_error3"
+CREATE FUNCTION test_type_record_error4() RETURNS type_record AS $$
+return 'foo'
+$$ LANGUAGE plpythonu;
+SELECT * FROM test_type_record_error4();
+ERROR: malformed record literal: "foo"
+DETAIL: Missing left parenthesis.
+CONTEXT: while creating return value
+PL/Python function "test_type_record_error4"
diff --git a/src/pl/plpython/expected/plpython_trigger.out b/src/pl/plpython/expected/plpython_trigger.out
index 2ef66a8..ea4d500 100644
--- a/src/pl/plpython/expected/plpython_trigger.out
+++ b/src/pl/plpython/expected/plpython_trigger.out
@@ -567,3 +567,40 @@ SELECT * FROM composite_trigger_test;
(3,f) | (7,t)
(1 row)
+-- bug
Re: [HACKERS] plpython triggers are broken for composite-type columns
On 10/04/12 21:27, Tom Lane wrote: =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes: Yes, that would be ideal, even though not backwards-compatible. Back-patching is out of the question, but do we want to change trigger functions to receive dictionaries in NEW? Hm, I was not thinking of this as being trigger-specific, but more a general principle that composite columns of tuples ought to be handled in a recursive fashion. Sure, that would be the way. If so, should this be 9.2 material, or just a TODO? If it can be done quickly and with not much risk, I'd vote for squeezing it into 9.2, because it seems to me to be a clear bug that the two directions are not handled consistently. If you don't have time for it now or you don't think it would be a small/safe patch, we'd better just put it on TODO. I'll see if making the conversion function recursive is easy and independently whip up a patch to check for strings and routes them through InputFunctionCall, for back-patching purposes. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython triggers are broken for composite-type columns
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes: > On 10/04/12 20:47, Tom Lane wrote: >> On reflection, can't we fix this as follows: if the value coming in from >> Python is a string, just feed it to record_in, the same as we used to. >> When I traced through the logic before, it seemed like it was failing >> to distinguish strings from sequences, but I would hope that Python >> is more strongly typed than that. > Yeah, we can fix PLyObject_ToTuple to check for strings too and use the > default PG input function. The reason it was complaining about length is > that we're checking if the object passed implements the sequence > protocol, which Python strings do (length, iteration, etc). Sticking a > if branch that will catch the string case above that should be sufficient. Ah, makes sense then. (Perhaps the dict case ought to be tested before the sequence case, too, just to be safe?) >> I still think the conversion in the other direction ought to yield a >> dict, but that's clearly not back-patch material. > Yes, that would be ideal, even though not backwards-compatible. > Back-patching is out of the question, but do we want to change trigger > functions to receive dictionaries in NEW? Hm, I was not thinking of this as being trigger-specific, but more a general principle that composite columns of tuples ought to be handled in a recursive fashion. > If so, should this be 9.2 material, or just a TODO? If it can be done quickly and with not much risk, I'd vote for squeezing it into 9.2, because it seems to me to be a clear bug that the two directions are not handled consistently. If you don't have time for it now or you don't think it would be a small/safe patch, we'd better just put it on TODO. 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
Re: [HACKERS] plpython triggers are broken for composite-type columns
On 10/04/12 20:47, Tom Lane wrote: I wrote: =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes: Now that I understand what's been going on, I'll try to think of a non-invasive way of fixing that... ISTM that conversion of a composite value to Python ought to produce a dict, now that the other direction expects a dict. I can see that this is probably infeasible for compatibility reasons in 9.1, but it's not too late to fix it for 9.2. We might have to leave the bug unfixed in 9.1, since anything we do about it will represent a compatibility break. On reflection, can't we fix this as follows: if the value coming in from Python is a string, just feed it to record_in, the same as we used to. When I traced through the logic before, it seemed like it was failing to distinguish strings from sequences, but I would hope that Python is more strongly typed than that. Yeah, we can fix PLyObject_ToTuple to check for strings too and use the default PG input function. The reason it was complaining about length is that we're checking if the object passed implements the sequence protocol, which Python strings do (length, iteration, etc). Sticking a if branch that will catch the string case above that should be sufficient. I still think the conversion in the other direction ought to yield a dict, but that's clearly not back-patch material. Yes, that would be ideal, even though not backwards-compatible. Back-patching is out of the question, but do we want to change trigger functions to receive dictionaries in NEW? If so, should this be 9.2 material, or just a TODO? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython triggers are broken for composite-type columns
I wrote: > =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes: >> Now that I understand what's been going on, I'll try to think of a >> non-invasive way of fixing that... > ISTM that conversion of a composite value to Python ought to produce a > dict, now that the other direction expects a dict. I can see that this > is probably infeasible for compatibility reasons in 9.1, but it's not > too late to fix it for 9.2. We might have to leave the bug unfixed in > 9.1, since anything we do about it will represent a compatibility break. On reflection, can't we fix this as follows: if the value coming in from Python is a string, just feed it to record_in, the same as we used to. When I traced through the logic before, it seemed like it was failing to distinguish strings from sequences, but I would hope that Python is more strongly typed than that. I still think the conversion in the other direction ought to yield a dict, but that's clearly not back-patch material. 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
Re: [HACKERS] plpython triggers are broken for composite-type columns
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes:
>> On 10/04/12 04:20, Tom Lane wrote:
>>> Don't know if anybody noticed bug #6559
>>> http://archives.postgresql.org/pgsql-bugs/2012-03/msg00180.php
> So, I know what's going on, I still don't know what's the best way to
> handle it.
> The function that converts Python objects to PG data checks what type
> it's supposed to produce and acts accordingly. In 9.0 it checked for
> bool, bytea and arrays, in 9.1 it also takes composite types into account.
> This has been done to support functions returning composite types - to
> do that they need to return a dictionary or a list, for instance
> {'col1': 1, 'col2': 2}.
> The problem is that the routine that converts PG data into Python
> objects does not handle composite type inputs all that well - it just
> bails and returns the string representation, hence '(3)' appearing in
> Python land.
> Now previously, the Python->PG function did not see that the given
> conversion is supposed to return a composite so it also bailed and used
> a default text->composite conversion, so '(3)' was converted to ROW(3)
> and all went well. The new code tries to treat what it gets as a
> dictionary/list/tuple and fails in a more or less random way.
> Now that I understand what's been going on, I'll try to think of a
> non-invasive way of fixing that...
ISTM that conversion of a composite value to Python ought to produce a
dict, now that the other direction expects a dict. I can see that this
is probably infeasible for compatibility reasons in 9.1, but it's not
too late to fix it for 9.2. We might have to leave the bug unfixed in
9.1, since anything we do about it will represent a compatibility break.
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
Re: [HACKERS] plpython triggers are broken for composite-type columns
On 10/04/12 07:35, Jan Urbański wrote:
On 10/04/12 04:20, Tom Lane wrote:
Don't know if anybody noticed bug #6559
http://archives.postgresql.org/pgsql-bugs/2012-03/msg00180.php
I've confirmed that the given test case works in 9.0 but fails in
9.1 and HEAD.
So, I know what's going on, I still don't know what's the best way to
handle it.
The function that converts Python objects to PG data checks what type
it's supposed to produce and acts accordingly. In 9.0 it checked for
bool, bytea and arrays, in 9.1 it also takes composite types into account.
This has been done to support functions returning composite types - to
do that they need to return a dictionary or a list, for instance
{'col1': 1, 'col2': 2}.
The problem is that the routine that converts PG data into Python
objects does not handle composite type inputs all that well - it just
bails and returns the string representation, hence '(3)' appearing in
Python land.
Now previously, the Python->PG function did not see that the given
conversion is supposed to return a composite so it also bailed and used
a default text->composite conversion, so '(3)' was converted to ROW(3)
and all went well. The new code tries to treat what it gets as a
dictionary/list/tuple and fails in a more or less random way.
Now that I understand what's been going on, I'll try to think of a
non-invasive way of fixing that...
Cheers,
Jan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython triggers are broken for composite-type columns
On 10/04/12 04:20, Tom Lane wrote: Don't know if anybody noticed bug #6559 http://archives.postgresql.org/pgsql-bugs/2012-03/msg00180.php I've confirmed that the given test case works in 9.0 but fails in 9.1 and HEAD. I find this code pretty unreadable, though, and know nothing to speak of about the Python side of things anyhow. So somebody else had better pick this up. I'll look into that. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] plpython triggers are broken for composite-type columns
Don't know if anybody noticed bug #6559 http://archives.postgresql.org/pgsql-bugs/2012-03/msg00180.php I've confirmed that the given test case works in 9.0 but fails in 9.1 and HEAD. It's not terribly sensitive to the details of the SQL: any non-null value for the composite column fails, for instance you can try INSERT INTO tbl VALUES (row(3), 4); and it spits up just the same. The long and the short of it is that PLy_modify_tuple fails to make sense of what PLyDict_FromTuple produced for the table row. I tried to trace through things to see exactly where it was going wrong, and noted that (1) When converting the table row to a Python dict, the composite column value is fed through the generic PLyString_FromDatum() function, which seems likely to be the wrong choice. (2) When converting back, the composite column value is routed to PLyObject_ToTuple, which decides it is a Python sequence, which seems a bit improbable considering it was merely a string a moment ago. I find this code pretty unreadable, though, and know nothing to speak of about the Python side of things anyhow. So somebody else had better pick this up. 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
Re: [HACKERS] plpython SPI cursors
On mån, 2011-12-05 at 13:12 -0500, Bruce Momjian wrote: > Jan Urbański wrote: > > On 05/12/11 18:58, Peter Eisentraut wrote: > > > On ons, 2011-11-23 at 19:58 +0100, Jan Urba?ski wrote: > > >> On 20/11/11 19:14, Steve Singer wrote: > > >> Responding now to all questions and attaching a revised patch based on > > >> your comments. > > > > > > Committed > > > > > > Please refresh the other patch. > > > > Great, thanks! > > > > I'll try to send an updated version of the other patch this evening. > > I assume this is _not_ related to this TODO item: > > Add a DB-API compliant interface on top of the SPI interface No, but this is: http://petereisentraut.blogspot.com/2011/11/plpydbapi-db-api-for-plpython.html -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython SPI cursors
On 05/12/11 19:12, Bruce Momjian wrote: > Jan Urbański wrote: >> On 05/12/11 18:58, Peter Eisentraut wrote: >>> On ons, 2011-11-23 at 19:58 +0100, Jan Urba?ski wrote: On 20/11/11 19:14, Steve Singer wrote: Responding now to all questions and attaching a revised patch based on your comments. >>> >>> Committed >>> >>> Please refresh the other patch. >> >> Great, thanks! >> >> I'll try to send an updated version of the other patch this evening. > > I assume this is _not_ related to this TODO item: > > Add a DB-API compliant interface on top of the SPI interface No, not related. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython SPI cursors
Jan Urba??ski wrote: > On 05/12/11 18:58, Peter Eisentraut wrote: > > On ons, 2011-11-23 at 19:58 +0100, Jan Urba?ski wrote: > >> On 20/11/11 19:14, Steve Singer wrote: > >> Responding now to all questions and attaching a revised patch based on > >> your comments. > > > > Committed > > > > Please refresh the other patch. > > Great, thanks! > > I'll try to send an updated version of the other patch this evening. I assume this is _not_ related to this TODO item: Add a DB-API compliant interface on top of the SPI interface -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython SPI cursors
On 05/12/11 18:58, Peter Eisentraut wrote: > On ons, 2011-11-23 at 19:58 +0100, Jan Urbański wrote: >> On 20/11/11 19:14, Steve Singer wrote: >> Responding now to all questions and attaching a revised patch based on >> your comments. > > Committed > > Please refresh the other patch. Great, thanks! I'll try to send an updated version of the other patch this evening. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython SPI cursors
On ons, 2011-11-23 at 19:58 +0100, Jan Urbański wrote: > On 20/11/11 19:14, Steve Singer wrote: > > On 11-10-15 07:28 PM, Jan Urbański wrote: > >> Hi, > >> > >> attached is a patch implementing the usage of SPI cursors in PL/Python. > >> Currently when trying to process a large table in PL/Python you have > >> slurp it all into memory (that's what plpy.execute does). > >> > >> J > > > > I found a few bugs (see my testing section below) that will need fixing > > + a few questions about the code > > Responding now to all questions and attaching a revised patch based on > your comments. Committed Please refresh the other patch. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython SPI cursors
On lör, 2011-11-26 at 11:21 -0500, Steve Singer wrote: > I've looked over the revised version of the patch and it now seems > fine. > > Ready for committer. I can take it from here. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython SPI cursors
On 11-11-23 01:58 PM, Jan Urbański wrote:
On 20/11/11 19:14, Steve Singer wrote:
On 11-10-15 07:28 PM, Jan Urbański wrote:
Hi,
attached is a patch implementing the usage of SPI cursors in PL/Python.
Currently when trying to process a large table in PL/Python you have
slurp it all into memory (that's what plpy.execute does).
J
I found a few bugs (see my testing section below) that will need fixing
+ a few questions about the code
Responding now to all questions and attaching a revised patch based on
your comments.
I've looked over the revised version of the patch and it now seems fine.
Ready for committer.
Do we like the name plpy.cursor or would we rather call it something
like
plpy.execute_cursor(...) or plpy.cursor_open(...) or
plpy.create_cursor(...)
Since we will be mostly stuck with the API once we release 9.2 this is
worth
some opinions on. I like cursor() but if anyone disagrees now is the
time.
We use plpy.subtransaction() to create Subxact objects, so I though
plpy.cursor() would be most appropriate.
This patch does not provide a wrapper around SPI_cursor_move. The patch
is useful without that and I don't see anything that preculdes
someone else
adding that later if they see a need.
My idea is to add keyword arguments to plpy.cursor() that will allow
you to decide whether you want a scrollable cursor and after that
provide a move() method.
The patch includes documentation updates that describes the new feature.
The Database Access page doesn't provide a API style list of database
access
functions like the plperl
http://www.postgresql.org/docs/9.1/interactive/plperl-builtins.html page
does. I think the organization of the perl page is
clearer than the python one and we should think about a doing some
documentaiton refactoring. That should be done as a seperate patch and
shouldn't be a barrier to committing this one.
Yeah, the PL/Python docs are a bit chaotic right now. I haven't yet
summoned force to overhaul them.
in PLy_cursor_plan line 4080
+ PG_TRY();
+ {
+ Portal portal;
+ char *volatile nulls;
+ volatile int j;
I am probably not seeing a code path or misunderstanding something
about the setjmp/longjump usages but I don't see why nulls and j need
to be
volatile here.
It looked like you could drop volatile there (and in
PLy_spi_execute_plan, where this is copied from (did I mention there's
quite some code duplication in PL/Python?)) but digging in git I found
this commit:
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=2789b7278c11785750dd9d2837856510ffc67000
that added the original volatile qualification, so I guess there's a
reason.
line 444
PLy_cursor(PyObject *self, PyObject *args)
+ {
+ char *query;
+ PyObject *plan;
+ PyObject *planargs = NULL;
+
+ if (PyArg_ParseTuple(args, "s", &query))
+ return PLy_cursor_query(query);
+
Should query be freed with PyMem_free()
No, PyArg_ParseTuple returns a string on the stack, I check that
repeatedly creating a cursor with a plan argument does not leak memory
and that adding PyMem_Free there promptly leads to a segfault.
I tested both python 2.6 and 3 on a Linux system
[test cases demonstrating bugs]
Turns out it's a really bad idea to store pointers to Portal
structures, because they get invalidated by the subtransaction abort
hooks.
I switched to storing the cursor name and looking it up in the
appropriate hash table every time it's used. The examples you sent
(which I included as regression tests) now cause a ValueError to be
raised with a message stating that the cursor has been created in an
aborted subtransaction.
Not sure about the wording of the error message, though.
Thanks again for the review!
Cheers,
Jan
Re: [HACKERS] plpython SPI cursors
On 20/11/11 19:14, Steve Singer wrote:
On 11-10-15 07:28 PM, Jan Urbański wrote:
Hi,
attached is a patch implementing the usage of SPI cursors in PL/Python.
Currently when trying to process a large table in PL/Python you have
slurp it all into memory (that's what plpy.execute does).
J
I found a few bugs (see my testing section below) that will need fixing
+ a few questions about the code
Responding now to all questions and attaching a revised patch based on
your comments.
Do we like the name plpy.cursor or would we rather call it something like
plpy.execute_cursor(...) or plpy.cursor_open(...) or
plpy.create_cursor(...)
Since we will be mostly stuck with the API once we release 9.2 this is
worth
some opinions on. I like cursor() but if anyone disagrees now is the time.
We use plpy.subtransaction() to create Subxact objects, so I though
plpy.cursor() would be most appropriate.
This patch does not provide a wrapper around SPI_cursor_move. The patch
is useful without that and I don't see anything that preculdes someone else
adding that later if they see a need.
My idea is to add keyword arguments to plpy.cursor() that will allow you
to decide whether you want a scrollable cursor and after that provide a
move() method.
The patch includes documentation updates that describes the new feature.
The Database Access page doesn't provide a API style list of database
access
functions like the plperl
http://www.postgresql.org/docs/9.1/interactive/plperl-builtins.html page
does. I think the organization of the perl page is
clearer than the python one and we should think about a doing some
documentaiton refactoring. That should be done as a seperate patch and
shouldn't be a barrier to committing this one.
Yeah, the PL/Python docs are a bit chaotic right now. I haven't yet
summoned force to overhaul them.
in PLy_cursor_plan line 4080
+ PG_TRY();
+ {
+ Portal portal;
+ char *volatile nulls;
+ volatile int j;
I am probably not seeing a code path or misunderstanding something
about the setjmp/longjump usages but I don't see why nulls and j need to be
volatile here.
It looked like you could drop volatile there (and in
PLy_spi_execute_plan, where this is copied from (did I mention there's
quite some code duplication in PL/Python?)) but digging in git I found
this commit:
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=2789b7278c11785750dd9d2837856510ffc67000
that added the original volatile qualification, so I guess there's a reason.
line 444
PLy_cursor(PyObject *self, PyObject *args)
+ {
+ char *query;
+ PyObject *plan;
+ PyObject *planargs = NULL;
+
+ if (PyArg_ParseTuple(args, "s", &query))
+ return PLy_cursor_query(query);
+
Should query be freed with PyMem_free()
No, PyArg_ParseTuple returns a string on the stack, I check that
repeatedly creating a cursor with a plan argument does not leak memory
and that adding PyMem_Free there promptly leads to a segfault.
I tested both python 2.6 and 3 on a Linux system
[test cases demonstrating bugs]
Turns out it's a really bad idea to store pointers to Portal structures,
because they get invalidated by the subtransaction abort hooks.
I switched to storing the cursor name and looking it up in the
appropriate hash table every time it's used. The examples you sent
(which I included as regression tests) now cause a ValueError to be
raised with a message stating that the cursor has been created in an
aborted subtransaction.
Not sure about the wording of the error message, though.
Thanks again for the review!
Cheers,
Jan
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index eda2bbf..d08c3d1 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -892,6 +892,15 @@ $$ LANGUAGE plpythonu;
+Note that calling plpy.execute will cause the entire
+result set to be read into memory. Only use that function when you are sure
+that the result set will be relatively small. If you don't want to risk
+excessive memory usage when fetching large results,
+use plpy.cursor rather
+than plpy.execute.
+
+
+
For example:
rv = plpy.execute("SELECT * FROM my_table", 5)
@@ -958,6 +967,77 @@ $$ LANGUAGE plpythonu;
+
+Accessing data with cursors
+
+
+The plpy.cursor function accepts the same arguments
+as plpy.execute (except for limit)
+and returns a cursor object, which allows you to process large result sets
+in smaller chunks. As with plpy.execute, either a query
+string or a plan object along with a list of arguments can be used. The
+cursor object provides a fetch method that accepts an
+integer paramter and returns a result object. Each time you
+call fetch, the returned object will contain the next
+batch of rows, never larger than the parameter value. Once all rows are
+exhausted, fetch starts returning an empty result
+object. Cursor objects also provide an
+http://docs.py
Re: [HACKERS] plpython SPI cursors
On 20/11/11 19:14, Steve Singer wrote: On 11-10-15 07:28 PM, Jan Urbański wrote: Hi, attached is a patch implementing the usage of SPI cursors in PL/Python. I found a few bugs (see my testing section below) that will need fixing + a few questions about the code Hi Steve, thanks a lot for the review, I'll investigate the errors you were getting and post a follow-up. Good catch on trying cursors with explicit subtransactions, I didn't think about how they would interact. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython SPI cursors
On 11-10-15 07:28 PM, Jan Urbański wrote:
Hi,
attached is a patch implementing the usage of SPI cursors in PL/Python.
Currently when trying to process a large table in PL/Python you have
slurp it all into memory (that's what plpy.execute does).
J
I found a few bugs (see my testing section below) that will need fixing
+ a few questions about the code
Overview & Feature Review
---
This patch adds cursor support to plpython. SPI cursors will allow
a plpython function to read process a large results set without having to
read it all into memory at once. This is a good thing. Without this
patch I think you could accomplish the same with using SQL DECLARE CURSOR
and SQL fetch.This feature allows you to use a python cursor as
an iterator resulting in much cleaner python code than the SQL FETCH
approach. I think the feature is worth having
Usability Review
--
The patch adds the user methods
cursor=plpy.cursor(query_or_plan)
cursor.fetch(100)
cursor.close()
Do we like the name plpy.cursor or would we rather call it something like
plpy.execute_cursor(...) or plpy.cursor_open(...) or plpy.create_cursor(...)
Since we will be mostly stuck with the API once we release 9.2 this is worth
some opinions on. I like cursor() but if anyone disagrees now is the time.
This patch does not provide a wrapper around SPI_cursor_move. The patch
is useful without that and I don't see anything that preculdes someone else
adding that later if they see a need.
Documentation Review
-
The patch includes documentation updates that describes the new feature.
The Database Access page doesn't provide a API style list of database access
functions like the plperl
http://www.postgresql.org/docs/9.1/interactive/plperl-builtins.html page
does. I think the organization of the perl page is
clearer than the python one and we should think about a doing some
documentaiton refactoring. That should be done as a seperate patch and
shouldn't be a barrier to committing this one.
Code Review
in PLy_cursor_plan line 4080
+ PG_TRY();
+ {
+ Portalportal;
+ char *volatile nulls;
+ volatile int j;
+
+ if (nargs > 0)
+ nulls = palloc(nargs * sizeof(char));
+ else
+ nulls = NULL;
+
+ for (j = 0; j < nargs; j++)
+ {
+ PyObject *elem;
I am probably not seeing a code path or misunderstanding something
about the setjmp/longjump usages but I don't see why nulls and j need to be
volatile here.
line 444
PLy_cursor(PyObject *self, PyObject *args)
+ {
+ char*query;
+ PyObject*plan;
+ PyObject *planargs = NULL;
+
+ if (PyArg_ParseTuple(args, "s", &query))
+ return PLy_cursor_query(query);
+
Should query be freed with PyMem_free()
Testing
---
I tested both python 2.6 and 3 on a Linux system
create or replace function x() returns text as $$
cur=None
try:
with plpy.subtransaction():
cur=plpy.cursor('select generate_series(1,1000)')
rows=cur.fetch(10);
plpy.execute('select f()')
except plpy.SPIError:
rows=cur.fetch(10);
return rows[0]['generate_series']
return 'none'
$$ language plpythonu;
select x();
crashes the backend
test=# select x();
The connection to the server was lost. Attempting reset: LOG: server
process (PID 3166) was terminated by signal 11: Segmentation fault
The below test gives me a strange error message:
create or replace function x1() returns text as $$
plan=None
try:
with plpy.subtransaction():
plpy.execute('CREATE TEMP TABLE z AS select generate_series(1,1000)')
plan=plpy.prepare('select * FROM z')
plpy.execute('select * FROM does_not_exist')
except plpy.SPIError, e:
cur=plpy.cursor(plan)
rows=cur.fetch(10)
return rows[0]['generate_series']
return '1'
$$ language plpythonu;
select x1();
test=# select x1()
test-# ;
ERROR: TypeError: Expected sequence of 82187072 arguments, got 0:
CONTEXT: Traceback (most recent call last):
PL/Python function "x1", line 9, in
cur=plpy.cursor(plan)
PL/Python function "x1"
STATEMENT: select x1()
I was expecting an error from the function just a bit more useful one.
Performance
---
I did not do any specific performance testing but I don't see this patch
as having any impact to performance
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython extension control files installation
On Sat, Nov 5, 2011 at 10:42 AM, Peter Eisentraut wrote: > We only build the language plpython2u or plpython3u, not both, in any > build, but we always install the extension control files for all > variants. Is there a reason for this, or just an oversight? Sounds like an oversight. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] plpython extension control files installation
We only build the language plpython2u or plpython3u, not both, in any build, but we always install the extension control files for all variants. Is there a reason for this, or just an oversight? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] plpython SPI cursors
Hi,
attached is a patch implementing the usage of SPI cursors in PL/Python.
Currently when trying to process a large table in PL/Python you have
slurp it all into memory (that's what plpy.execute does).
This patch allows reading the result set in smaller chunks, using a SPI
cursor behind the scenes.
Example usage:
cursor = plpy.cursor("select a, b from hugetable")
for row in cursor:
plpy.info("a is %s and b is %s" % (row['a'], row['b']))
The patch itself is simple, but there's a lot of boilerplate dedicated
to opening a subtransaction and handling prepared plans. I'd like to do
some refactoring of they way PL/Python uses SPI to reduce the amount of
boilerplate needed, but that'll come as a separate patch (just before
the patch to split plpython.c in smaller chunks).
This feature has been sponsored by Nomao.
Cheers,
Jan
PS: I already added it to the November CF.
J
>From 9ad14957e7b4ae19667df3bb8cc2aa5ef5bf96c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?=
Date: Tue, 13 Sep 2011 14:42:41 +0200
Subject: [PATCH] Add cursor support to plpythonu.
Exposes SPI cursors as plpythonu objects allowing processing large
result sets without loading them entirely into memory, as plpy.execute
is doing.
---
doc/src/sgml/plpython.sgml | 80
src/pl/plpython/expected/plpython_spi.out | 151 +++
src/pl/plpython/expected/plpython_test.out |6 +-
src/pl/plpython/plpython.c | 605
src/pl/plpython/sql/plpython_spi.sql | 116 ++
5 files changed, 955 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index eda2bbf..d08c3d1 100644
*** a/doc/src/sgml/plpython.sgml
--- b/doc/src/sgml/plpython.sgml
*** $$ LANGUAGE plpythonu;
*** 892,897
--- 892,906
+ Note that calling plpy.execute will cause the entire
+ result set to be read into memory. Only use that function when you are sure
+ that the result set will be relatively small. If you don't want to risk
+ excessive memory usage when fetching large results,
+ use plpy.cursor rather
+ than plpy.execute.
+
+
+
For example:
rv = plpy.execute("SELECT * FROM my_table", 5)
*** $$ LANGUAGE plpythonu;
*** 958,963
--- 967,1043
+
+ Accessing data with cursors
+
+
+ The plpy.cursor function accepts the same arguments
+ as plpy.execute (except for limit)
+ and returns a cursor object, which allows you to process large result sets
+ in smaller chunks. As with plpy.execute, either a query
+ string or a plan object along with a list of arguments can be used. The
+ cursor object provides a fetch method that accepts an
+ integer paramter and returns a result object. Each time you
+ call fetch, the returned object will contain the next
+ batch of rows, never larger than the parameter value. Once all rows are
+ exhausted, fetch starts returning an empty result
+ object. Cursor objects also provide an
+ http://docs.python.org/library/stdtypes.html#iterator-types";>iterator
+ interface, yielding one row at a time until all rows are exhausted.
+ Data fetched that way is not returned as result objects, but rather as
+ dictionaries, each dictionary corresponding to a single result row.
+
+
+
+ Cursors are automatically disposed of, but if you want to explicitly
+ release all resources held by a cursor, use the close
+ method. Once closed, a cursor cannot be fetched from anymore.
+
+
+
+
+ Do not confuse objects created by plpy.cursor with
+ DBAPI cursors as defined by
+ the http://www.python.org/dev/peps/pep-0249/";>Python Database API specification.
+ They don't have anything in common except for the name.
+
+
+
+
+ An example of two ways of processing data from a large table would be:
+
+ CREATE FUNCTION count_odd_iterator() RETURNS integer AS $$
+ odd = 0
+ for row in plpy.cursor("select num from largetable"):
+ if row['num'] % 2:
+ odd += 1
+ return odd
+ $$ LANGUAGE plpythonu;
+
+ CREATE FUNCTION count_odd_fetch(batch_size integer) RETURNS integer AS $$
+ odd = 0
+ cursor = plpy.cursor("select num from largetable")
+ while True:
+ rows = cursor.fetch(batch_size)
+ if not rows:
+ break
+ for row in rows:
+ if row['num'] % 2:
+ odd += 1
+ return odd
+ $$ LANGUAGE plpythonu;
+
+ CREATE FUNCTION count_odd_prepared() RETURNS integer AS $$
+ odd = 0
+ plan = plpy.prepare("select num from largetable where num % $1 != 0", ["integer"])
+ rows = list(plpy.cursor(plan, [2]))
+
+ return len(rows)
+ $$ LANGUAGE plpythonu;
+
+
+
+
Trapping Errors
diff --git a/src/pl/plpython/expected/plpython_spi.out b/src/pl/plpython/expected/plpython_spi.out
index 7f4ae5c..3b4d7a3 100644
*** a/src/pl/plpython/expected/plpython_spi.out
--
Re: [HACKERS] plpython crash
On 17/08/11 23:10, Tom Lane wrote: > =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes: >> Here are two patches that fix two separate bugs that you found >> simultaneously. Because they're actually separate issues, it turned out >> fixing them was a bit more tricky than I expected (fixing one was >> unmasking the other one etc). > > Applied with one non-cosmetic change: I got rid of the test on > TransactionIdIsValid(arg->typrel_xmin) in PLy_input_tuple_funcs, > as well as where you'd copied that logic in PLy_output_tuple_funcs. > AFAICS skipping the update on the xmin/tid, if we're coming through > there a second time, would be simply wrong. Thanks! The way things are set up now I think you never go through PLy_input_tuple_funcs twice, unless the cache is determined to be invalid and then you recreate the function from scratch. But of course it's better to be safe than sorry and even if I'm right and it was never executed twice, any refactoring effort might have broken it easily. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes: > Here are two patches that fix two separate bugs that you found > simultaneously. Because they're actually separate issues, it turned out > fixing them was a bit more tricky than I expected (fixing one was > unmasking the other one etc). Applied with one non-cosmetic change: I got rid of the test on TransactionIdIsValid(arg->typrel_xmin) in PLy_input_tuple_funcs, as well as where you'd copied that logic in PLy_output_tuple_funcs. AFAICS skipping the update on the xmin/tid, if we're coming through there a second time, would be simply wrong. 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
Re: [HACKERS] plpython crash
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes: >> On 16/08/11 19:07, Jean-Baptiste Quenot wrote: >>> [plpython is buggy] > Here are two patches that fix two separate bugs that you found > simultaneously. Because they're actually separate issues, it turned out > fixing them was a bit more tricky than I expected (fixing one was > unmasking the other one etc). These look generally sane although I have some minor stylistic gripes. Will clean them up and apply in a few hours (I have to leave for an appointment shortly). 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
Re: [HACKERS] plpython crash
On 17/08/11 11:40, Jan Urbański wrote:
> On 16/08/11 19:12, Jan Urbański wrote:
>> On 16/08/11 19:07, Jean-Baptiste Quenot wrote:
>>>
>>> [plpython is buggy]
> I'll have a patch ready soon.
Here are two patches that fix two separate bugs that you found
simultaneously. Because they're actually separate issues, it turned out
fixing them was a bit more tricky than I expected (fixing one was
unmasking the other one etc).
Thanks for the report!
Jan
>From 3c0bf7519cad735160d9d222d6f86f84987b38b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?=
Date: Wed, 17 Aug 2011 16:07:54 +0200
Subject: [PATCH 2/2] Guard against return type changing in PL/Python
functions.
Functions cache their I/O routines and in case their return type is
composite, a change of the underlying type can cause the cache to
become invalid. PL/Python was already checking for composite type
changes for input arguments, now the check is extended to cover the
return type as well.
Per bug report from Jean-Baptiste Quenot.
---
src/pl/plpython/expected/plpython_record.out | 21 ++
src/pl/plpython/plpython.c | 93 ++---
src/pl/plpython/sql/plpython_record.sql | 15
3 files changed, 103 insertions(+), 26 deletions(-)
diff --git a/src/pl/plpython/expected/plpython_record.out b/src/pl/plpython/expected/plpython_record.out
index 7c60089..0bcc46c 100644
--- a/src/pl/plpython/expected/plpython_record.out
+++ b/src/pl/plpython/expected/plpython_record.out
@@ -308,6 +308,27 @@ SELECT * FROM test_inout_params('test_in');
test_in_inout
(1 row)
+-- try changing the return types and call functions again
+ALTER TABLE table_record DROP COLUMN first;
+ALTER TABLE table_record DROP COLUMN second;
+ALTER TABLE table_record ADD COLUMN first text;
+ALTER TABLE table_record ADD COLUMN second int4;
+SELECT * FROM test_table_record_as('obj', 'one', 1, false);
+ first | second
+---+
+ one | 1
+(1 row)
+
+ALTER TYPE type_record DROP ATTRIBUTE first;
+ALTER TYPE type_record DROP ATTRIBUTE second;
+ALTER TYPE type_record ADD ATTRIBUTE first text;
+ALTER TYPE type_record ADD ATTRIBUTE second int4;
+SELECT * FROM test_type_record_as('obj', 'one', 1, false);
+ first | second
+---+
+ one | 1
+(1 row)
+
-- errors cases
CREATE FUNCTION test_type_record_error1() RETURNS type_record AS $$
return { 'first': 'first' }
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 90d3c47..a254ffa 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -1489,6 +1489,42 @@ PLy_function_delete_args(PLyProcedure *proc)
PyDict_DelItemString(proc->globals, proc->argnames[i]);
}
+static bool
+PLy_procedure_argument_valid(PLyTypeInfo *arg)
+{
+ Oid relid;
+ HeapTuple relTup;
+ bool valid;
+
+ /* Only check input arguments that are composite */
+ if (arg->is_rowtype != 1) {
+ return true;
+ }
+
+ /* An uninitialised typ_relid means that we got called on an output
+ * argument of a function returning a unnamed record type */
+ if (!OidIsValid(arg->typ_relid)) {
+ return true;
+ }
+
+ Assert(TransactionIdIsValid(arg->typrel_xmin));
+ Assert(ItemPointerIsValid(&arg->typrel_tid));
+
+ /* Get the pg_class tuple for the argument type */
+ relid = arg->typ_relid;
+ relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+ if (!HeapTupleIsValid(relTup))
+ elog(ERROR, "cache lookup failed for relation %u", relid);
+
+ /* If it has changed, the function is not valid */
+ valid = (arg->typrel_xmin == HeapTupleHeaderGetXmin(relTup->t_data) &&
+ ItemPointerEquals(&arg->typrel_tid, &relTup->t_self));
+
+ ReleaseSysCache(relTup);
+
+ return valid;
+}
+
/*
* Decide whether a cached PLyProcedure struct is still valid
*/
@@ -1509,33 +1545,16 @@ PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
/* If there are composite input arguments, they might have changed */
for (i = 0; i < proc->nargs; i++)
{
- Oid relid;
- HeapTuple relTup;
-
/* Short-circuit on first changed argument */
if (!valid)
break;
- /* Only check input arguments that are composite */
- if (proc->args[i].is_rowtype != 1)
- continue;
-
- Assert(OidIsValid(proc->args[i].typ_relid));
- Assert(TransactionIdIsValid(proc->args[i].typrel_xmin));
- Assert(ItemPointerIsValid(&proc->args[i].typrel_tid));
-
- /* Get the pg_class tuple for the argument type */
- relid = proc->args[i].typ_relid;
- relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
- if (!HeapTupleIsValid(relTup))
- elog(ERROR, "cache lookup failed for relation %u", relid);
-
- /* If it has changed, the function is not valid */
- if (!(proc->args[i].typrel_xmin == HeapTupleHeaderGetXmin(relTup->t_data) &&
- ItemPointerEquals(&proc->args[i].typrel_tid, &relTup->t_self)))
- valid = false;
+ valid = PLy_procedure_argument_valid(&proc->args[i]);
+ }
- ReleaseSysCache(relTup);
+ /* if the output argument is composite, it might have changed */
+ if (valid
Re: [HACKERS] plpython crash
On 16/08/11 19:12, Jan Urbański wrote: > On 16/08/11 19:07, Jean-Baptiste Quenot wrote: >> Dear Jan, >> >> Sorry I typed the wrong git commands. With latest plpython from >> branch master I got the same gdb backtrace as reported before. I >> managed to wrap up a testcase that fails 100% of times on my setup: >> https://gist.github.com/1149512 >> >> Hope it crashes on your side too :-) > > Awesome, it segfaults for me with HEAD ;) > > Now it's just a simple matter of programming... I'll take a look at it > this evening. Found it, we're invalidating the compiled functions cache when input composite arguments change, but not when output composite arguments change and the function gets called with pointers to invalid I/O routines. I'll have a patch ready soon. Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash
On 16/08/11 19:07, Jean-Baptiste Quenot wrote: > Dear Jan, > > Sorry I typed the wrong git commands. With latest plpython from > branch master I got the same gdb backtrace as reported before. I > managed to wrap up a testcase that fails 100% of times on my setup: > https://gist.github.com/1149512 > > Hope it crashes on your side too :-) Awesome, it segfaults for me with HEAD ;) Now it's just a simple matter of programming... I'll take a look at it this evening. Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash
Dear Jan, Sorry I typed the wrong git commands. With latest plpython from branch master I got the same gdb backtrace as reported before. I managed to wrap up a testcase that fails 100% of times on my setup: https://gist.github.com/1149512 Hope it crashes on your side too :-) This is the result on PG 9.0.4: https://gist.github.com/1149543 This is the result on PG 9.0.4 with plpython.c backported from HEAD: https://gist.github.com/1149558 Cheers, -- Jean-Baptiste Quenot -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash
On 16/08/11 17:06, Jan Urbański wrote: > On 16/08/11 16:52, Jean-Baptiste Quenot wrote: >> After backporting plpython.c from HEAD, this is the error message I get: >> >> ERROR: key "pg.dropped.6" not found in mapping >> HINT: To return null in a column, add the value None to the mapping >> with the key named after the column. >> CONTEXT: while creating return value >> PL/Python function "myfunc" >> >> What does it mean? I did a couple of simple tests and can't see how can the code not skip dropped columns. It seems like you're missing this commit: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=41282111e6cc73aca4b63dffe950ba7a63e4bd8a Could you try running this query? (assuming your function is called 'myfync') select proname, relname, attname, attisdropped from pg_proc join pg_class on (prorettype = reltype) join pg_attribute on (attrelid = pg_class.oid) where proname = 'myfunc'; Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash
On 16/08/11 16:52, Jean-Baptiste Quenot wrote: > After backporting plpython.c from HEAD, this is the error message I get: > > ERROR: key "pg.dropped.6" not found in mapping > HINT: To return null in a column, add the value None to the mapping > with the key named after the column. > CONTEXT: while creating return value > PL/Python function "myfunc" > > What does it mean? Ah, interesting, I think that this means that you are returning a table type and that table has a dropped column. The code should skip over dropped columns, but apparently it does not and tries to find a value for that column in the mapping you are returning. I'll try to reproduce it here. Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash
After backporting plpython.c from HEAD, this is the error message I get: ERROR: key "pg.dropped.6" not found in mapping HINT: To return null in a column, add the value None to the mapping with the key named after the column. CONTEXT: while creating return value PL/Python function "myfunc" What does it mean? -- Jean-Baptiste Quenot -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash
On 12/08/11 13:55, Jean-Baptiste Quenot wrote: > Here is the same with -O0: > > https://gist.github.com/1140005 > > sys.version reports this: > > INFO: 2.6.6 (r266:84292, Sep 15 2010, 16:41:53) > [GCC 4.4.5] I'm still at a loss. Did you reproduce it with git HEAD? I see that the query being execute is "select * from myfunc()"; would it be possible to share the code of myfunc? Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash
Here is the same with -O0: https://gist.github.com/1140005 sys.version reports this: INFO: 2.6.6 (r266:84292, Sep 15 2010, 16:41:53) [GCC 4.4.5] -- Jean-Baptiste Quenot -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython crash
On 11/08/11 18:01, Jean-Baptiste Quenot wrote: > Hi there, > > plpython crashes on me on various 64-bit Ubuntu hosts, see the gdb > backtrace at: https://gist.github.com/1140005 > > Do you believe there was recent bugfixes regarding PLyMapping_ToTuple() ? > > This is PG 9.0.4 with HEAD of plpython taken in march 2011 and backported. > > Please tell me if you need more information. Hi, there were no changes to that area of plpython after March 2011. Could you try to see if the error also appears if you run your app with current PostgreSQL HEAD (both the server and plpython)? Which Python version is that? You can get that info by running: do $$ import sys; plpy.info(sys.version) $$ language plpythonu; Could you try to extract a self-contained example of how to reproduce it? If the bug only appears under your application's specific workload, perhaps you could try running it with Postgres compiled with -O0, because compiling with -O2 causes the gdb backtrace to be missing optimised out values and inlined functions? Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] plpython crash
Hi there, plpython crashes on me on various 64-bit Ubuntu hosts, see the gdb backtrace at: https://gist.github.com/1140005 Do you believe there was recent bugfixes regarding PLyMapping_ToTuple() ? This is PG 9.0.4 with HEAD of plpython taken in march 2011 and backported. Please tell me if you need more information. -- Jean-Baptiste Quenot -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython thinks it's hooked into "make distprep", but not so much
I wrote: > I noticed while wrapping 9.1beta2 that plpython tries to build a file > spiexceptions.h to be included in the tarballs, but no such file is > actually appearing therein. The reason is that src/pl/Makefile doesn't > recurse into the plpython subdirectory unless with_python is set. > Which it isn't, because the tarball build script doesn't configure > --with-python (or with any other options for that matter). If it did, > it would fail, since there's no python installation on > developer.postgresql.org. > I left this go for the moment, since the only implication is that you > have to have perl available to build plpython from the tarball. But we > oughta fix it before final. Any opinions on what's the least ugly way > to get control to recurse to src/pl/plpython for "make distprep" > regardless of with_python? > (More generally, I would like to see cleanup targets such as make > distclean and make maintainer-clean recurse to all subdirectories > regardless of configure options, but that might be too much to ask for > right now.) Attached is a proposed patch to fix this. I found that the general case was not too hard to solve, so this patch causes the make system to recurse everywhere for distprep, clean, distclean, maintainer-clean cases. Anybody have a cleaner way to do it, or want to bikeshed on the macro names? regards, tom lane diff --git a/contrib/Makefile b/contrib/Makefile index 6967767..0c238aa 100644 *** a/contrib/Makefile --- b/contrib/Makefile *** SUBDIRS = \ *** 52,69 --- 52,77 ifeq ($(with_openssl),yes) SUBDIRS += sslinfo + else + ALWAYS_SUBDIRS += sslinfo endif ifeq ($(with_ossp_uuid),yes) SUBDIRS += uuid-ossp + else + ALWAYS_SUBDIRS += uuid-ossp endif ifeq ($(with_libxml),yes) SUBDIRS += xml2 + else + ALWAYS_SUBDIRS += xml2 endif ifeq ($(with_selinux),yes) SUBDIRS += sepgsql + else + ALWAYS_SUBDIRS += sepgsql endif # Missing: *** endif *** 71,73 --- 79,82 $(recurse) + $(recurse_always) diff --git a/src/Makefile.global.in b/src/Makefile.global.in index ba08223..3bf658d 100644 *** a/src/Makefile.global.in --- b/src/Makefile.global.in *** *** 19,24 --- 19,26 # Meta configuration standard_targets = all install installdirs uninstall distprep clean distclean maintainer-clean coverage check installcheck maintainer-check + # these targets should recurse even into subdirectories not being built: + standard_always_targets = distprep clean distclean maintainer-clean .PHONY: $(standard_targets) install-strip html man installcheck-parallel *** endef *** 603,608 --- 605,620 # $3: target to run in subdir (defaults to current element of $1) recurse = $(foreach target,$(if $1,$1,$(standard_targets)),$(foreach subdir,$(if $2,$2,$(SUBDIRS)),$(eval $(call _create_recursive_target,$(target),$(subdir),$(if $3,$3,$(target)) + # If a makefile's list of SUBDIRS varies depending on configuration, then + # any subdirectories excluded from SUBDIRS should instead be added to + # ALWAYS_SUBDIRS, and then it must call recurse_always as well as recurse. + # This ensures that distprep, distclean, etc will apply to all subdirectories. + # In the normal case all arguments will be defaulted. + # $1: targets to make recursive (defaults to standard_always_targets) + # $2: list of subdirs (defaults to ALWAYS_SUBDIRS variable) + # $3: target to run in subdir (defaults to current element of $1) + recurse_always = $(foreach target,$(if $1,$1,$(standard_always_targets)),$(foreach subdir,$(if $2,$2,$(ALWAYS_SUBDIRS)),$(eval $(call _create_recursive_target,$(target),$(subdir),$(if $3,$3,$(target)) + ## # diff --git a/src/bin/Makefile b/src/bin/Makefile index 3809412..c333a48 100644 *** a/src/bin/Makefile --- b/src/bin/Makefile *** include $(top_builddir)/src/Makefile.glo *** 15,22 SUBDIRS = initdb pg_ctl pg_dump \ psql scripts pg_config pg_controldata pg_resetxlog pg_basebackup ifeq ($(PORTNAME), win32) ! SUBDIRS+=pgevent endif $(recurse) --- 15,26 SUBDIRS = initdb pg_ctl pg_dump \ psql scripts pg_config pg_controldata pg_resetxlog pg_basebackup + ifeq ($(PORTNAME), win32) ! SUBDIRS += pgevent ! else ! ALWAYS_SUBDIRS += pgevent endif $(recurse) + $(recurse_always) diff --git a/src/pl/Makefile b/src/pl/Makefile index ef630fe..c4a0d1c 100644 *** a/src/pl/Makefile --- b/src/pl/Makefile *** SUBDIRS = plpgsql *** 16,29 --- 16,36 ifeq ($(with_perl), yes) SUBDIRS += plperl + else + ALWAYS_SUBDIRS += plperl endif ifeq ($(with_python), yes) SUBDIRS += plpython + else + ALWAYS_SUBDIRS += plpython endif ifeq ($(with_tcl), yes) SUBDIRS += tcl + else + ALWAYS_SUBDIRS += tcl endif $(recurse) + $(recurse_always) diff --git a/src/pl/plpython/Makefile b/src/
[HACKERS] plpython thinks it's hooked into "make distprep", but not so much
I noticed while wrapping 9.1beta2 that plpython tries to build a file spiexceptions.h to be included in the tarballs, but no such file is actually appearing therein. The reason is that src/pl/Makefile doesn't recurse into the plpython subdirectory unless with_python is set. Which it isn't, because the tarball build script doesn't configure --with-python (or with any other options for that matter). If it did, it would fail, since there's no python installation on developer.postgresql.org. I left this go for the moment, since the only implication is that you have to have perl available to build plpython from the tarball. But we oughta fix it before final. Any opinions on what's the least ugly way to get control to recurse to src/pl/plpython for "make distprep" regardless of with_python? (More generally, I would like to see cleanup targets such as make distclean and make maintainer-clean recurse to all subdirectories regardless of configure options, but that might be too much to ask for right now.) 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
Re: [HACKERS] plpython versus gcc 4.5.x
On 22/01/11 19:38, Tom Lane wrote: > Buildfarm members bobcat (Fedora 14 x86) and frogmouth (mingw) have > been showing an assertion failure in the plpython regression tests > since this patch went in: > http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=740e54c > Given these facts, and the fact that the above-mentioned patch has no > purpose other than code beautification, I recommend reverting the patch. Yep, that (sadly) seems to be the best way to proceed. I'll merge master into the rest of my branches and mop up the resulting conflicts (which shouldn't be that bad). Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython versus gcc 4.5.x
On 01/22/2011 01:38 PM, Tom Lane wrote: Buildfarm members bobcat (Fedora 14 x86) and frogmouth (mingw) have been showing an assertion failure in the plpython regression tests since this patch went in: http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=740e54c I have been able to reproduce the problem locally on Fedora 14 x86_64. The common thread appears to be use of gcc 4.5 --- Andrew says that frogmouth is using 4.5.0, and current F-14 has gcc 4.5.1. I have found that manually re-inlining PLy_procedure_input_conversion is sufficient to make the problem go away, as is reducing the optimization level below -O2. Based on these observations, I believe it's not our bug but an optimization bug in gcc, and have accordingly filed a gcc bug: https://bugzilla.redhat.com/show_bug.cgi?id=671899 However, it's hard to guess how long it might take for the gcc boys to provide a fix, and in any case I fear it will be a long time before the fix propagates to places like mingw. Given these facts, and the fact that the above-mentioned patch has no purpose other than code beautification, I recommend reverting the patch. We could just revert the de-inlining of PLy_procedure_input_conversion, but there doesn't seem to be a lot of point in having one separate and the other not. Since there's only one call site for each function, I don't think it's worth making herculean efforts to do anything else. so +1 cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] plpython versus gcc 4.5.x
Buildfarm members bobcat (Fedora 14 x86) and frogmouth (mingw) have been showing an assertion failure in the plpython regression tests since this patch went in: http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=740e54c I have been able to reproduce the problem locally on Fedora 14 x86_64. The common thread appears to be use of gcc 4.5 --- Andrew says that frogmouth is using 4.5.0, and current F-14 has gcc 4.5.1. I have found that manually re-inlining PLy_procedure_input_conversion is sufficient to make the problem go away, as is reducing the optimization level below -O2. Based on these observations, I believe it's not our bug but an optimization bug in gcc, and have accordingly filed a gcc bug: https://bugzilla.redhat.com/show_bug.cgi?id=671899 However, it's hard to guess how long it might take for the gcc boys to provide a fix, and in any case I fear it will be a long time before the fix propagates to places like mingw. Given these facts, and the fact that the above-mentioned patch has no purpose other than code beautification, I recommend reverting the patch. We could just revert the de-inlining of PLy_procedure_input_conversion, but there doesn't seem to be a lot of point in having one separate and the other not. 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
Re: [HACKERS] PlPython bug in 9.0/8.4.4
Teodor Sigaev writes: > The way to reproduce: Well, of course plpython's error handling is fundamentally brain dead. The immediate problem here seems to be that the PLy_error_in_progress struct is stored in a memory context that's been cleared by the time that control gets to the place in PLy_procedure_call() where we try to re-throw it. It might be that we need to copy the ErrorData into a less short-lived context than the one that's current when PLy_spi_prepare is called. Or maybe PLy_function_handler just needs to think a bit harder about where it has to check PLy_error_in_progress and/or where it's safe to shut down the SPI call. Or all of the above. Really I'd like to see that whole mess thrown out and rewritten. pltcl and plperl got rid of static error state years ago, because of essentially this same type of risk. > 8.4.4 haves essentially the same bug. I'm sure it goes all the way back. 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
[HACKERS] PlPython bug in 9.0/8.4.4
The way to reproduce:
CREATE OR REPLACE FUNCTION foobar(a integer[])
RETURNS SETOF int8 AS
$$
def getplan(name, query, args):
if SD.has_key(name):
plpy.warning("Using cached plan %s" % name)
return SD[name]
plpy.warning("Prepare plan %s" % name);
plan = plpy.prepare(query, args)
plpy.warning("Plan %s prepared" % name);
SD[name] = plan
return plan
for s in a:
userPlan = getplan('fooplan', 'select 1', []);
rrr = plpy.execute(plan, []) # variable plan isn't defined!
ret = s
yield ret
$$
LANGUAGE 'plpythonu';
On 9.0beta2 (instead of correct message ": global
name 'plan' is not defined"):
postgres=# select foobar('{1,2,3}');
WARNING: Prepare plan fooplan
CONTEXT: PL/Python function "foobar"
WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function "foobar"
ERROR: error fetching next item from iterator
CONTEXT: PL/Python function "foobar"
postgres=# select foobar('{1,2,3}');
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!?>
Log:
TRAP: FailedAssertion("!(edata->elevel == 20)", File: "elog.c", Line: 1280)
8.4.4 haves essentially the same bug.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython sets
Ühel kenal päeval, E, 2006-07-17 kell 23:22, kirjutas Matteo Bertini: > Tino Wildenhain ha scritto: > > http://python.projects.postgresql.org/ > > This works very well for me - although it needs some more > > finish (docs and so on) maybe if more people using it > > it can get better. > > > > SRF - even lazy ones (e.g. generators) work nicely there. > > > > > > Regards > > Tino Wildenhain > > > > > eheh... I wasn't able to build it... > > In the meantime some guy in the list provided me a "setof" enabled > plpython.c. Now built and working fine! > > Do someone know why the back-end effort is duplicated? http://python.projects.postgresql.org/ seems to be aiming at a much larger python/postgres integration scheme than src/pl/python. and it has taken a different and more pythonic approach at many places which is nice but takes a lot longer to mature. -- Hannu Krosing Database Architect Skype Technologies OÜ Akadeemia tee 21 F, Tallinn, 12618, Estonia Skype me: callto:hkrosing Get Skype for free: http://www.skype.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] plpython sets
Tino Wildenhain ha scritto: > http://python.projects.postgresql.org/ > This works very well for me - although it needs some more > finish (docs and so on) maybe if more people using it > it can get better. > > SRF - even lazy ones (e.g. generators) work nicely there. > > > Regards > Tino Wildenhain > > eheh... I wasn't able to build it... In the meantime some guy in the list provided me a "setof" enabled plpython.c. Now built and working fine! Do someone know why the back-end effort is duplicated? Thanks, Matteo Bertini begin:vcard fn:Matteo Bertini n:Bertini;Matteo email;internet:[EMAIL PROTECTED] tel;cell:+39(0)3284729474 note;quoted-printable:Ci sono 10 tipi di persone, quelle che capiscono il Binario e quelle chen= on lo capiscono.=0D=0A= OpenPGP: http://blog.naufraghi.net/openpgp=0D=0A= ICQ: 33956256 url:http://www.slug.it/naufraghi/ version:2.1 end:vcard ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] plpython sets
Ühel kenal päeval, E, 2006-07-17 kell 22:54, kirjutas Tino Wildenhain: > Matteo Bertini wrote: > > Hello all, > > I'm working with pl/python and I'd like to use the set returning > > function feature. > > > > I'm not working in a debug python, so the iterator bug is not a problem me. > > > > Can someone point me to some plpython.c "setof" enabled sources? > > > > Hint to build them in an ubuntu dapper environment are welcome too :-P ! > > > > Thanks a lot every developer involved in postgres! > > PL/setyourlanguagehere is fantastic! > > > http://python.projects.postgresql.org/ > > This works very well for me - although it needs some more > finish (docs and so on) maybe if more people using it > it can get better. > > SRF - even lazy ones (e.g. generators) work nicely there. We at Skype or more precisely Sven Suursoho :) , has added these to the version of plpython in the core and they will be available in 8.2. Code for 8.0 and 8.1 will be available on request, and soon also from https://developer.skype.com/ Enchancements added are: * named parameters (args[] still valid) * returning composite types (dict, tuple, list, class) * returning SETOF as any iterable object (list, tuple, iterator, generator) -- Hannu Krosing Database Architect Skype Technologies OÜ Akadeemia tee 21 F, Tallinn, 12618, Estonia Skype me: callto:hkrosing Get Skype for free: http://www.skype.com ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] plpython sets
Matteo Bertini wrote: > Hello all, > I'm working with pl/python and I'd like to use the set returning > function feature. > > I'm not working in a debug python, so the iterator bug is not a problem me. > > Can someone point me to some plpython.c "setof" enabled sources? > > Hint to build them in an ubuntu dapper environment are welcome too :-P ! > > Thanks a lot every developer involved in postgres! > PL/setyourlanguagehere is fantastic! http://python.projects.postgresql.org/ This works very well for me - although it needs some more finish (docs and so on) maybe if more people using it it can get better. SRF - even lazy ones (e.g. generators) work nicely there. Regards Tino Wildenhain ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[HACKERS] plpython sets
Hello all, I'm working with pl/python and I'd like to use the set returning function feature. I'm not working in a debug python, so the iterator bug is not a problem me. Can someone point me to some plpython.c "setof" enabled sources? Hint to build them in an ubuntu dapper environment are welcome too :-P ! Thanks a lot every developer involved in postgres! PL/setyourlanguagehere is fantastic! Matteo Bertini begin:vcard fn:Matteo Bertini n:Bertini;Matteo email;internet:[EMAIL PROTECTED] tel;cell:+39(0)3284729474 note;quoted-printable:Ci sono 10 tipi di persone, quelle che capiscono il Binario e quelle chen= on lo capiscono.=0D=0A= OpenPGP: http://blog.naufraghi.net/openpgp=0D=0A= ICQ: 33956256 url:http://www.slug.it/naufraghi/ version:2.1 end:vcard ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[HACKERS] plpython improvements on patches
Hi Anyone interested in plpython, please check out/review http://archives.postgresql.org/pgsql-patches/2006-04/msg00087.php It fixes the most obvious shortcomings of plpython by making it possible to return both sets and records (and setof records) and to use named input parameters in addition to args[]. The patch is for pg 8.0.7, but batches for 8.1 and HEAD will also be made available soon. - Hannu ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] plpython and bytea
On Mon, 2005-11-21 at 02:11 +0200, Hannu Krosing wrote:
> Hi
>
> It seems that plpython is unable to return bytea string when it contains
> NUL bytes:
>
> hannu=# CREATE OR REPLACE FUNCTION get_bytea_with_nul() RETURNS bytea AS
> '
> return ''aa\\0bb''
> ' LANGUAGE plpythonu SECURITY DEFINER;
>
> hannu=# select get_bytea_with_nul();
> get_bytea_with_nul
>
> aa
> (1 row)
>
>
> probably related to plpythons way of generating return value via
> converting python objcet to its string representation and then letting
> postgres's input func to convert it back.
Ok, I was able to successfuly return all bytea values from plpython by
creating a bytea class that oveloads strings __str__ method to generate
something that postgresql's bytea type input method understands:
create or replace function get_bytea256() returns bytea as $$
class bytea(str):
def __str__(self):
res = []
for c in self:
if (c in ("\000","'","\\")):
res.append(r"\%03o" % ord(c))
else:
res.append(c)
return ''.join(res)
return bytea("".join([chr(i) for i in range(256)]))
$$ language plpythonu;
please note that this is a quick proof-of-concept implementation which
contains several gross inefficiencies :p
---
Hannu Krosing
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [HACKERS] plpython and bytea
Am Montag, den 21.11.2005, 09:08 -0700 schrieb James William Pye:
> On Mon, 2005-11-21 at 15:18 +0200, Hannu Krosing wrote:
> > The project seems quite interesting, will surely take a deeper look
> >
...
>
> The 'layout' package needs to be installed first.
>
> See this quick start section:
> http://python.projects.postgresql.org/quick.html#Fetch+and+Install+the
> +Backend
> ('be' depends on 'lo' and 'ex')
There is:
cvs -d :pserver:anonymous:@cvs.pgfoundry.org co lo ex be
which should be:
cvs -d :pserver:anonymous:@cvs.pgfoundry.org:/cvsroot/python co lo ex be
to work.
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [HACKERS] plpython and bytea
On Mon, 2005-11-21 at 15:18 +0200, Hannu Krosing wrote:
> The project seems quite interesting, will surely take a deeper look
>
> It looks quite alpha, seems to mess to deeply with backend, and the cvs
> checkout of module be does not build, so I will probably not be able to
> use it in production for quite a while :(
>
> [EMAIL PROTECTED] be]$ python setup.py build
> running build
> running config
> Traceback (most recent call last):
> File "setup.py", line 256, in ?
> setup(defaults)
> File "setup.py", line 249, in setup
> d = distutils.core.setup(**kw)
> File "/usr/lib/python2.4/distutils/core.py", line 149, in setup
> dist.run_commands()
> File "/usr/lib/python2.4/distutils/dist.py", line 946, in run_commands
> self.run_command(cmd)
> File "/usr/lib/python2.4/distutils/dist.py", line 966, in run_command
> cmd_obj.run()
> File "/home/hannu/work/postgresPy/be/lib/ldistutils.py", line 193, in
> run
> self.run_command('config')
> File "/usr/lib/python2.4/distutils/cmd.py", line 333, in run_command
> self.distribution.run_command(command)
> File "/usr/lib/python2.4/distutils/dist.py", line 966, in run_command
> cmd_obj.run()
> File "/home/hannu/work/postgresPy/be/lib/ldistutils.py", line 166, in
> run
> self.distribution.config = cfg = self.load()
> File "/home/hannu/work/postgresPy/be/lib/ldistutils.py", line 129, in
> load
> cfgsrc(self)
> File "setup.py", line 19, in configure
> import postgresql.utility.config
> ImportError: No module named postgresql.utility.config
The 'layout' package needs to be installed first.
See this quick start section:
http://python.projects.postgresql.org/quick.html#Fetch+and+Install+the
+Backend
('be' depends on 'lo' and 'ex')
--
Regards, James William Pye
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] plpython and bytea
Hannu Krosing schrieb: On Mon, 2005-11-21 at 08:37 +0100, Tino Wildenhain wrote: Am Montag, den 21.11.2005, 02:11 +0200 schrieb Hannu Krosing: Hi It seems that plpython is unable to return bytea string when it contains NUL bytes: ... Did you also try: http://python.projects.postgresql.org/project/be.html ? Afaic it works a little different. The project seems quite interesting, will surely take a deeper look It looks quite alpha, seems to mess to deeply with backend, and the cvs checkout of module be does not build, so I will probably not be able to use it in production for quite a while :( Well I had it running and it was very impressive. However it seems no easy install yet - tried a cvs head with similar problems. I hope the author reads this here. At least I met him on freenode #postgresql ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] plpython and bytea
On Mon, 2005-11-21 at 08:37 +0100, Tino Wildenhain wrote:
> Am Montag, den 21.11.2005, 02:11 +0200 schrieb Hannu Krosing:
> > Hi
> >
> > It seems that plpython is unable to return bytea string when it contains
> > NUL bytes:
> >
...
> Did you also try:
>
> http://python.projects.postgresql.org/project/be.html
>
> ? Afaic it works a little different.
The project seems quite interesting, will surely take a deeper look
It looks quite alpha, seems to mess to deeply with backend, and the cvs
checkout of module be does not build, so I will probably not be able to
use it in production for quite a while :(
[EMAIL PROTECTED] be]$ python setup.py build
running build
running config
Traceback (most recent call last):
File "setup.py", line 256, in ?
setup(defaults)
File "setup.py", line 249, in setup
d = distutils.core.setup(**kw)
File "/usr/lib/python2.4/distutils/core.py", line 149, in setup
dist.run_commands()
File "/usr/lib/python2.4/distutils/dist.py", line 946, in run_commands
self.run_command(cmd)
File "/usr/lib/python2.4/distutils/dist.py", line 966, in run_command
cmd_obj.run()
File "/home/hannu/work/postgresPy/be/lib/ldistutils.py", line 193, in
run
self.run_command('config')
File "/usr/lib/python2.4/distutils/cmd.py", line 333, in run_command
self.distribution.run_command(command)
File "/usr/lib/python2.4/distutils/dist.py", line 966, in run_command
cmd_obj.run()
File "/home/hannu/work/postgresPy/be/lib/ldistutils.py", line 166, in
run
self.distribution.config = cfg = self.load()
File "/home/hannu/work/postgresPy/be/lib/ldistutils.py", line 129, in
load
cfgsrc(self)
File "setup.py", line 19, in configure
import postgresql.utility.config
ImportError: No module named postgresql.utility.config
-
Hannu Krosing
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] plpython and bytea
Am Montag, den 21.11.2005, 02:11 +0200 schrieb Hannu Krosing: > Hi > > It seems that plpython is unable to return bytea string when it contains > NUL bytes: > > hannu=# CREATE OR REPLACE FUNCTION get_bytea_with_nul() RETURNS bytea AS > ' > return ''aa\\0bb'' > ' LANGUAGE plpythonu SECURITY DEFINER; > > hannu=# select get_bytea_with_nul(); > get_bytea_with_nul > > aa > (1 row) > > > probably related to plpythons way of generating return value via > converting python objcet to its string representation and then letting > postgres's input func to convert it back. Did you also try: http://python.projects.postgresql.org/project/be.html ? Afaic it works a little different. > > Btw, does anyone know where Andrew Bosma (the original author of > plpython) is ? > > I would probably have a paid job improving an opensource project for > him :) > > ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[HACKERS] plpython and bytea
Hi It seems that plpython is unable to return bytea string when it contains NUL bytes: hannu=# CREATE OR REPLACE FUNCTION get_bytea_with_nul() RETURNS bytea AS ' return ''aa\\0bb'' ' LANGUAGE plpythonu SECURITY DEFINER; hannu=# select get_bytea_with_nul(); get_bytea_with_nul aa (1 row) probably related to plpythons way of generating return value via converting python objcet to its string representation and then letting postgres's input func to convert it back. Btw, does anyone know where Andrew Bosma (the original author of plpython) is ? I would probably have a paid job improving an opensource project for him :) -- Hannu Krosing <[EMAIL PROTECTED]> ---(end of broadcast)--- TIP 6: explain analyze is your friend
