Обсуждение: [GENERAL] Small patch for PL/Perl Misbehavior with Runtime Error Reporting

Good day,

I just stumbled across this peculiarity in PL/Perl today writing a method
to invoke Perl Regexes from a function: if a run-time error is raised in
an otherwise good function, the function will never run correctly again
until the connection to the database is reset. I poked around in the code
and it appears that it's because when elog() raises the ERROR, it doesn't
first take action to erase the system error message ($@) and consequently
every subsequent run has an error raised, even if it runs successfully.

For example:

-- This comparison works fine.

template1=# SELECT perl_re_match('test', 'test');perl_re_match
---------------t
(1 row)

-- This one dies, for obvious reasons.

template1=# SELECT perl_re_match('test', 't{1}+?');
ERROR:  plperl: error from function:    (in cleanup) Nested quantifiers
before HERE mark in regex m/t{1}+ << HERE ?/ at (eval 2) line 4.

-- This should work fine again, but we still have this error raised...!

template1=# SELECT perl_re_match('test', 'test');
ERROR:  plperl: error from function:    (in cleanup) Nested quantifiers
before HERE mark in regex m/t{1}+ << HERE ?/ at (eval 2) line 4.

I don't know if the following is the best way to solve it, but I got
around it by modifying the error report in this part of PL/Perl to be a
NOTICE, cleared the $@ variable, and then raised the fatal ERROR. A simple
three line patch to plperl.c follows, and is attached.

src/pl/plperl/plperl.c:
443c443,445
<               elog(ERROR, "plperl: error from function: %s", SvPV(ERRSV, PL_na));
---
>               elog(NOTICE, "plperl: error from function: %s", SvPV(ERRSV, PL_na));
>               sv_setpv(perl_get_sv("@",FALSE),"");
>               elog(ERROR, "plperl: error was fatal.");

Best Regards,
Jw.
-- 
John Worsley - lx@openvein.com
http://www.openvein.com/

John Worsley <lx@openvein.com> writes:
> I just stumbled across this peculiarity in PL/Perl today writing a method
> to invoke Perl Regexes from a function: if a run-time error is raised in
> an otherwise good function, the function will never run correctly again
> until the connection to the database is reset. I poked around in the code
> and it appears that it's because when elog() raises the ERROR, it doesn't
> first take action to erase the system error message ($@) and consequently
> every subsequent run has an error raised, even if it runs successfully.

That seems a little weird.  Does Perl really expect people to do that
(ie, is it a documented part of some API)?  I wonder whether there is
some other action that we're supposed to take instead, but are
missing...

> src/pl/plperl/plperl.c:
> 443c443,445
> <               elog(ERROR, "plperl: error from function: %s", SvPV(ERRSV, PL_na));
> ---
>> elog(NOTICE, "plperl: error from function: %s", SvPV(ERRSV, PL_na));
>> sv_setpv(perl_get_sv("@",FALSE),"");
>> elog(ERROR, "plperl: error was fatal.");

If this is what we'd have to do, I think a better way would be
perlerrmsg = pstrdup(SvPV(ERRSV, PL_na));sv_setpv(perl_get_sv("@",FALSE),"");elog(ERROR, "plperl: error from function:
%s",perlerrmsg);
 

Splitting the ERROR into a NOTICE with the useful info and an ERROR
without any isn't real good, because the NOTICE could get dropped on the
floor (either because of min_message_level or a client that just plain
loses notices).
        regards, tom lane

On Thu, 3 Oct 2002, Tom Lane wrote:
>That seems a little weird.  Does Perl really expect people to do that
>(ie, is it a documented part of some API)?  I wonder whether there is
>some other action that we're supposed to take instead, but are
>missing...

Not that I know of: clearing out the $@ variable manually was just my way
of getting around the problem in practice. I think the underlying issue
may be tied to the fact that it's running a function generated within a
Safe Module, but I'm not enough of a Perl Guru to say anything more
decisive than that.

>If this is what we'd have to do, I think a better way would be
>
>    perlerrmsg = pstrdup(SvPV(ERRSV, PL_na));
>    sv_setpv(perl_get_sv("@",FALSE),"");
>    elog(ERROR, "plperl: error from function: %s", perlerrmsg);
>Splitting the ERROR into a NOTICE with the useful info and an ERROR
>without any isn't real good, because the NOTICE could get dropped on the
>floor (either because of min_message_level or a client that just plain
>loses notices).

Yeah, that's a cleaner solution. I take it anything pstrdup'd by
PostgreSQL gets freed automatically by the backend? (I wasn't familiar
enough with the backend to know how to ask for memory confident in the
understanding that it would at some point be freed. ;)

Jw.
-- 
John Worsley - lx@openvein.com
http://www.openvein.com/

John Worsley <lx@openvein.com> writes:
> Yeah, that's a cleaner solution. I take it anything pstrdup'd by
> PostgreSQL gets freed automatically by the backend?

Pretty much.  The only situation where it wouldn't be is if
CurrentMemoryContext is pointing at TopMemoryContext or another
long-lived context --- but we are *very* chary about how much code we
allow to run with such a setting.  User-definable functions can safely
assume that palloc'd space will live only long enough for them to return
something to their caller in it.
        regards, tom lane

Did we ever address this plperl issue?  I see the code unchanged in CVS.

---------------------------------------------------------------------------

Tom Lane wrote:
> John Worsley <lx@openvein.com> writes:
> > I just stumbled across this peculiarity in PL/Perl today writing a method
> > to invoke Perl Regexes from a function: if a run-time error is raised in
> > an otherwise good function, the function will never run correctly again
> > until the connection to the database is reset. I poked around in the code
> > and it appears that it's because when elog() raises the ERROR, it doesn't
> > first take action to erase the system error message ($@) and consequently
> > every subsequent run has an error raised, even if it runs successfully.
> 
> That seems a little weird.  Does Perl really expect people to do that
> (ie, is it a documented part of some API)?  I wonder whether there is
> some other action that we're supposed to take instead, but are
> missing...
> 
> > src/pl/plperl/plperl.c:
> > 443c443,445
> > <               elog(ERROR, "plperl: error from function: %s", SvPV(ERRSV, PL_na));
> > ---
> >> elog(NOTICE, "plperl: error from function: %s", SvPV(ERRSV, PL_na));
> >> sv_setpv(perl_get_sv("@",FALSE),"");
> >> elog(ERROR, "plperl: error was fatal.");
> 
> If this is what we'd have to do, I think a better way would be
> 
>     perlerrmsg = pstrdup(SvPV(ERRSV, PL_na));
>     sv_setpv(perl_get_sv("@",FALSE),"");
>     elog(ERROR, "plperl: error from function: %s", perlerrmsg);
> 
> Splitting the ERROR into a NOTICE with the useful info and an ERROR
> without any isn't real good, because the NOTICE could get dropped on the
> floor (either because of min_message_level or a client that just plain
> loses notices).
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Did we ever address this plperl issue?  I see the code unchanged in CVS.

It's fixed, although not in the way proposed in that patch.
        regards, tom lane