Re: PL/Perl regression tests with use_strict

The attached patch allows the PL/Perl regression tests to pass when
use_strict is enabled. I've also attached a variant of plperl_elog.out
to account for an elog() message that shows a different line number
when run under use_strict.

--
Michael Fuhr

On Sat, Aug 20, 2005 at 01:52:42PM -0600, Michael Fuhr wrote:
> The attached patch allows the PL/Perl regression tests to pass when
> use_strict is enabled. I've also attached a variant of plperl_elog.out
> to account for an elog() message that shows a different line number
> when run under use_strict.

Here's an updated version of the PL/Perl regression test patch that
works with Andrew Dunstan's strict mode patch, both when use_strict
is enabled and when it's disabled. The variant of plperl_elog.out
is no longer needed.

--
Michael Fuhr

Michael Fuhr <mike(at)fuhr(dot)org> writes:
> Here's an updated version of the PL/Perl regression test patch that
> works with Andrew Dunstan's strict mode patch, both when use_strict
> is enabled and when it's disabled. The variant of plperl_elog.out
> is no longer needed.

Actually, the main reason I didn't apply the prior version right
away was that the variant .out file was bugging me. Why does the
error report contain a line number that's dependent on implementation
internals in the first place? Changing it to a different number
doesn't seem like an improvement; can't we get rid of that entirely?

regards, tom lane

On Tue, Aug 23, 2005 at 11:58:25PM -0400, Tom Lane wrote:
> Michael Fuhr <mike(at)fuhr(dot)org> writes:
> > Here's an updated version of the PL/Perl regression test patch that
> > works with Andrew Dunstan's strict mode patch, both when use_strict
> > is enabled and when it's disabled. The variant of plperl_elog.out
> > is no longer needed.
>
> Actually, the main reason I didn't apply the prior version right
> away was that the variant .out file was bugging me. Why does the
> error report contain a line number that's dependent on implementation
> internals in the first place? Changing it to a different number
> doesn't seem like an improvement; can't we get rid of that entirely?

Actually, I just noticed that the varying number isn't a line number
but rather a sequence number. Example:

% cat foo
#!/usr/bin/perl
use strict;
use warnings;
my $code = '$x = 123;';
eval $code; print $@;
eval $code; print $@;
eval $code; print $@;

% ./foo
Global symbol "$x" requires explicit package name at (eval 1) line 1.
Global symbol "$x" requires explicit package name at (eval 2) line 1.
Global symbol "$x" requires explicit package name at (eval 3) line 1.

If I'm reading the Perl source code correctly (pp_ctl.c), the number
following "eval" comes from a variable named PL_evalseq that's
incremented each time it appears in one of these messages. It looks
like we'd have to munge the error message to get rid of that.

--
Michael Fuhr

On Tue, Aug 23, 2005 at 10:30:51PM -0600, Michael Fuhr wrote:
> Global symbol "$x" requires explicit package name at (eval 3) line 1.
>
> If I'm reading the Perl source code correctly (pp_ctl.c), the number
> following "eval" comes from a variable named PL_evalseq that's
> incremented each time it appears in one of these messages. It looks
> like we'd have to munge the error message to get rid of that.

Hmmm...tests suggest that we might be able to munge $@ in the
mk*safefunc functions. That is, instead of doing

return eval($stuff);

we might be able to do

my $retval = eval($stuff);
$@ =~ s/ \(eval \d+\) / /g if $@;
return $retval;

That would convert messages like

Global symbol "$x" requires explicit package name at (eval 3) line 1.

into

Global symbol "$x" requires explicit package name at line 1.

Is that what you're looking for? So far I've done only simple tests
in standalone embedded Perl programs, so I don't know if this approach
would work in PL/Perl or have unintended effects.

--
Michael Fuhr

