Re: [HACKERS] plpython is broken for recursive use

[Andrew Dunstan]

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

[Tom Lane]

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

[Tom Lane]

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

[Tom Lane]

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

[Josh Berkus]

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

[Tom Lane]

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: [HACKERS] plpython is broken for recursive use

[Joshua D. Drake]

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

[Andrew Dunstan]

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 implementation

[Hannu Krosing]

On 07/01/2013 07:53 AM, Claudio Freire wrote:
 On Mon, Jul 1, 2013 at 2:29 AM, james ja...@mansionfamily.plus.com 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

[Andres Freund]

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

[Peter Eisentraut]

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

PL/Lua (was: [HACKERS] plpython implementation)

[Luis Carvalho]

Hi all,

Claudio Freire wrote:
 On Mon, Jul 1, 2013 at 2:29 AM, james ja...@mansionfamily.plus.com 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.

snip
 
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: PL/Lua (was: [HACKERS] plpython implementation)

[Peter Eisentraut]

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

Re: PL/Lua (was: [HACKERS] plpython implementation)

[Luis Carvalho]

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)

[Andreas Karlsson]

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

[HACKERS] plpython implementation

[Szymon Guz]

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 implementation

[Martijn van Oosterhout]

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   klep...@svana.org   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

[Andrew Dunstan]

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

[Szymon Guz]

On 30 June 2013 14:13, Andrew Dunstan and...@dunslane.net 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

[Martijn van Oosterhout]

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   klep...@svana.org   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

[Andrew Dunstan]

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

[Szymon Guz]

On 30 June 2013 14:31, Martijn van Oosterhout klep...@svana.org 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

[Andres Freund]

On 2013-06-30 14:42:24 +0200, Szymon Guz wrote:
 On 30 June 2013 14:31, Martijn van Oosterhout klep...@svana.org 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

[Szymon Guz]

On 30 June 2013 14:45, Andres Freund and...@2ndquadrant.com wrote:

 On 2013-06-30 14:42:24 +0200, Szymon Guz wrote:
  On 30 June 2013 14:31, Martijn van Oosterhout klep...@svana.org 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

[Claudio Freire]

On Sun, Jun 30, 2013 at 9:45 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-06-30 14:42:24 +0200, Szymon Guz wrote:
 On 30 June 2013 14:31, Martijn van Oosterhout klep...@svana.org 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

[james]

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

[Claudio Freire]

On Mon, Jul 1, 2013 at 2:29 AM, james ja...@mansionfamily.plus.com 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 issue with Win64 (PG 9.2)

[Heikki Linnakangas]

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)

[Jan Urbański]

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)

[Asif Naeem]

 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)

[Jan Urbański]

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)

[Jan Urbański]

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)

[Jan Urbański]

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)

[Asif Naeem]

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 wulc...@wulczer.org 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 foo 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)

[Jan Urbański]

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 foo 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

[HACKERS] plpython issue with Win64 (PG 9.2)

[Asif Naeem]

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 issue with Win64 (PG 9.2)

[Jan Urbański]

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

Re: [HACKERS] plpython crash (PG 92)

[Peter Eisentraut]

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]

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 crash (PG 92)

[Tom Lane]

Peter Eisentraut pete...@gmx.net 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)

[Tom Lane]

Peter Eisentraut pete...@gmx.net 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

[HACKERS] plpython crash (PG 92)

[Asif Naeem]

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 crash (PG 92)

[Asif Naeem]

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 asif.na...@enterprisedb.comwrote:

 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

Re: [HACKERS] plpython triggers are broken for composite-type columns

[Peter Eisentraut]

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 triggers are broken for composite-type columns

[Jan Urbański]

On 10/04/12 21:47, Jan Urbański wrote:

On 10/04/12 21:27, Tom Lane wrote:

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?=wulc...@wulczer.org 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?= wulc...@wulczer.org
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) 

Re: [HACKERS] plpython triggers are broken for composite-type columns

[Jan Urbański]

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

[Tom Lane]

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org 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

[Tom Lane]

I wrote:
 =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org 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

[Jan Urbański]

On 10/04/12 20:47, Tom Lane wrote:

I wrote:

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?=wulc...@wulczer.org  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

[Tom Lane]

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org 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

[Jan Urbański]

On 10/04/12 21:27, Tom Lane wrote:

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?=wulc...@wulczer.org  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

[HACKERS] plpython triggers are broken for composite-type columns

[Tom Lane]

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 triggers are broken for composite-type columns

[Jan Urbański]

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

Re: [HACKERS] plpython SPI cursors

[Peter Eisentraut]

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

[Peter Eisentraut]

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

[Jan Urbański]

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

[Bruce Momjian]

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  br...@momjian.ushttp://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

[Jan Urbański]

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

[Peter Eisentraut]

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

[Steve Singer]

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

[Jan Urbański]

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;
   /para
 
   para
+Note that calling literalplpy.execute/literal 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 literalplpy.cursor/literal rather
+than literalplpy.execute/literal.
+  /para
+
+  para
For example:
 programlisting
 rv = plpy.execute(SELECT * FROM my_table, 5)
@@ -958,6 +967,77 @@ $$ LANGUAGE plpythonu;
 
   /sect2
 
+  sect2
+titleAccessing data with cursors/title
+
+  para
+The literalplpy.cursor/literal function accepts the same arguments
+as literalplpy.execute/literal (except for literallimit/literal)
+and returns a cursor object, which allows you to process large result sets
+in smaller chunks.  As with literalplpy.execute/literal, either a query
+string or a plan object along with a list of arguments can be used.  The
+cursor object provides a literalfetch/literal method that accepts an
+integer paramter and returns a result object.  Each time you
+call literalfetch/literal, the returned object will contain the next
+

Re: [HACKERS] plpython SPI cursors

[Steve Singer]

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: NULL
CONTEXT:  Traceback (most recent call last):
  PL/Python function x1, line 9, in module
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 SPI cursors

[Jan Urbański]

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 extension control files installation

[Robert Haas]

On Sat, Nov 5, 2011 at 10:42 AM, Peter Eisentraut pete...@gmx.net 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

[Peter Eisentraut]

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

[Jan Urbański]

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?= wulc...@wulczer.org
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 
/para
  
para
+ Note that calling literalplpy.execute/literal 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 literalplpy.cursor/literal rather
+ than literalplpy.execute/literal.
+   /para
+ 
+   para
 For example:
  programlisting
  rv = plpy.execute(SELECT * FROM my_table, 5)
*** $$ LANGUAGE plpythonu;
*** 958,963 
--- 967,1043 
  
/sect2
  
+   sect2
+ titleAccessing data with cursors/title
+ 
+   para
+ The literalplpy.cursor/literal function accepts the same arguments
+ as literalplpy.execute/literal (except for literallimit/literal)
+ and returns a cursor object, which allows you to process large result sets
+ in smaller chunks.  As with literalplpy.execute/literal, either a query
+ string or a plan object along with a list of arguments can be used.  The
+ cursor object provides a literalfetch/literal method that accepts an
+ integer paramter and returns a result object.  Each time you
+ call literalfetch/literal, the returned object will contain the next
+ batch of rows, never larger than the parameter value.  Once all rows are
+ exhausted, literalfetch/literal starts returning an empty result
+ object.  Cursor objects also provide an
+ ulink url=http://docs.python.org/library/stdtypes.html#iterator-types;iterator
+ interface/ulink, 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.
+   /para
+ 
+   para
+ Cursors are automatically disposed of, but if you want to explicitly
+ release all resources held by a cursor, use the literalclose/literal
+ method.  Once closed, a cursor cannot be fetched from anymore.
+   /para
+ 
+note
+ para
+   Do not confuse objects created by literalplpy.cursor/literal with
+   DBAPI cursors as defined by
+   the ulink url=http://www.python.org/dev/peps/pep-0249/;Python Database API specification/ulink.
+   They don't have anything in common except for the name.
+ /para
+/note
+ 
+   para
+ An example of two ways of processing data from a large table would be:
+ programlisting
+ 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, 

Re: [HACKERS] plpython crash

[Jan Urbański]

On 17/08/11 23:10, Tom Lane wrote:
 =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org 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

[Jan Urbański]

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?= wulc...@wulczer.org
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) {
+		valid = 

Re: [HACKERS] plpython crash

[Tom Lane]

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org 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

[Tom Lane]

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org 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

[Jean-Baptiste Quenot]

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

[Jan Urbański]

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

[Jan Urbański]

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

[Jean-Baptiste Quenot]

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

[Jan Urbański]

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

[Jan Urbański]

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

Re: [HACKERS] plpython crash

[Jean-Baptiste Quenot]

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

[Jan Urbański]

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

[HACKERS] plpython crash

[Jean-Baptiste Quenot]

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

[Tom Lane]

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 

[HACKERS] plpython thinks it's hooked into make distprep, but not so much

[Tom Lane]

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

[HACKERS] plpython versus gcc 4.5.x

[Tom Lane]

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 versus gcc 4.5.x

[Andrew Dunstan]

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

Re: [HACKERS] plpython versus gcc 4.5.x

[Jan Urbański]

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

[HACKERS] PlPython bug in 9.0/8.4.4

[Teodor Sigaev]

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 type 'exceptions.NameError': 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 bug in 9.0/8.4.4

[Tom Lane]

Teodor Sigaev teo...@sigaev.ru 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

Re: [HACKERS] plpython sets

[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?

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

[Hannu Krosing]

Ü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

[HACKERS] plpython sets

[Matteo Bertini]

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

Re: [HACKERS] plpython sets

[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.


Regards
Tino Wildenhain

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [HACKERS] plpython sets

[Hannu Krosing]

Ü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

[HACKERS] plpython improvements on patches

[Hannu Krosing]

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

[Hannu Krosing]

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

[Hannu Krosing]

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

[Tino Wildenhain]

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

[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
 
 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

[Tino Wildenhain]

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

[HACKERS] plpython and bytea

[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.


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

Re: [HACKERS] plpython and bytea

[Tino Wildenhain]

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

Re: [HACKERS] plpython

[Tilo Schwarz]

James Pye writes:

 Type conversion

   plpython's current type conversion implementation appears to be dependent
 on strings as the common format. This is fine, but not very extensible as
 is, unless you don't mind explicitly parsing strings inside each function
 that takes an unsupported data type. I was thinking that a better solution
 would be creating a python object type inside the database. Thus allowing
 users to write casts to and from non-standard or unimplemented data types
 with little difficulty(well, maybe some :). This would allow conversion in
 an extensible way, which doesn't require modification to plpython. Storage
 could be easily achieved by pickling the object. Another thought would be
 to just pass valid PyObject pointers in and out of conversion procedures,
 effectively disallowing storage(outside the process in which the object was
 created in), unless it is possible to have a persistent storage mechanism
 that makes it possible to go through pickle?.?..(yeah, I'm new to pgsql
 dev).

As a first step I would be already be happy, if plpython would use more Python 
datatypes, for example:

- currently, a Point (or a box, polygon, etc.) is returned as a string
(0, 1)
instead of a Python tuple (or list)
(0, 1)

- the same holds for arrays: instead of getting an array as string (which I 
have to parse into a python list), I would like to get a python list in the 
first place.

Regards,

Tilo

---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send unregister YourEmailAddressHere to [EMAIL PROTECTED])