On Fri, Jan 22, 2010 at 08:59:10PM -0700, Alex Hunsaker wrote:
> On Thu, Jan 14, 2010 at 09:07, Tim Bunce <tim.bu...@pobox.com> wrote:
> > - Allow (ineffective) use of 'require' in plperl
> > If the required module is not already loaded then it dies.
> > So "use strict;" now works in plperl.
>
> [ BTW I think this is awesome! ]
Thanks!
> I'd vote for use warnings; as well.
I would to, but sadly it's not that simple.
warnings uses Carp and Carp uses eval { ... } and, owing to a sad bug in
perl < 5.11.4, Safe can't distinguish between eval "..." and eval {...}
http://rt.perl.org/rt3/Ticket/Display.html?id=70970
So trying to load warnings fails (at least for some versions of perl).
I have a version of my final "Package namespace and Safe init cleanup
for plperl" that works around that. I opted to post a less potentially
controversial version of that patch in the end. If you think allowing
plperl code to 'use warnings;' is important (and I'd tend to agree)
then I'll update that final patch.
> > - Stored procedure subs are now given names.
> > The names are not visible in ordinary use, but they make
> > tools like Devel::NYTProf and Devel::Cover _much_ more useful.
>
> This needs to quote at least '. Any others you can think of? Also I
> think we should sort the imports in ::mkfunsort so that they are
> stable.
Sort for stability, yes. The quoting is fine though (I see you've come
to the same conclusion via David).
> The cleanups were nice, the code worked as described.
Thanks.
> Other than the quoting issue it looks good to me. Find below an
> incremental patch that fixes the items above.
> diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
> index daef469..fa5df0a 100644
> --- a/src/pl/plperl/plc_perlboot.pl
> +++ b/src/pl/plperl/plc_perlboot.pl
> @@ -27,16 +27,14 @@ sub ::mkfuncsrc {
> my $BEGIN = join "\n", map {
> my $names = $imports->{$_} || [];
> "$_->import(qw(@$names));"
> - } keys %$imports;
> + } sort keys %$imports;
Ok, good.
> $name =~ s/\\/\\\\/g;
> $name =~ s/::|'/_/g; # avoid package delimiters
> + $name =~ s/'/\'/g;
Not needed.
> - my $funcsrc;
> - $funcsrc .= qq[ undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src
> } ];
> - #warn "plperl mkfuncsrc: $funcsrc\n";
> - return $funcsrc;
> + return qq[ undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
> }
Ok. (I don't think that'll clash with any later patches.)
> # see also mksafefunc() in plc_safe_ok.pl
> diff --git a/src/pl/plperl/plc_safe_ok.pl b/src/pl/plperl/plc_safe_ok.pl
> index 8d35357..79d64ce 100644
> --- a/src/pl/plperl/plc_safe_ok.pl
> +++ b/src/pl/plperl/plc_safe_ok.pl
> @@ -25,6 +25,7 @@ $PLContainer->share(qw[&elog &return_next
> $PLContainer->permit(qw[caller]);
> ::safe_eval(q{
> require strict;
> + require warnings;
> require feature if $] >= 5.010000;
> 1;
> }) or die $@;
Not viable, sadly.
> On Sat, Jan 23, 2010 at 12:42, David E. Wheeler <da...@kineticode.com> wrote:
> > On Jan 23, 2010, at 11:20 AM, Alex Hunsaker wrote:
> >
> >> Well no, i suppose we could fix that via:
> >> $name =~ s/[:|']/_/g;
> >>
> >> Im betting that was the intent.
> >
> > Doubtful. In Perl, the package separator is either `::` or `'` (for
> > hysterical reasons). So the original code was replacing any package
> > separator with a single underscore. Your regex would change This::Module to
> > This__Module, which I'm certain was not the intent.
>
> Haha, yep your right. I could have sworn I tested it with a function
> name with a ' and it broke. But your obviously right :)
I could have sworn I wrote a test file with a bunch of stressful names.
All seems well though:
template1=# create or replace function "a'b*c}d!"() returns text language
plperl as '42';
CREATE FUNCTION
template1=# select "a'b*c}d!"();
a'b*c}d!
----------
42
So, what now? Should I resend the patch with the two 'ok' changes above
included, or can the committer make those very minor changes?
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
