Thread: pgsql: Transforms for jsonb to PL/Perl

pgsql: Transforms for jsonb to PL/Perl

From
Peter Eisentraut
Date:
Transforms for jsonb to PL/Perl

Add a new contrib module jsonb_plperl that provides a transform between
jsonb and PL/Perl.  jsonb values are converted to appropriate Perl types
such as arrays and hashes, and vice versa.

Author: Anthony Bykov <a.bykov@postgrespro.ru>
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Reviewed-by: Aleksander Alekseev <a.alekseev@postgrespro.ru>
Reviewed-by: Nikita Glukhov <n.gluhov@postgrespro.ru>

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/341e1661805879db958dde0a9ed1dc44b1bb10c3

Modified Files
--------------
contrib/Makefile                                |   4 +-
contrib/jsonb_plperl/.gitignore                 |   4 +
contrib/jsonb_plperl/Makefile                   |  40 ++++
contrib/jsonb_plperl/expected/jsonb_plperl.out  | 211 +++++++++++++++++++
contrib/jsonb_plperl/expected/jsonb_plperlu.out | 211 +++++++++++++++++++
contrib/jsonb_plperl/jsonb_plperl--1.0.sql      |  19 ++
contrib/jsonb_plperl/jsonb_plperl.c             | 262 ++++++++++++++++++++++++
contrib/jsonb_plperl/jsonb_plperl.control       |   6 +
contrib/jsonb_plperl/jsonb_plperlu--1.0.sql     |  19 ++
contrib/jsonb_plperl/jsonb_plperlu.control      |   6 +
contrib/jsonb_plperl/sql/jsonb_plperl.sql       |  86 ++++++++
contrib/jsonb_plperl/sql/jsonb_plperlu.sql      |  86 ++++++++
doc/src/sgml/json.sgml                          |  13 +-
13 files changed, 963 insertions(+), 4 deletions(-)


Re: pgsql: Transforms for jsonb to PL/Perl

From
Tom Lane
Date:
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


Re: pgsql: Transforms for jsonb to PL/Perl

From
Tom Lane
Date:
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


Re: pgsql: Transforms for jsonb to PL/Perl

From
Tom Lane
Date:
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

diff --git a/contrib/jsonb_plperl/expected/jsonb_plperl.out b/contrib/jsonb_plperl/expected/jsonb_plperl.out
index 5bb5677..4af2fbb 100644
*** a/contrib/jsonb_plperl/expected/jsonb_plperl.out
--- b/contrib/jsonb_plperl/expected/jsonb_plperl.out
*************** AS $$
*** 46,53 ****
  return ('1' =~ m(0\t2));
  $$;
  SELECT testRegexpToJsonb();
! ERROR:  cannot transform this Perl type to jsonb
! CONTEXT:  PL/Perl function "testregexptojsonb"
  CREATE FUNCTION roundtrip(val jsonb) RETURNS jsonb
  LANGUAGE plperl
  TRANSFORM FOR TYPE jsonb
--- 46,56 ----
  return ('1' =~ m(0\t2));
  $$;
  SELECT testRegexpToJsonb();
!  testregexptojsonb
! -------------------
!  0
! (1 row)
!
  CREATE FUNCTION roundtrip(val jsonb) RETURNS jsonb
  LANGUAGE plperl
  TRANSFORM FOR TYPE jsonb
diff --git a/contrib/jsonb_plperl/expected/jsonb_plperlu.out b/contrib/jsonb_plperl/expected/jsonb_plperlu.out
index 9527e9e..6fc6c3d 100644
*** a/contrib/jsonb_plperl/expected/jsonb_plperlu.out
--- b/contrib/jsonb_plperl/expected/jsonb_plperlu.out
*************** AS $$
*** 46,53 ****
  return ('1' =~ m(0\t2));
  $$;
  SELECT testRegexpToJsonb();
! ERROR:  cannot transform this Perl type to jsonb
! CONTEXT:  PL/Perl function "testregexptojsonb"
  CREATE FUNCTION roundtrip(val jsonb) RETURNS jsonb
  LANGUAGE plperlu
  TRANSFORM FOR TYPE jsonb
