On 2013-01-24 11:40:33 -0500, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > On 2013-01-24 15:03:36 +0100, Sandro Santilli wrote:
> > ISTM the caching code for plpythonu trigger functions has been broken
> > for some time. The bug seem to be that PLy_procedure_get looks up the
> > function in a separate cache for trigger functions (PLy_trigger_cache)
> > but still only uses the function oid for lookup.
> > So the same function can be looked up for two tables and when the cached
> > entry originally refers to the old table that has been dropped we see
> > the above error.
>
> > The fix seems to be to make PLy_trigger_cache have a key of (reloid,
> > fn_oid) instead of just fn_oid.
>
> If there's anything in either the cache entry itself or the function's
> other persistent state that is dependent on which table it's attached
> to, then yeah, obviously. But another conceivable solution would be to
> not store any such information.
>
> In a quick look, it seems like the "PLyTypeInfo result" field might
> depend on which table the trigger is attached to, so unless we can
> get along without that, we'll need to expand the hash key.
It looks to me like getting rid of that requirement would be too much
code for something backbrancheable . Although I dislike seing code like
/* If the pg_proc tuple has changed, it's not valid */
if (!(proc->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
ItemPointerEquals(&proc->fn_tid, &procTup->t_self)))
return false;
in a pl. I have to admit though, that I don't immediately know how I
would prevent constant recompilation in a pl handler without such a
hack.
> Actually I'm wondering a bit why there are two hash tables at all.
> Seems like one table with a hash key of (fnoid, reloid) could be
> used, with the convention of reloid = 0 for non-trigger calls.
Seems fine to me. Currently there seems no point in having two distinct
hashes at all, given that they have exactly the same key and that
trigger functions cannot be called plainly anyway.
Unless somebody screams that will mean some stuff defined in plpython
headers will change but that doesn't seem like a legitimate concern to
me.
Tom, any other committer: I am happy to provide a <= 9.1 version of the
patch because plpython has been noticeably restructured in 9.1=>9.2, but
I am not sure if that helps you at all.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
