pgsql: Transforms for jsonb to PL/Perl

Peter Eisentraut <peter_e@gmx.net> writes:

Transforms for jsonb to PL/Perl

Buildfarm's not terribly happy with this. I notice this perhaps-
relevant warning on, eg, dromedary:

ccache gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -g -O2 -ansi -I../../src/pl/plpython -I/System/Library/Frameworks/Python.framework/Versions/2.6/include/python2.6 -I../../contrib/hstore -DPLPYTHON_LIBNAME='"plpython2"' -I. -I. -I../../src/include -DCOPY_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -c -o hstore_plpython.o hstore_plpython.c
In file included from jsonb_plperl.c:8:
jsonb_plperl.c: In function 'HV_to_JsonbValue':
jsonb_plperl.c:160: warning: passing argument 4 of 'Perl_hv_iternextsv' from incompatible pointer type

regards, tom lane

I wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Transforms for jsonb to PL/Perl

Buildfarm's not terribly happy with this.

Hm, it fails on my own machine too (RHEL6, perl 5.10.1), with the
same "cannot transform this Perl type to jsonb" symptoms. A bit
of tracing shows that SvTYPE(in) is returning SVt_PVIV in some
of the failing cases, and SVt_PVNV in others.

regards, tom lane

I wrote:

Hm, it fails on my own machine too (RHEL6, perl 5.10.1), with the
same "cannot transform this Perl type to jsonb" symptoms. A bit
of tracing shows that SvTYPE(in) is returning SVt_PVIV in some
of the failing cases, and SVt_PVNV in others.

I tried to fix this by reducing the amount of knowledge that function
embeds about the possible SvTYPEs. After the special cases for AV,
HV, and NULL, the attached just tests SvIOK, SvNOK, and SvPOK, and
does the right thing for each case.

This results in one change in the module's test results: the example
that thinks it's returning a regexp match result no longer fails,
but just returns the scalar result (0). I'm inclined to think that
this is correct/desirable and the existing behavior is an accidental
artifact of not coping with Perl's various augmented representations
of scalar values.

Thoughts?

regards, tom lane

On Tue, 03 Apr 2018 17:37:04 -0400
Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

Hm, it fails on my own machine too (RHEL6, perl 5.10.1), with the
same "cannot transform this Perl type to jsonb" symptoms. A bit
of tracing shows that SvTYPE(in) is returning SVt_PVIV in some
of the failing cases, and SVt_PVNV in others.

I tried to fix this by reducing the amount of knowledge that function
embeds about the possible SvTYPEs. After the special cases for AV,
HV, and NULL, the attached just tests SvIOK, SvNOK, and SvPOK, and
does the right thing for each case.

This results in one change in the module's test results: the example
that thinks it's returning a regexp match result no longer fails,
but just returns the scalar result (0). I'm inclined to think that
this is correct/desirable and the existing behavior is an accidental
artifact of not coping with Perl's various augmented representations
of scalar values.

Thoughts?

regards, tom lane

Hello,
I don't think that user expect having 0 in jsonb when they have regexp:
it should have a possibility to convert resulting jsonb back to perl
with exact same type and data.

--
Anthony Bykov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On Tue, 03 Apr 2018 17:37:04 -0400
Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

Hm, it fails on my own machine too (RHEL6, perl 5.10.1), with the
same "cannot transform this Perl type to jsonb" symptoms. A bit
of tracing shows that SvTYPE(in) is returning SVt_PVIV in some
of the failing cases, and SVt_PVNV in others.

I tried to fix this by reducing the amount of knowledge that function
embeds about the possible SvTYPEs. After the special cases for AV,
HV, and NULL, the attached just tests SvIOK, SvNOK, and SvPOK, and
does the right thing for each case.

This results in one change in the module's test results: the example
that thinks it's returning a regexp match result no longer fails,
but just returns the scalar result (0). I'm inclined to think that
this is correct/desirable and the existing behavior is an accidental
artifact of not coping with Perl's various augmented representations
of scalar values.

Thoughts?

regards, tom lane

Hello.
I think that there is a mistake in test:
CREATE FUNCTION testRegexpToJsonb() RETURNS jsonb
LANGUAGE plperl
TRANSFORM FOR TYPE jsonb
AS $$
return ('1' =~ m(0\t2));
$$;

=~ is the operator testing a regular expression match.
Hence, testRegexpToJsonb function returns true/false values
(when used in scalar context, the return value
generally indicates the success of the operation).

I guess the right test will look a little bit different:
CREATE FUNCTION testRegexpToJsonb() RETURNS jsonb
LANGUAGE plperl
TRANSFORM FOR TYPE jsonb
AS $$
$a = qr//;
return ($a);
$$;

So, this may be the reason why the original testRegexpToJsonb returns
the scalar result.

--
Anthony Bykov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Anthony Bykov <a.bykov@postgrespro.ru> writes:

Tom Lane <tgl@sss.pgh.pa.us> wrote:

This results in one change in the module's test results: the example
that thinks it's returning a regexp match result no longer fails,
but just returns the scalar result (0). I'm inclined to think that
this is correct/desirable and the existing behavior is an accidental
artifact of not coping with Perl's various augmented representations
of scalar values.

I think that there is a mistake in test:
CREATE FUNCTION testRegexpToJsonb() RETURNS jsonb
LANGUAGE plperl
TRANSFORM FOR TYPE jsonb
AS $$
return ('1' =~ m(0\t2));
$$;

=~ is the operator testing a regular expression match.
Hence, testRegexpToJsonb function returns true/false values
(when used in scalar context, the return value
generally indicates the success of the operation).

Right, that was my point: this is returning a scalar result that
just happens to have been derived from a regexp match. So the
output ought to be 1 or 0, and the fact that (on some platforms?)
it isn't represents a bug.

I guess the right test will look a little bit different:
CREATE FUNCTION testRegexpToJsonb() RETURNS jsonb
LANGUAGE plperl
TRANSFORM FOR TYPE jsonb
AS $$
$a = qr//;
return ($a);
$$;

This is testing something else. I don't object to adding it,
but we should keep the existing test in some form to verify
that the bug stays fixed.

regards, tom lane

I wrote:

Anthony Bykov <a.bykov@postgrespro.ru> writes:

I guess the right test will look a little bit different:
CREATE FUNCTION testRegexpToJsonb() RETURNS jsonb
LANGUAGE plperl
TRANSFORM FOR TYPE jsonb
AS $$
$a = qr//;
return ($a);
$$;

This is testing something else. I don't object to adding it,
but we should keep the existing test in some form to verify
that the bug stays fixed.

Huh. I put that in, and it turns out that on some Perl versions
we get a string out instead of "don't know what that is".

***************
*** 48,55 ****
  return ($a);
  $$;
  SELECT testRegexpToJsonb();
! ERROR:  cannot transform this Perl type to jsonb
! CONTEXT:  PL/Perl function "testregexptojsonb"
  -- this revealed a bug in the original implementation
  CREATE FUNCTION testRegexpResultToJsonb() RETURNS jsonb
  LANGUAGE plperl
--- 48,58 ----
  return ($a);
  $$;
  SELECT testRegexpToJsonb();
!  testregexptojsonb 
! -------------------
!  "(?^:foo)"
! (1 row)
! 
  -- this revealed a bug in the original implementation
  CREATE FUNCTION testRegexpResultToJsonb() RETURNS jsonb
  LANGUAGE plperl

So that's probably useful for the people it works for,
but I don't think we want a Perl-version-dependent
regression test for this. I'm inclined to just take
this test case out again.

regards, tom lane