Re: Bytea PL/Perl transform

Hi

so 6. 1. 2024 v 16:51 odesílatel vignesh C <vignesh21(at)gmail(dot)com> napsal:

> On Fri, 21 Jul 2023 at 02:59, Ivan Panchenko <wao(at)mail(dot)ru> wrote:
> >
> > Friday, 14 July 2023, 23:27 +03:00 от Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> >
> > =?UTF-8?B?SXZhbiBQYW5jaGVua28=?= <wao(at)mail(dot)ru> writes:
> > > Четверг, 6 июля 2023, 14:48 +03:00 от Peter Eisentraut <
> peter(at)eisentraut(dot)org >:
> > >> If the transform deals with a built-in type, then they should just be
> > >> added to the respective pl extension directly.
> >
> > > The new extension bytea_plperl can be easily moved into plperl now,
> but what should be do with the existing ones, namely jsonb_plperl and
> bool_plperl ?
> > > If we leave them where they are, it would be hard to explain why some
> transforms are inside plperl while other ones live separately. If we move
> them into plperl also, wouldn’t it break some compatibility?
> >
> > It's kind of a mess, indeed. But I think we could make plperl 1.1
> > contain the additional transforms and just tell people they have
> > to drop the obsolete extensions before they upgrade to 1.1.
> > Fortunately, it doesn't look like functions using a transform
> > have any hard dependency on the transform, so "drop extension
> > jsonb_plperl" followed by "alter extension plperl update" should
> > work without cascading to all your plperl functions.
> >
> > Having said that, we'd still be in the position of having to
> > explain why some transforms are packaged with plperl and others
> > not. The distinction between built-in and contrib types might
> > be obvious to us hackers, but I bet a lot of users see it as
> > pretty artificial. So maybe the existing packaging design is
> > fine and we should just look for a way to reduce the code
> > duplication.
> >
> > The code duplication between different transforms is not in C code, but
> mostly in copy-pasted Makefile, *.control, *.sql and meson-build. These
> files could be generated from some universal templates. But, keeping in
> mind Windows compatibility and presence of three build system, this hardly
> looks like a simplification.
> > Probably at present time it would be better to keep the existing code
> duplication until plperl 1.1.
> > Nevertheless, dealing with code duplication is a wider task than the
> bytea transform, so let me suggest to keep this extension in the present
> form. If we decide what to do with the duplication, it would be another
> patch.
> >
> > The mesonified and rebased version of the transform patch is attached.
>
> The patch needs to be rebased as these changes are not required anymore:
> diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
> index 9e05eb91b1..ec0a3f8097 100644
> --- a/src/tools/msvc/Mkvcbuild.pm
> +++ b/src/tools/msvc/Mkvcbuild.pm
> @@ -43,7 +43,7 @@ my $contrib_extralibs = { 'libpq_pipeline' =>
> ['ws2_32.lib'] };
> my $contrib_extraincludes = {};
> my $contrib_extrasource = {};
> my @contrib_excludes = (
> - 'bool_plperl', 'commit_ts',
> + 'bool_plperl', 'bytea_plperl', 'commit_ts',
> 'hstore_plperl', 'hstore_plpython',
> 'intagg', 'jsonb_plperl',
> 'jsonb_plpython', 'ltree_plpython',
> @@ -791,6 +791,9 @@ sub mkvcbuild
> my $bool_plperl = AddTransformModule(
> 'bool_plperl', 'contrib/bool_plperl',
> 'plperl', 'src/pl/plperl');
> + my $bytea_plperl = AddTransformModule(
> + 'bytea_plperl', 'contrib/bytea_plperl',
> + 'plperl', 'src/pl/plperl');
>
> Regards,
> Vignesh
>
>
>
I am checking this patch, it looks well. All tests passed. I am sending a
cleaned patch.

I did minor formatting cleaning.

I inserted perl reference support - hstore_plperl and json_plperl does it.

+<->/* Dereference references recursively. */
+<->while (SvROK(in))
+<-><-->in = SvRV(in);

Regards

Pavel