[Sorry for the delay. I'd stopped checking the pgsql mailing lists. Thanks to David Wheeler and Josh Berkus for the heads-up.]
On Sun, May 30, 2010 at 06:58:32PM -0400, Andrew Dunstan wrote: > > Tim Bunce wrote: > >On Mon, May 17, 2010 at 11:34:37AM -0400, Tom Lane wrote: > >>Andrew Dunstan <and...@dunslane.net> writes: > >>>Why do the release notes say this, under plperl: > >>> * PL/Perl subroutines are now given names (Tim Bunce) > >>> This is for the use of profiling and code coverage tools. DIDN'T > >>> THEY HAVE NAMES BEFORE? > >>> If whoever put this in the release notes had bothered > >>>to ask I am sure we would have been happy to explain. > >>You don't need to complain, just fix it. The point of the comment is > >>that more explanation is needed. > > > >I think you can just delete it. It's too esoteric to be worth noting. > > > >Tim. > > > >p.s. It also turned to be insufficiently useful for NYTProf since it > >doesn't also update some internals to record the 'filename' and line > >number range of the sub. So PostgreSQL::PLPerl::NYTProf works around > >that by wrapping mkfuncsrc() to edit the generated perl code to include > >a subname in the usual "sub $name { ... }" style. I may offer a patch > >for 9.1 to to make it work that way. > > I put this aside to think about it. > > If the "feature" is not any use should we rip it out? We pretty much > included it because you said it was what you needed for the > profiler. Yes, it can go. *** a/src/pl/plperl/plperl.c --- b/src/pl/plperl/plperl.c *************** plperl_create_sub(plperl_proc_desc *prod *** 1319,1328 **** (errmsg("didn't get a CODE ref from compiling %s", prodesc->proname))); - /* give the subroutine a proper name in the main:: symbol table */ - CvGV(SvRV(subref)) = (GV *) newSV(0); - gv_init(CvGV(SvRV(subref)), PL_defstash, subname, strlen(subname), TRUE); - prodesc->reference = subref; return; > I'm seriously nervous about adding function names - making functions > directly callable like that could be a major footgun recipe, since > now we are free to hide some stuff in the calling mechanism, and can > (and have in the past) changed that to suit our needs, e.g. when we > added trigger support via a hidden function argument. That has been > safe precisely because nobody has had a handle on the subroutine > which would enable it to be called directly from perl code. There > have been suggestions in the past of using this for other features, > so we should not assume that the calling mechanism is fixed in stone. Agreed. It is a very hard to use footgun though because the oid is included in the name. It's certainly not something anyone would shoot themselves with by accident. [Speaking of calling conventions: I never did get time for some decent performance optimization. I'd like to get rid of the explicit extra trigger data argument and corresponding "local $_TD=shift;" prologue. That could be done much faster in C.] > Perhaps we could decorate the subs with attributes, or is that not > doable for anonymous subs? Perhaps. I'll look into it when I get around to working on the PL/Perl NYTProf again. For the profiler it would be enough to only enable the naming when the appropriate perl debugging flag bits are set, so it wouldn't happen normally. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers