Re: Bytea PL/Perl transform

From: Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Ivan Panchenko <wao(at)mail(dot)ru>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Greg Sabino Mullane <htamfids(at)gmail(dot)com>
Subject: Re: Bytea PL/Perl transform
Date: 2024-01-30 16:18:21
Message-ID: 87r0hylrqq.fsf@wibble.ilmari.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:

> út 30. 1. 2024 v 16:43 odesílatel Dagfinn Ilmari Mannsåker <
> ilmari(at)ilmari(dot)org> napsal:
>
>> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>>
>> > I inserted perl reference support - hstore_plperl and json_plperl does
>> it.
>> >
>> > +<->/* Dereference references recursively. */
>> > +<->while (SvROK(in))
>> > +<-><-->in = SvRV(in);
>>
>> That code in hstore_plperl and json_plperl is only relevant because they
>> deal with non-scalar values (hashes for hstore, and also arrays for
>> json) which must be passed as references. The recursive nature of the
>> dereferencing is questionable, and masked the bug fixed by commit
>> 1731e3741cbbf8e0b4481665d7d523bc55117f63.
>>
>> bytea_plperl only deals with scalars (specifically strings), so should
>> not concern itself with references. In fact, this code breaks returning
>> objects with overloaded stringification, for example:
>>
>> CREATE FUNCTION plperlu_overload() RETURNS bytea LANGUAGE plperlu
>> TRANSFORM FOR TYPE bytea
>> AS $$
>> package StringOverload { use overload '""' => sub { "stuff" }; }
>> return bless {}, "StringOverload";
>> $$;
>>
>> This makes the server crash with an assertion failure from Perl because
>> SvPVbyte() was passed a non-scalar value:
>>
>> postgres: ilmari regression_bytea_plperl [local] SELECT: sv.c:2865:
>> Perl_sv_2pv_flags:
>> Assertion `SvTYPE(sv) != SVt_PVAV && SvTYPE(sv) != SVt_PVHV && SvTYPE(sv)
>> != SVt_PVFM' failed.
>>
>> If I remove the dereferincing loop it succeeds:
>>
>> SELECT encode(plperlu_overload(), 'escape') AS string;
>> string
>> --------
>> stuff
>> (1 row)
>>
>> Attached is a v2 patch which removes the dereferencing and includes the
>> above example as a test.
>>
>
> But without dereference it returns bad value.

Where exactly does it return a bad value? The existing tests pass, and
the one I included shows that it does the right thing in that case too.
If you pass it an unblessed reference it returns the stringified version
of that, as expected.

CREATE FUNCTION plperl_reference() RETURNS bytea LANGUAGE plperl
TRANSFORM FOR TYPE bytea
AS $$ return []; $$;

SELECT encode(plperl_reference(), 'escape') string;
string
-----------------------
ARRAY(0x559a3109f0a8)
(1 row)

This would also crash if the dereferencing loop was left in place.

> Maybe there should be a check so references cannot be returned? Probably is
> not safe pass pointers between Perl and Postgres.

There's no reason to ban references, that would break every Perl
programmer's expectations. And there are no pointers being passed,
SvPVbyte() returns the stringified form of whatever's passed in, which
is well-behaved for both blessed and unblessed references.

If we really want to be strict, we should at least allow references to
objects that overload stringification, as they are explicitly designed
to be well-behaved as strings. But that would be a lot of extra code
for very little benefit over just letting Perl stringify everything.

- ilmari

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2024-01-30 16:24:21 Re: Bytea PL/Perl transform
Previous Message Akshat Jaimini 2024-01-30 16:16:20 Re: Parallelize correlated subqueries that execute within each worker