Re: BUG #13638: Exception texts from plperl has bad encoding

On Sun, Sep 27, 2015 at 5:46 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> writes:
> > I think you want to call croak_sv(SV) with an SV containing the error
> > message and flagged as UTF-8.
>
> I found croak_sv() in the headers for Perl 5.18 (present on OS X Yosemite)
> but not in Perl 5.10 (present on RHEL6), much less 5.8 which is the oldest
> Perl version we claim to support. This doesn't look like a workable
> answer unless we want to move the compatibility goalposts a long way.
>

Yeah, I was just looking at that. I couldn't find a good way to support
5.8. Doing:

const char *utf8_str = utf_e2u(str);
SV *sv = get_sv("@", TRUE);
sv_setpvf(sv, "%s", utf8_str);
SvUTF8_on(sv);
croak(NULL);

Works, but screws up the error line numbers. For example, given the below
test in plperl.out:

--
-- Test with a bad type
--
CREATE OR REPLACE FUNCTION perl_spi_prepared_bad(double precision) RETURNS
double precision AS $$
my $x = spi_prepare('SELECT 10.0 * $1 AS a', 'does_not_exist');
my $q = spi_query_prepared($x,$_[0]);
my $result;
while (defined (my $y = spi_fetchrow($q))) {
$result = $y->{a};
}
spi_freeplan($x);
return $result;
$$ LANGUAGE plperl;

Instead of:
ERROR: type "does_not_exist" does not exist at line 2.

We get:
ERROR: type "does_not_exist" does not exist at -e line 56.

I poked at this for a bit and didn't see a way around that.

Attached is a draft patch that at least fixes the issue for 5.10 and up. I
guess we don't want to commit the regression test unless we want to add 2
new plperl_elog.out files (one for 5.8 and one for the already existing alt
file).

Another thing to note is sv2cstr() can elog(ERROR) if the message has an
invalid byte sequence:

create or replace function perl_test_err() returns text
language plperl as $$
elog(ERROR, "\0");
$$;

-- or
create or replace function perl_test_err() returns text
language plperl as $$
elog(INFO, "\0");
$$;
select perl_test_err();
ERROR: invalid byte sequence for encoding "UTF8": 0x00k

Without the PG_TRY block we can't punt these errors back to perl. Maybe
that does not matter all that much. After-all we could still have an error
converting that new error to utf8 for perl (realistically this probably
falls into the impossible category...). Thoughts?

Attached is a patch that does /not/ take into account errors thrown by
pg_any_to_server() along the lines of Tom's above suggestion. I added a
croak_cstr() function (trying to be in the spirit of the other
plperl_helper functions) and converted various croak("%s", e->message);
over to it. Tested on 5.8.9 and 5.18.2 from 9.1-stable to 9.4
(9.5 doesn't seem to pass a standard make check on my machine atm, but I
don't think there have been any plperl changes).

Note it does not apply cleaning to 9.1-9.3 (trivial rejects), I'm happy to
do the leg work if we like this proposed patch.