PL/Perl function naming (was: release notes) - Mailing list pgsql-hackers

[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 <andrew@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.