--- 46,56 ----
  return ('1' =~ m(0\t2));
  $$;
  SELECT testRegexpToJsonb();
!  testregexptojsonb
! -------------------
!  0
! (1 row)
!
  CREATE FUNCTION roundtrip(val jsonb) RETURNS jsonb
  LANGUAGE plperlu
  TRANSFORM FOR TYPE jsonb
diff --git a/contrib/jsonb_plperl/jsonb_plperl.c b/contrib/jsonb_plperl/jsonb_plperl.c
index ad9e655..79301d9 100644
*** a/contrib/jsonb_plperl/jsonb_plperl.c
--- b/contrib/jsonb_plperl/jsonb_plperl.c
*************** SV_to_JsonbValue(SV *in, JsonbParseState
*** 188,233 ****
          case SVt_PVHV:
              return HV_to_JsonbValue((HV *) in, jsonb_state);

!         case SVt_NV:
!         case SVt_IV:
              {
!                 char       *str = sv2cstr(in);

!                 /*
!                  * Use case-insensitive comparison because infinity
!                  * representation varies across Perl versions.
!                  */
!                 if (pg_strcasecmp(str, "inf") == 0)
                      ereport(ERROR,
!                             (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                               (errmsg("cannot convert infinite value to jsonb"))));

                  out.type = jbvNumeric;
!                 out.val.numeric = DatumGetNumeric(DirectFunctionCall3(numeric_in,
!                                                                       CStringGetDatum(str), 0, -1));
              }
-             break;
-
-         case SVt_NULL:
-             out.type = jbvNull;
-             break;
-
-         case SVt_PV:            /* string */
-             out.type = jbvString;
-             out.val.string.val = sv2cstr(in);
-             out.val.string.len = strlen(out.val.string.val);
-             break;
-
-         default:
-
-             /*
-              * XXX It might be nice if we could include the Perl type in the
-              * error message.
-              */
-             ereport(ERROR,
-                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                      (errmsg("cannot transform this Perl type to jsonb"))));
-             return NULL;
      }

      /* Push result into 'jsonb_state' unless it is a raw scalar. */
--- 188,238 ----
          case SVt_PVHV:
              return HV_to_JsonbValue((HV *) in, jsonb_state);

!         case SVt_NULL:
!             out.type = jbvNull;
!             break;
!
!         default:
!             if (SvIOK(in))
              {
!                 IV            ival = SvIV(in);

!                 out.type = jbvNumeric;
!                 out.val.numeric =
!                     DatumGetNumeric(DirectFunctionCall1(int8_numeric,
!                                                         Int64GetDatum((int64) ival)));
!             }
!             else if (SvNOK(in))
!             {
!                 double        nval = SvNV(in);
!
!                 if (isinf(nval))
                      ereport(ERROR,
!                             (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                               (errmsg("cannot convert infinite value to jsonb"))));

                  out.type = jbvNumeric;
!                 out.val.numeric =
!                     DatumGetNumeric(DirectFunctionCall1(float8_numeric,
!                                                         Float8GetDatum(nval)));
!             }
!             else if (SvPOK(in))
!             {
!                 out.type = jbvString;
!                 out.val.string.val = sv2cstr(in);
!                 out.val.string.len = strlen(out.val.string.val);
!             }
!             else
!             {
!                 /*
!                  * XXX It might be nice if we could include the Perl type in
!                  * the error message.
!                  */
!                 ereport(ERROR,
!                         (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                          (errmsg("cannot transform this Perl type to jsonb"))));
!                 return NULL;
              }
      }

      /* Push result into 'jsonb_state' unless it is a raw scalar. */

Re: pgsql: Transforms for jsonb to PL/Perl

From
Anthony Bykov
Date:
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


Re: pgsql: Transforms for jsonb to PL/Perl

From
Anthony Bykov
Date:
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


Re: pgsql: Transforms for jsonb to PL/Perl

From
Tom Lane
Date:
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


Re: pgsql: Transforms for jsonb to PL/Perl

From
Tom Lane
Date:
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