pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Ivan Panchenko <wao@mail.ru>
To: pgsql-hackers@lists.postgresql.org
Subject: jsonb_plperl bug
Date: Sat, 03 Aug 2019 01:05:33 +0300
Message-ID: <1564783533.324795401@f193.i.mail.ru> (raw)

Hi, 

I have found a bug in jsonb_plperl extension. A possible fix is proposed below.

jsonb_perl is the contrib module, which defines TRANSFORM functions for jsonb data type and PL/Perl procedural language.

The bug can be reproduced as follows:

CREATE EXTENSION plperl;
CREATE EXTENSION jsonb_plperl;

CREATE OR REPLACE FUNCTION text2jsonb (text) RETURNS jsonb 
    LANGUAGE plperl TRANSFORM FOR TYPE jsonb AS 
$$ 
    my $x = shift; 
    my $ret = {a=>$x};
    return $ret;
$$;
SELECT text2jsonb(NULL);
SELECT text2jsonb('11');
SELECT text2jsonb(NULL);

The last SELECT produces a strange error.

ERROR:  cannot transform this Perl type to jsonb

A brief research has shown that the problem is in an incomplete logic inside the transform function. The reason can be illustrated by the flollowing Perl one-liner:


perl -MDevel::Peek  -e 'sub x { my $x = shift; Dump $x; warn "----\n\n"; }; x(undef); x("a"); x(undef); '

It outputs:
SV = NULL(0x0) at 0x73a1b8
  REFCNT = 1
  FLAGS = (PADMY)
----

SV = PV(0x71da50) at 0x73a1b8
  REFCNT = 1
  FLAGS = (PADMY,POK,pPOK)
  PV = 0x7409a0 "a"\0
  CUR = 1
  LEN = 16
----

SV = PV(0x71da50) at 0x73a1b8
  REFCNT = 1
  FLAGS = (PADMY)
  PV = 0x7409a0 "a"\0
  CUR = 1
  LEN = 16
----

This shows that internal representation of the same  undef   in perl is different in first and third function calls. 
It is the way Perl reuses the the lexical variable, probably, for optimization reasons.

Current jsonb_plperl implementation works good for the first (most evident) case, but does not work at all for the third, which results in the abovementioned error.

The attached patch solves this issue and defines corresponding tests.

Regards,
Ivan















Attachments:

  [application/x-patch] jsonb_plperl_null_bug.patch (3.7K, ../1564783533.324795401@f193.i.mail.ru/3-jsonb_plperl_null_bug.patch)
  download | inline diff:
diff --git a/contrib/jsonb_plperl/expected/jsonb_plperl.out b/contrib/jsonb_plperl/expected/jsonb_plperl.out
index 6dc090a..b784ca1 100644
--- a/contrib/jsonb_plperl/expected/jsonb_plperl.out
+++ b/contrib/jsonb_plperl/expected/jsonb_plperl.out
@@ -228,6 +228,31 @@ SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}', 'HASH');
  {"1": {"2": [3, 4, 5]}, "2": 3}
 (1 row)
 
+CREATE FUNCTION text2jsonb (text) RETURNS jsonb
+LANGUAGE plperl
+TRANSFORM FOR TYPE jsonb
+AS $$
+    my $x = shift;
+    return {a=>$x};
+$$;
+SELECT text2jsonb(NULL);
+ text2jsonb  
+-------------
+ {"a": null}
+(1 row)
+
+SELECT text2jsonb('11');
+ text2jsonb  
+-------------
+ {"a": "11"}
+(1 row)
+
+SELECT text2jsonb(NULL);
+ text2jsonb  
+-------------
+ {"a": null}
+(1 row)
+
 \set VERBOSITY terse \\ -- suppress cascade details
 DROP EXTENSION plperl CASCADE;
-NOTICE:  drop cascades to 7 other objects
+NOTICE:  drop cascades to 8 other objects
diff --git a/contrib/jsonb_plperl/expected/jsonb_plperlu.out b/contrib/jsonb_plperl/expected/jsonb_plperlu.out
index 434327b..7fe0721 100644
--- a/contrib/jsonb_plperl/expected/jsonb_plperlu.out
+++ b/contrib/jsonb_plperl/expected/jsonb_plperlu.out
@@ -255,6 +255,31 @@ INFO:  $VAR1 = {'1' => {'2' => ['3','4','5']},'2' => '3'};
  {"1": {"2": [3, 4, 5]}, "2": 3}
 (1 row)
 
+CREATE FUNCTION text2jsonb (text) RETURNS jsonb
+LANGUAGE plperlu
+TRANSFORM FOR TYPE jsonb
+AS $$
+    my $x = shift;
+    return {a=>$x};
+$$;
+SELECT text2jsonb(NULL);
+ text2jsonb  
+-------------
+ {"a": null}
+(1 row)
+
+SELECT text2jsonb('11');
+ text2jsonb  
+-------------
+ {"a": "11"}
+(1 row)
+
+SELECT text2jsonb(NULL);
+ text2jsonb  
+-------------
+ {"a": null}
+(1 row)
+
 \set VERBOSITY terse \\ -- suppress cascade details
 DROP EXTENSION plperlu CASCADE;
-NOTICE:  drop cascades to 7 other objects
+NOTICE:  drop cascades to 8 other objects
diff --git a/contrib/jsonb_plperl/jsonb_plperl.c b/contrib/jsonb_plperl/jsonb_plperl.c
index 79c5f57..5244ab7 100644
--- a/contrib/jsonb_plperl/jsonb_plperl.c
+++ b/contrib/jsonb_plperl/jsonb_plperl.c
@@ -257,6 +257,12 @@ SV_to_JsonbValue(SV *in, JsonbParseState **jsonb_state, bool is_elem)
 			}
 			else
 			{
+				/* SVt_PV without POK flag is also NULL */
+				if(SvTYPE(in) == SVt_PV) 
+				{
+					out.type = jbvNull;
+					break;
+				}
 				/*
 				 * XXX It might be nice if we could include the Perl type in
 				 * the error message.
diff --git a/contrib/jsonb_plperl/sql/jsonb_plperl.sql b/contrib/jsonb_plperl/sql/jsonb_plperl.sql
index 8b062df..622141b 100644
--- a/contrib/jsonb_plperl/sql/jsonb_plperl.sql
+++ b/contrib/jsonb_plperl/sql/jsonb_plperl.sql
@@ -99,6 +99,17 @@ SELECT roundtrip('{"1": "string1"}', 'HASH');
 
 SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}', 'HASH');
 
+CREATE FUNCTION text2jsonb (text) RETURNS jsonb
+LANGUAGE plperl
+TRANSFORM FOR TYPE jsonb
+AS $$
+    my $x = shift;
+    return {a=>$x};
+$$;
+
+SELECT text2jsonb(NULL);
+SELECT text2jsonb('11');
+SELECT text2jsonb(NULL);
 
 \set VERBOSITY terse \\ -- suppress cascade details
 DROP EXTENSION plperl CASCADE;
diff --git a/contrib/jsonb_plperl/sql/jsonb_plperlu.sql b/contrib/jsonb_plperl/sql/jsonb_plperlu.sql
index 8d8e841..9981c37 100644
--- a/contrib/jsonb_plperl/sql/jsonb_plperlu.sql
+++ b/contrib/jsonb_plperl/sql/jsonb_plperlu.sql
@@ -103,6 +103,17 @@ SELECT roundtrip('{"1": "string1"}', 'HASH');
 
 SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}', 'HASH');
 
+CREATE FUNCTION text2jsonb (text) RETURNS jsonb
+LANGUAGE plperlu
+TRANSFORM FOR TYPE jsonb
+AS $$
+    my $x = shift;
+    return {a=>$x};
+$$;
+
+SELECT text2jsonb(NULL);
+SELECT text2jsonb('11');
+SELECT text2jsonb(NULL);
 
 \set VERBOSITY terse \\ -- suppress cascade details
 DROP EXTENSION plperlu CASCADE;


view thread (2+ messages)  latest in thread

Message-ID: <1564783533.324795401@f193.i.mail.ru>
Permalink:  ../1564783533.324795401@f193.i.mail.ru/
Also on:    postgresql.org/message-id/1564783533.324795401@f193.i.mail.ru

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: wao@mail.ru, pgsql-hackers@lists.postgresql.org
  Subject: Re: jsonb_plperl bug
  In-Reply-To: <1564783533.324795401@f193.i.mail.ru>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox