Andrew Dunstan <andrew@dunslane.net> 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
