Re: plpython is broken for recursive use

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Josh Berkus <josh(at)agliodbs(dot)com>, pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: plpython is broken for recursive use
Date: 2015-10-16 21:00:40
Message-ID: 30972.1445029240@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-10-16 21:08:26 shm_mq fix for non-blocking mode
Previous Message l 2015-10-16 20:30:31 BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby