Feedback-ID: i1ff147bf:Fastmail
From: =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>,  Ivan Panchenko <wao@mail.ru>,  Tom
 Lane <tgl@sss.pgh.pa.us>,  Peter Eisentraut <peter@eisentraut.org>,
  pgsql-hackers@lists.postgresql.org,  Greg Sabino Mullane
 <htamfids@gmail.com>
Subject: Re: Bytea PL/Perl transform
References: <1679178327.555465221@f141.i.mail.ru>
	<1689349073.74310713@f308.i.mail.ru>
	<3450081.1689366442@sss.pgh.pa.us>
	<1689888552.84649942@f381.i.mail.ru>
	<CALDaNm0GWMdDPtJq6XN3vurE2=YcBkOKGpTWbRrpm9T21A5+rA@mail.gmail.com>
	<CAFj8pRB24y1VPNbB3Ej5UK+3-Qp7-TxYtFKdJj6dwqZVAim7=A@mail.gmail.com>
	<87ttmultc8.fsf@wibble.ilmari.org>
	<CAFj8pRAC-+ce7b4RPOYOcO+NfBO8hH-cZbOjS8cPsPgu49vAgg@mail.gmail.com>
	<87r0hylrqq.fsf@wibble.ilmari.org>
	<CAFj8pRAWUEDXHtMj3aNKxunTEeQtweQ_wE5wad469d_B2-3TOQ@mail.gmail.com>
Date: Tue, 30 Jan 2024 16:46:10 +0000
In-Reply-To: 
 <CAFj8pRAWUEDXHtMj3aNKxunTEeQtweQ_wE5wad469d_B2-3TOQ@mail.gmail.com>
	(Pavel Stehule's message of "Tue, 30 Jan 2024 17:24:21 +0100")
Message-ID: <87o7d2lqgd.fsf@wibble.ilmari.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/87o7d2lqgd.fsf%40wibble.ilmari.org>
Precedence: bulk

Pavel Stehule <pavel.stehule@gmail.com> writes:

> út 30. 1. 2024 v 17:18 odesílatel Dagfinn Ilmari Mannsåker <
> ilmari@ilmari.org> napsal:
>
>> Pavel Stehule <pavel.stehule@gmail.com> writes:
>>
>> > út 30. 1. 2024 v 16:43 odesílatel Dagfinn Ilmari Mannsåker <
>> > ilmari@ilmari.org> napsal:
>> >
>> >> Pavel Stehule <pavel.stehule@gmail.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.
>>
>
> ugly test code
>
> (2024-01-30 13:44:28) postgres=# CREATE or replace FUNCTION
> perl_inverse_bytes(bytea) RETURNS bytea
> TRANSFORM FOR TYPE bytea
> AS $$ my $bytes =  pack 'H*', '0123'; my $ref = \$bytes;

You are returning a reference, not a string.

> return $ref;
> $$ LANGUAGE plperlu;
> CREATE FUNCTION
> (2024-01-30 13:44:33) postgres=# select perl_inverse_bytes(''), ' '::bytea;
> ┌──────────────────────────────────────┬───────┐
> │          perl_inverse_bytes          │ bytea │
> ╞══════════════════════════════════════╪═══════╡
> │ \x5343414c41522830783130656134333829 │ \x20  │
> └──────────────────────────────────────┴───────┘
> (1 row)

~=# select encode('\x5343414c41522830783130656134333829', 'escape');
┌───────────────────┐
│      encode       │
├───────────────────┤
│ SCALAR(0x10ea438) │
└───────────────────┘

This is how Perl stringifies references in the absence of overloading.
Return the byte string directly from your function and it will do the
right thing:

CREATE FUNCTION plperlu_bytes() RETURNS bytea LANGUAGE plperlu
 TRANSFORM FOR TYPE bytea
 AS $$ return pack  'H*', '0123'; $$;

SELECT plperlu_bytes();
 plperlu_bytes
---------------
 \x0123
(1 row)
    

- ilmari