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
