Received: from malur.postgresql.org ([2a02:16a8:dc51::56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fQvMx-0003CF-11 for pgsql-hackers@arkaria.postgresql.org; Thu, 07 Jun 2018 13:54:27 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fQvMv-00044x-MI for pgsql-hackers@arkaria.postgresql.org; Thu, 07 Jun 2018 13:54:25 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fQvMv-00044o-DZ for pgsql-hackers@lists.postgresql.org; Thu, 07 Jun 2018 13:54:25 +0000 Received: from ping.uio.no ([2001:700:100:570::196]) by magus.postgresql.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fQvMs-0000Fs-8q for pgsql-hackers@lists.postgresql.org; Thu, 07 Jun 2018 13:54:24 +0000 Received: from [2001:700:100:570::211] (helo=dalvik.ping.uio.no ident=Debian-exim) by ping.uio.no with esmtp (Exim 4.72 #1 (Debian)) id 1fQvMh-0006lI-1Z; Thu, 07 Jun 2018 15:54:15 +0200 Received: from ilmari by dalvik.ping.uio.no with local (Exim 4.84_2) (envelope-from ) id 1fQvMg-0003PG-Rs; Thu, 07 Jun 2018 15:54:10 +0200 From: ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) To: Peter Eisentraut Cc: Alvaro Herrera , Tom Lane , Pavel Stehule , Nikita Glukhov , PostgreSQL Hackers Subject: Re: Transform for pl/perl References: <20180606161453.tvp4ltavlcvejmmz@alvherre.pgsql> Date: Thu, 07 Jun 2018 14:54:10 +0100 In-Reply-To: (Peter Eisentraut's message of "Wed, 6 Jun 2018 15:46:52 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Exiscan-Spam-Score: -3.7 (---) X-Exiscan-Spam-Report: SpamAssassin 3.3.1 (2010-03-16) on pike.ping.uio.no Score Rule * -5.0 PING_UIO_MAIL_IS_INTERNAL Message has never been outside * 129.240.0.0/16 or 193.157.115.0/24 * 1.3 RDNS_NONE Delivered to internal network by a host with no rDNS List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --=-=-= Content-Type: text/plain Peter Eisentraut writes: > On 6/6/18 12:14, Alvaro Herrera wrote: >> On 2018-May-17, Peter Eisentraut wrote: >> >>> The items that are still open from the original email are: >>> >>> 2) jsonb scalar values are passed to the plperl function wrapped in not >>> one, but _two_ layers of references >>> >>> 3) jsonb numeric values are passed as perl's NV (floating point) type, >>> losing precision if they're integers that would fit in an IV or UV. >>> >>> #2 appears to be a quality of implementation issue without any >>> user-visible effects. >>> >>> #3 is an opportunity for future improvement, but works as intended right >>> now. >>> >>> I think patches for these issues could still be considered during beta, >>> but they are not release blockers IMO. >> >> It appears to me that item #2 definitely would need to be fixed before >> release, so that it doesn't become a backwards-incompatibility later on. > > The way I understand it, it's only how things are passed around > internally. Nothing is noticeable externally, and so there is no > backward compatibility issue. > > At least that's how I understand it. So far this is only a claim by one > person. I haven't seen anything conclusive about whether there is an > actual issue. It's not just how things are passed internally, but how they are passed to pl/perl functions with a jsonb transform: JSON scalar values at the top level (strings, numbers, booleans, null) get passed as a reference to a reference to the value, e.g. \\42 instead of 42. The reason the current tests don't pick this up is that they don't check the value inside the pl/perl functions, only that they roundtrip back to jsonb, and the plperl to jsonb transform recursively dereferences references. Another side effect of the recursive dereferencing is that returning undef from the perl function returns an SQL NULL while returning a reference to undef (\undef) returns a jsonb null. The attached patch fixes the excess enreferencing, but does not touch the dereferencing part. - ilmari -- "I use RMS as a guide in the same way that a boat captain would use a lighthouse. It's good to know where it is, but you generally don't want to find yourself in the same spot." - Tollef Fog Heen --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Fix-excess-enreferencing-in-plperl-jsonb-transform.patch From e0948ca29055241874ba455d39728da66fdf3fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= Date: Tue, 17 Apr 2018 18:18:31 +0100 Subject: [PATCH] Fix excess enreferencing in plperl jsonb transform --- .../jsonb_plperl/expected/jsonb_plperl.out | 42 +++++++++---------- .../jsonb_plperl/expected/jsonb_plperlu.out | 36 ++++++++-------- contrib/jsonb_plperl/jsonb_plperl.c | 8 ++-- contrib/jsonb_plperl/sql/jsonb_plperl.sql | 39 ++++++++--------- contrib/jsonb_plperl/sql/jsonb_plperlu.sql | 38 +++++++++-------- 5 files changed, 82 insertions(+), 81 deletions(-) diff --git a/contrib/jsonb_plperl/expected/jsonb_plperl.out b/contrib/jsonb_plperl/expected/jsonb_plperl.out index e4f3cdd41a..3364057a64 100644 --- a/contrib/jsonb_plperl/expected/jsonb_plperl.out +++ b/contrib/jsonb_plperl/expected/jsonb_plperl.out @@ -52,16 +52,18 @@ SELECT testRegexpResultToJsonb(); 0 (1 row) -CREATE FUNCTION roundtrip(val jsonb) RETURNS jsonb +CREATE FUNCTION roundtrip(val jsonb, ref text = '') RETURNS jsonb LANGUAGE plperl TRANSFORM FOR TYPE jsonb AS $$ +die 'unexpected '.(ref($_[0]) || 'not a').' reference' + if ref($_[0]) ne $_[1]; return $_[0]; $$; SELECT roundtrip('null'); roundtrip ----------- - null + (1 row) SELECT roundtrip('1'); @@ -97,12 +99,6 @@ SELECT roundtrip('"string"'); "string" (1 row) -SELECT roundtrip('"NaN"'); - roundtrip ------------ - "NaN" -(1 row) - SELECT roundtrip('true'); roundtrip ----------- @@ -115,91 +111,91 @@ SELECT roundtrip('false'); 0 (1 row) -SELECT roundtrip('[]'); +SELECT roundtrip('[]', 'ARRAY'); roundtrip ----------- [] (1 row) -SELECT roundtrip('[null, null]'); +SELECT roundtrip('[null, null]', 'ARRAY'); roundtrip -------------- [null, null] (1 row) -SELECT roundtrip('[1, 2, 3]'); +SELECT roundtrip('[1, 2, 3]', 'ARRAY'); roundtrip ----------- [1, 2, 3] (1 row) -SELECT roundtrip('[-1, 2, -3]'); +SELECT roundtrip('[-1, 2, -3]', 'ARRAY'); roundtrip ------------- [-1, 2, -3] (1 row) -SELECT roundtrip('[1.2, 2.3, 3.4]'); +SELECT roundtrip('[1.2, 2.3, 3.4]', 'ARRAY'); roundtrip ----------------- [1.2, 2.3, 3.4] (1 row) -SELECT roundtrip('[-1.2, 2.3, -3.4]'); +SELECT roundtrip('[-1.2, 2.3, -3.4]', 'ARRAY'); roundtrip ------------------- [-1.2, 2.3, -3.4] (1 row) -SELECT roundtrip('["string1", "string2"]'); +SELECT roundtrip('["string1", "string2"]', 'ARRAY'); roundtrip ------------------------ ["string1", "string2"] (1 row) -SELECT roundtrip('{}'); +SELECT roundtrip('{}', 'HASH'); roundtrip ----------- {} (1 row) -SELECT roundtrip('{"1": null}'); +SELECT roundtrip('{"1": null}', 'HASH'); roundtrip ------------- {"1": null} (1 row) -SELECT roundtrip('{"1": 1}'); +SELECT roundtrip('{"1": 1}', 'HASH'); roundtrip ----------- {"1": 1} (1 row) -SELECT roundtrip('{"1": -1}'); +SELECT roundtrip('{"1": -1}', 'HASH'); roundtrip ----------- {"1": -1} (1 row) -SELECT roundtrip('{"1": 1.1}'); +SELECT roundtrip('{"1": 1.1}', 'HASH'); roundtrip ------------ {"1": 1.1} (1 row) -SELECT roundtrip('{"1": -1.1}'); +SELECT roundtrip('{"1": -1.1}', 'HASH'); roundtrip ------------- {"1": -1.1} (1 row) -SELECT roundtrip('{"1": "string1"}'); +SELECT roundtrip('{"1": "string1"}', 'HASH'); roundtrip ------------------ {"1": "string1"} (1 row) -SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}'); +SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}', 'HASH'); roundtrip --------------------------------- {"1": {"2": [3, 4, 5]}, "2": 3} diff --git a/contrib/jsonb_plperl/expected/jsonb_plperlu.out b/contrib/jsonb_plperl/expected/jsonb_plperlu.out index b44995058f..c222e74e2d 100644 --- a/contrib/jsonb_plperl/expected/jsonb_plperlu.out +++ b/contrib/jsonb_plperl/expected/jsonb_plperlu.out @@ -52,16 +52,18 @@ SELECT testRegexpResultToJsonb(); 0 (1 row) -CREATE FUNCTION roundtrip(val jsonb) RETURNS jsonb +CREATE FUNCTION roundtrip(val jsonb, ref text = '') RETURNS jsonb LANGUAGE plperlu TRANSFORM FOR TYPE jsonb AS $$ +die 'unexpected '.(ref($_[0]) || 'not a').' reference' + if ref($_[0]) ne $_[1]; return $_[0]; $$; SELECT roundtrip('null'); roundtrip ----------- - null + (1 row) SELECT roundtrip('1'); @@ -115,91 +117,91 @@ SELECT roundtrip('false'); 0 (1 row) -SELECT roundtrip('[]'); +SELECT roundtrip('[]', 'ARRAY'); roundtrip ----------- [] (1 row) -SELECT roundtrip('[null, null]'); +SELECT roundtrip('[null, null]', 'ARRAY'); roundtrip -------------- [null, null] (1 row) -SELECT roundtrip('[1, 2, 3]'); +SELECT roundtrip('[1, 2, 3]', 'ARRAY'); roundtrip ----------- [1, 2, 3] (1 row) -SELECT roundtrip('[-1, 2, -3]'); +SELECT roundtrip('[-1, 2, -3]', 'ARRAY'); roundtrip ------------- [-1, 2, -3] (1 row) -SELECT roundtrip('[1.2, 2.3, 3.4]'); +SELECT roundtrip('[1.2, 2.3, 3.4]', 'ARRAY'); roundtrip ----------------- [1.2, 2.3, 3.4] (1 row) -SELECT roundtrip('[-1.2, 2.3, -3.4]'); +SELECT roundtrip('[-1.2, 2.3, -3.4]', 'ARRAY'); roundtrip ------------------- [-1.2, 2.3, -3.4] (1 row) -SELECT roundtrip('["string1", "string2"]'); +SELECT roundtrip('["string1", "string2"]', 'ARRAY'); roundtrip ------------------------ ["string1", "string2"] (1 row) -SELECT roundtrip('{}'); +SELECT roundtrip('{}', 'HASH'); roundtrip ----------- {} (1 row) -SELECT roundtrip('{"1": null}'); +SELECT roundtrip('{"1": null}', 'HASH'); roundtrip ------------- {"1": null} (1 row) -SELECT roundtrip('{"1": 1}'); +SELECT roundtrip('{"1": 1}', 'HASH'); roundtrip ----------- {"1": 1} (1 row) -SELECT roundtrip('{"1": -1}'); +SELECT roundtrip('{"1": -1}', 'HASH'); roundtrip ----------- {"1": -1} (1 row) -SELECT roundtrip('{"1": 1.1}'); +SELECT roundtrip('{"1": 1.1}', 'HASH'); roundtrip ------------ {"1": 1.1} (1 row) -SELECT roundtrip('{"1": -1.1}'); +SELECT roundtrip('{"1": -1.1}', 'HASH'); roundtrip ------------- {"1": -1.1} (1 row) -SELECT roundtrip('{"1": "string1"}'); +SELECT roundtrip('{"1": "string1"}', 'HASH'); roundtrip ------------------ {"1": "string1"} (1 row) -SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}'); +SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}', 'HASH'); roundtrip --------------------------------- {"1": {"2": [3, 4, 5]}, "2": 3} diff --git a/contrib/jsonb_plperl/jsonb_plperl.c b/contrib/jsonb_plperl/jsonb_plperl.c index bde93a71fc..85d5584e07 100644 --- a/contrib/jsonb_plperl/jsonb_plperl.c +++ b/contrib/jsonb_plperl/jsonb_plperl.c @@ -83,7 +83,7 @@ Jsonb_to_SV(JsonbContainer *jsonb) (r = JsonbIteratorNext(&it, &tmp, true)) != WJB_DONE) elog(ERROR, "unexpected jsonb token: %d", r); - return newRV(JsonbValue_to_SV(&v)); + return JsonbValue_to_SV(&v); } else { @@ -95,7 +95,7 @@ Jsonb_to_SV(JsonbContainer *jsonb) av_push(av, JsonbValue_to_SV(&v)); } - return (SV *) av; + return newRV((SV *) av); } case WJB_BEGIN_OBJECT: @@ -120,7 +120,7 @@ Jsonb_to_SV(JsonbContainer *jsonb) } } - return (SV *) hv; + return newRV((SV *) hv); } default: @@ -268,7 +268,7 @@ jsonb_to_plperl(PG_FUNCTION_ARGS) Jsonb *in = PG_GETARG_JSONB_P(0); SV *sv = Jsonb_to_SV(&in->root); - return PointerGetDatum(newRV(sv)); + return PointerGetDatum(sv); } diff --git a/contrib/jsonb_plperl/sql/jsonb_plperl.sql b/contrib/jsonb_plperl/sql/jsonb_plperl.sql index 8b0a8764af..e49a997cba 100644 --- a/contrib/jsonb_plperl/sql/jsonb_plperl.sql +++ b/contrib/jsonb_plperl/sql/jsonb_plperl.sql @@ -45,10 +45,12 @@ $$; SELECT testRegexpResultToJsonb(); -CREATE FUNCTION roundtrip(val jsonb) RETURNS jsonb +CREATE FUNCTION roundtrip(val jsonb, ref text = '') RETURNS jsonb LANGUAGE plperl TRANSFORM FOR TYPE jsonb AS $$ +die 'unexpected '.(ref($_[0]) || 'not a').' reference' + if ref($_[0]) ne $_[1]; return $_[0]; $$; @@ -60,28 +62,27 @@ SELECT roundtrip('-1'); SELECT roundtrip('1.2'); SELECT roundtrip('-1.2'); SELECT roundtrip('"string"'); -SELECT roundtrip('"NaN"'); SELECT roundtrip('true'); SELECT roundtrip('false'); -SELECT roundtrip('[]'); -SELECT roundtrip('[null, null]'); -SELECT roundtrip('[1, 2, 3]'); -SELECT roundtrip('[-1, 2, -3]'); -SELECT roundtrip('[1.2, 2.3, 3.4]'); -SELECT roundtrip('[-1.2, 2.3, -3.4]'); -SELECT roundtrip('["string1", "string2"]'); - -SELECT roundtrip('{}'); -SELECT roundtrip('{"1": null}'); -SELECT roundtrip('{"1": 1}'); -SELECT roundtrip('{"1": -1}'); -SELECT roundtrip('{"1": 1.1}'); -SELECT roundtrip('{"1": -1.1}'); -SELECT roundtrip('{"1": "string1"}'); - -SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}'); +SELECT roundtrip('[]', 'ARRAY'); +SELECT roundtrip('[null, null]', 'ARRAY'); +SELECT roundtrip('[1, 2, 3]', 'ARRAY'); +SELECT roundtrip('[-1, 2, -3]', 'ARRAY'); +SELECT roundtrip('[1.2, 2.3, 3.4]', 'ARRAY'); +SELECT roundtrip('[-1.2, 2.3, -3.4]', 'ARRAY'); +SELECT roundtrip('["string1", "string2"]', 'ARRAY'); + +SELECT roundtrip('{}', 'HASH'); +SELECT roundtrip('{"1": null}', 'HASH'); +SELECT roundtrip('{"1": 1}', 'HASH'); +SELECT roundtrip('{"1": -1}', 'HASH'); +SELECT roundtrip('{"1": 1.1}', 'HASH'); +SELECT roundtrip('{"1": -1.1}', 'HASH'); +SELECT roundtrip('{"1": "string1"}', 'HASH'); + +SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}', 'HASH'); \set VERBOSITY terse \\ -- suppress cascade details diff --git a/contrib/jsonb_plperl/sql/jsonb_plperlu.sql b/contrib/jsonb_plperl/sql/jsonb_plperlu.sql index 9287f7672f..0c19e9066a 100644 --- a/contrib/jsonb_plperl/sql/jsonb_plperlu.sql +++ b/contrib/jsonb_plperl/sql/jsonb_plperlu.sql @@ -45,10 +45,12 @@ $$; SELECT testRegexpResultToJsonb(); -CREATE FUNCTION roundtrip(val jsonb) RETURNS jsonb +CREATE FUNCTION roundtrip(val jsonb, ref text = '') RETURNS jsonb LANGUAGE plperlu TRANSFORM FOR TYPE jsonb AS $$ +die 'unexpected '.(ref($_[0]) || 'not a').' reference' + if ref($_[0]) ne $_[1]; return $_[0]; $$; @@ -65,23 +67,23 @@ SELECT roundtrip('"NaN"'); SELECT roundtrip('true'); SELECT roundtrip('false'); -SELECT roundtrip('[]'); -SELECT roundtrip('[null, null]'); -SELECT roundtrip('[1, 2, 3]'); -SELECT roundtrip('[-1, 2, -3]'); -SELECT roundtrip('[1.2, 2.3, 3.4]'); -SELECT roundtrip('[-1.2, 2.3, -3.4]'); -SELECT roundtrip('["string1", "string2"]'); - -SELECT roundtrip('{}'); -SELECT roundtrip('{"1": null}'); -SELECT roundtrip('{"1": 1}'); -SELECT roundtrip('{"1": -1}'); -SELECT roundtrip('{"1": 1.1}'); -SELECT roundtrip('{"1": -1.1}'); -SELECT roundtrip('{"1": "string1"}'); - -SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}'); +SELECT roundtrip('[]', 'ARRAY'); +SELECT roundtrip('[null, null]', 'ARRAY'); +SELECT roundtrip('[1, 2, 3]', 'ARRAY'); +SELECT roundtrip('[-1, 2, -3]', 'ARRAY'); +SELECT roundtrip('[1.2, 2.3, 3.4]', 'ARRAY'); +SELECT roundtrip('[-1.2, 2.3, -3.4]', 'ARRAY'); +SELECT roundtrip('["string1", "string2"]', 'ARRAY'); + +SELECT roundtrip('{}', 'HASH'); +SELECT roundtrip('{"1": null}', 'HASH'); +SELECT roundtrip('{"1": 1}', 'HASH'); +SELECT roundtrip('{"1": -1}', 'HASH'); +SELECT roundtrip('{"1": 1.1}', 'HASH'); +SELECT roundtrip('{"1": -1.1}', 'HASH'); +SELECT roundtrip('{"1": "string1"}', 'HASH'); + +SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}', 'HASH'); \set VERBOSITY terse \\ -- suppress cascade details -- 2.17.1 --=-=-=--