Re: Package namespace and Safe init cleanup for plperl [PATCH] - Mailing list pgsql-hackers

On Sat, Jan 30, 2010 at 07:51, Tim Bunce <Tim.Bunce@pobox.com> wrote:
> On Fri, Jan 29, 2010 at 08:07:30PM -0700, Alex Hunsaker wrote:
>> A couple of comments. *note* I have not tested this as a whole yet
>> (due to rejects).
>>
>> in plc_perboot.pl
>> +$funcsrc .= qq[ package main; undef *{'$name'}; *{'$name'} = sub {
>> $BEGIN $prolog $src } ];
>>
>> Any thoughts on using a package other than main?  Maybe something like
>> package PlPerl or what not?  Not that I think it really matters...
>> But it might be nicer to see in say NYTprof ?
>
> The only interface Safe provides for sharing things with the compartment
> shares them with the root package of the compartment which, from within
> the compartment, is 'main::'.
>
> It's an unfortunate limitation of Safe. I could have added more code to
> wordaround that but opted to keep it simple for this commitfest.

I thought it might be something like that.

>> in plc_safe_ok.pl
>> +use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe);
>>
>> Is there some reason not to use my here?  The only reason I can think
>> of is you want the *_init gucs to be able to stick things into
>> @ShareIntoSafe and friends?  And if so should we document that
>> somewhere (or was that in an earlier patch that i missed?)?
>
> It's a step towards providing an interface to give the DBA control over
> what gets loaded into the Safe compartment and what gets shared with it.
>
> I opted to not try to define a formal documented interface because I
> felt it was likely to be a source of controversy and/or bikeshedding.
> This late in the game I didn't want to take that chance.
>
> I'd rather a few brave souls get some experience with it as-as, and collect
> some good use-cases, before proposing a formal documented API.

Fine with me.

>> Also whats the use case for $SafeClass?  Multiple versions of Safe.pm?
>>  Other modules that are like Safe.pm? Im just curious...
>
> It's possible someone might want to use an alternative module.
> Most likely their own local subclass of Safe that adds extra features.
> I'd explored that when trying to integrate NYTProf.  It seemed worth
> at least making possible.
>
>> Hrm I also dont see the point of the new "use Safe;"  as we still eval
>> it in in plperl_safe_init() care to enlighten a lost soul?
>
> It just makes the file more self-contained for syntax checking:
>    perl -cw plc_safeboot.pl
>
>> Other than those really quite minor questions that are arguably me
>> nitpicking...  It looks great to me.
>
> Great, thanks Alex!

I marked it as ready, though ill add a comment that it depends on the
other patch to apply cleanly (and if that one gets rebased...
obviously this one probably will need to as well).