Michael Fuhr said:
> On Tue, Aug 23, 2005 at 10:30:51PM -0600, Michael Fuhr wrote:
>> Global symbol "$x" requires explicit package name at (eval 3) line 1.
>>
>> If I'm reading the Perl source code correctly (pp_ctl.c), the number
>> following "eval" comes from a variable named PL_evalseq that's
>> incremented each time it appears in one of these messages. It looks
>> like we'd have to munge the error message to get rid of that.
>
> Hmmm...tests suggest that we might be able to munge $@ in the
> mk*safefunc functions. That is, instead of doing
>
> return eval($stuff);
>
> we might be able to do
>
> my $retval = eval($stuff);
> $@ =~ s/ \(eval \d+\) / /g if $@;
> return $retval;
>
> That would convert messages like
>
> Global symbol "$x" requires explicit package name at (eval 3) line 1.
>
> into
>
> Global symbol "$x" requires explicit package name at line 1.
>
> Is that what you're looking for? So far I've done only simple tests in
> standalone embedded Perl programs, so I don't know if this approach
> would work in PL/Perl or have unintended effects.

It would probably be more efficient and less convoluted to munge this in a
__DIE__ handler. The we wouldn't need the extra level of eval.

e.g.

$SIG{__DIE__} =
sub { my $msg = $_[0]; $msg =~ s/\(eval \d+\) //; die $msg; };

cheers

andrew

I wrote:

>Michael Fuhr said:
>
>
>>we might be able to do
>>
>> my $retval = eval($stuff);
>> $@ =~ s/ \(eval \d+\) / /g if $@;
>> return $retval;
>>
>>T
>>
>It would probably be more efficient and less convoluted to munge this in a
>__DIE__ handler. The we wouldn't need the extra level of eval.
>
>e.g.
>
>$SIG{__DIE__} =
> sub { my $msg = $_[0]; $msg =~ s/\(eval \d+\) //; die $msg; };
>
>
>
>

Or rather it would do if we didn't carefully avoid the die handler so we
can get our hands on the message.

Here's an updated patch incorporating Michael's ideas, and this time
*with* a small regression test that dynamically turns strict mode on/off.

cheers

andrew

On Wed, Aug 24, 2005 at 09:50:06AM -0400, Andrew Dunstan wrote:
> Here's an updated patch incorporating Michael's ideas, and this time
> *with* a small regression test that dynamically turns strict mode on/off.

Shouldn't the $@ munging patterns include the /g flag so they remove
all occurrences of the pattern?

SET plperl.use_strict TO on;

CREATE FUNCTION foo() RETURNS integer AS $$
$x = 1;
$y = 2;
return $x + $y;
$$ LANGUAGE plperl;

ERROR: creation of Perl function failed: Global symbol "$x" requires explicit package name at line 2.
Global symbol "$y" requires explicit package name at (eval 10) line 3.
Global symbol "$x" requires explicit package name at (eval 10) line 4.
Global symbol "$y" requires explicit package name at (eval 10) line 4.

--
Michael Fuhr

Michael Fuhr wrote:

>On Wed, Aug 24, 2005 at 09:50:06AM -0400, Andrew Dunstan wrote:
>
>
>>Here's an updated patch incorporating Michael's ideas, and this time
>>*with* a small regression test that dynamically turns strict mode on/off.
>>
>>
>
>Shouldn't the $@ munging patterns include the /g flag so they remove
>all occurrences of the pattern?
>
>SET plperl.use_strict TO on;
>
>CREATE FUNCTION foo() RETURNS integer AS $$
>$x = 1;
>$y = 2;
>return $x + $y;
>$$ LANGUAGE plperl;
>
>ERROR: creation of Perl function failed: Global symbol "$x" requires explicit package name at line 2.
>Global symbol "$y" requires explicit package name at (eval 10) line 3.
>Global symbol "$x" requires explicit package name at (eval 10) line 4.
>Global symbol "$y" requires explicit package name at (eval 10) line 4.
>
>
>

good point.

Here's yet another revision ;-)

cheers

andrew

Michael Fuhr <mike(at)fuhr(dot)org> writes:
> The attached patch allows the PL/Perl regression tests to pass when
> use_strict is enabled. I've also attached a variant of plperl_elog.out
> to account for an elog() message that shows a different line number
> when run under use_strict.

Now that we've got the use_strict mess sorted, I've applied this.

regards, tom lane