Andres Freund <andres@2ndquadrant.com> writes:
> The bug got introduced in 46211da1b84bc3537e799ee1126098e71c2428e8 which
> interestingly says "Using HTABs instead of Python dictionaries makes
> error recovery easier, and allows for procedures to be cached based on
> their OIDs, not their names." - but the caching seems to always have
> used ("%u_%u", fn_oid, tgreloid) as its key which explains why the bug
> didn't exist back then.
> Anyway, as far as I can see only 9.1 upwards are affected.
Patch applied with some editorialization, mostly cosmetic but one thing
not so much: you introduced uninitialized-variable hazards into
PLy_procedure_get, since if use_cache is false then found, entry, and
proc wouldn't get set up.
Neither version of gcc I tried complained about this, and I assume yours
didn't either, which is a bit annoying/scary. I wonder whether the
"volatile" marker prevents that?
BTW, it seems to me that there is another bug in PLy_procedure_get,
which is that it's not thinking about recursive use of a procedure.
Consider
call f
... somebody updates f's pg_proc tuple
... recursively call f
At this point PLy_procedure_get will decide the cache entry is obsolete
and happily trash the procedure data that the outer call is still using.
We ran into this long ago with plpgsql and fixed it by adding
reference-counting logic. Probably the same needs to be done here.
There are probably not that many people trying to use plpython functions
in such a way (note the recursion would have to happen via a SPI query,
not directly within Python). But still...
regards, tom lane
