From 1731e3741cbbf8e0b4481665d7d523bc55117f63 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 18 Jun 2018 14:31:42 -0400 Subject: [PATCH] Fix excessive enreferencing in jsonb-to-plperl transform. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit We want, say, 2 to be transformed as 2, not \\2 which is what the original coding produced. Perl's standard seems to be to add an RV wrapper only for hash and array SVs, so do it like that. This was missed originally because the test cases only checked what came out of a round trip back to SQL, and the strip-all-dereferences loop at the top of SV_to_JsonbValue hides the extra refs from view. As a better test, print the Perl value with Data::Dumper, like the hstore_plperlu tests do. While we can't do that in the plperl test, only plperlu, that should be good enough because this code is the same for both PLs. But also add a simplistic test for extra REFs, which we can do in both. That strip-all-dereferences behavior is now a bit dubious; it's unlike what happens for other Perl-to-SQL conversions. However, the best thing to do seems to be to leave it alone and make the other conversions act similarly. That will be done separately. Dagfinn Ilmari Mannsåker, adjusted a bit by me Discussion: https://postgr.es/m/d8jlgbq66t9.fsf@dalvik.ping.uio.no --- .../jsonb_plperl/expected/jsonb_plperl.out | 49 +++++++----- .../jsonb_plperl/expected/jsonb_plperlu.out | 78 ++++++++++++++----- contrib/jsonb_plperl/jsonb_plperl.c | 10 +-- contrib/jsonb_plperl/sql/jsonb_plperl.sql | 42 +++++----- contrib/jsonb_plperl/sql/jsonb_plperlu.sql | 45 ++++++----- 5 files changed, 141 insertions(+), 83 deletions(-) diff --git a/contrib/jsonb_plperl/expected/jsonb_plperl.out b/contrib/jsonb_plperl/expected/jsonb_plperl.out index e4f3cdd41a..d2d90eb483 100644 --- a/contrib/jsonb_plperl/expected/jsonb_plperl.out +++ b/contrib/jsonb_plperl/expected/jsonb_plperl.out @@ -52,16 +52,19 @@ 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 $$ +# can't use Data::Dumper, but let's at least check for unexpected ref type +die 'unexpected '.(ref($_[0]) || 'not a').' reference' + if ref($_[0]) ne $_[1]; return $_[0]; $$; -SELECT roundtrip('null'); - roundtrip ------------ - null +SELECT roundtrip('null') is null; + ?column? +---------- + t (1 row) SELECT roundtrip('1'); @@ -115,91 +118,97 @@ 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('[["string1", "string2"]]', 'ARRAY'); + roundtrip +-------------------------- + [["string1", "string2"]] +(1 row) + +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..3f5c5c681d 100644 --- a/contrib/jsonb_plperl/expected/jsonb_plperlu.out +++ b/contrib/jsonb_plperl/expected/jsonb_plperlu.out @@ -52,154 +52,192 @@ 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 $$ +use Data::Dumper; +$Data::Dumper::Sortkeys = 1; +$Data::Dumper::Indent = 0; +elog(INFO, Dumper($_[0])); +die 'unexpected '.(ref($_[0]) || 'not a').' reference' + if ref($_[0]) ne $_[1]; return $_[0]; $$; -SELECT roundtrip('null'); - roundtrip ------------ - null +SELECT roundtrip('null') is null; +INFO: $VAR1 = undef; + ?column? +---------- + t (1 row) SELECT roundtrip('1'); +INFO: $VAR1 = '1'; roundtrip ----------- 1 (1 row) SELECT roundtrip('1E+131071'); +INFO: $VAR1 = 'inf'; ERROR: cannot convert infinity to jsonb CONTEXT: PL/Perl function "roundtrip" SELECT roundtrip('-1'); +INFO: $VAR1 = '-1'; roundtrip ----------- -1 (1 row) SELECT roundtrip('1.2'); +INFO: $VAR1 = '1.2'; roundtrip ----------- 1.2 (1 row) SELECT roundtrip('-1.2'); +INFO: $VAR1 = '-1.2'; roundtrip ----------- -1.2 (1 row) SELECT roundtrip('"string"'); +INFO: $VAR1 = 'string'; roundtrip ----------- "string" (1 row) SELECT roundtrip('"NaN"'); +INFO: $VAR1 = 'NaN'; roundtrip ----------- "NaN" (1 row) SELECT roundtrip('true'); +INFO: $VAR1 = '1'; roundtrip ----------- 1 (1 row) SELECT roundtrip('false'); +INFO: $VAR1 = '0'; roundtrip ----------- 0 (1 row) -SELECT roundtrip('[]'); +SELECT roundtrip('[]', 'ARRAY'); +INFO: $VAR1 = []; roundtrip ----------- [] (1 row) -SELECT roundtrip('[null, null]'); +SELECT roundtrip('[null, null]', 'ARRAY'); +INFO: $VAR1 = [undef,undef]; roundtrip -------------- [null, null] (1 row) -SELECT roundtrip('[1, 2, 3]'); +SELECT roundtrip('[1, 2, 3]', 'ARRAY'); +INFO: $VAR1 = ['1','2','3']; roundtrip ----------- [1, 2, 3] (1 row) -SELECT roundtrip('[-1, 2, -3]'); +SELECT roundtrip('[-1, 2, -3]', 'ARRAY'); +INFO: $VAR1 = ['-1','2','-3']; roundtrip ------------- [-1, 2, -3] (1 row) -SELECT roundtrip('[1.2, 2.3, 3.4]'); +SELECT roundtrip('[1.2, 2.3, 3.4]', 'ARRAY'); +INFO: $VAR1 = ['1.2','2.3','3.4']; 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'); +INFO: $VAR1 = ['-1.2','2.3','-3.4']; roundtrip ------------------- [-1.2, 2.3, -3.4] (1 row) -SELECT roundtrip('["string1", "string2"]'); +SELECT roundtrip('["string1", "string2"]', 'ARRAY'); +INFO: $VAR1 = ['string1','string2']; roundtrip ------------------------ ["string1", "string2"] (1 row) -SELECT roundtrip('{}'); +SELECT roundtrip('[["string1", "string2"]]', 'ARRAY'); +INFO: $VAR1 = [['string1','string2']]; + roundtrip +-------------------------- + [["string1", "string2"]] +(1 row) + +SELECT roundtrip('{}', 'HASH'); +INFO: $VAR1 = {}; roundtrip ----------- {} (1 row) -SELECT roundtrip('{"1": null}'); +SELECT roundtrip('{"1": null}', 'HASH'); +INFO: $VAR1 = {'1' => undef}; roundtrip ------------- {"1": null} (1 row) -SELECT roundtrip('{"1": 1}'); +SELECT roundtrip('{"1": 1}', 'HASH'); +INFO: $VAR1 = {'1' => '1'}; roundtrip ----------- {"1": 1} (1 row) -SELECT roundtrip('{"1": -1}'); +SELECT roundtrip('{"1": -1}', 'HASH'); +INFO: $VAR1 = {'1' => '-1'}; roundtrip ----------- {"1": -1} (1 row) -SELECT roundtrip('{"1": 1.1}'); +SELECT roundtrip('{"1": 1.1}', 'HASH'); +INFO: $VAR1 = {'1' => '1.1'}; roundtrip ------------ {"1": 1.1} (1 row) -SELECT roundtrip('{"1": -1.1}'); +SELECT roundtrip('{"1": -1.1}', 'HASH'); +INFO: $VAR1 = {'1' => '-1.1'}; roundtrip ------------- {"1": -1.1} (1 row) -SELECT roundtrip('{"1": "string1"}'); +SELECT roundtrip('{"1": "string1"}', 'HASH'); +INFO: $VAR1 = {'1' => 'string1'}; roundtrip ------------------ {"1": "string1"} (1 row) -SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}'); +SELECT roundtrip('{"1": {"2": [3, 4, 5]}, "2": 3}', 'HASH'); +INFO: $VAR1 = {'1' => {'2' => ['3','4','5']},'2' => '3'}; 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..1b63fc4b30 100644 --- a/contrib/jsonb_plperl/jsonb_plperl.c +++ b/contrib/jsonb_plperl/jsonb_plperl.c @@ -26,7 +26,7 @@ JsonbValue_to_SV(JsonbValue *jbv) switch (jbv->type) { case jbvBinary: - return newRV(Jsonb_to_SV(jbv->val.binary.data)); + return Jsonb_to_SV(jbv->val.binary.data); case jbvNumeric: { @@ -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..642a71deb7 100644 --- a/contrib/jsonb_plperl/sql/jsonb_plperl.sql +++ b/contrib/jsonb_plperl/sql/jsonb_plperl.sql @@ -45,15 +45,18 @@ $$; 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 $$ +# can't use Data::Dumper, but let's at least check for unexpected ref type +die 'unexpected '.(ref($_[0]) || 'not a').' reference' + if ref($_[0]) ne $_[1]; return $_[0]; $$; -SELECT roundtrip('null'); +SELECT roundtrip('null') is null; SELECT roundtrip('1'); SELECT roundtrip('1E+131071'); SELECT roundtrip('-1'); @@ -65,23 +68,24 @@ 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('[["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..49c71fbc44 100644 --- a/contrib/jsonb_plperl/sql/jsonb_plperlu.sql +++ b/contrib/jsonb_plperl/sql/jsonb_plperlu.sql @@ -45,15 +45,21 @@ $$; 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 $$ +use Data::Dumper; +$Data::Dumper::Sortkeys = 1; +$Data::Dumper::Indent = 0; +elog(INFO, Dumper($_[0])); +die 'unexpected '.(ref($_[0]) || 'not a').' reference' + if ref($_[0]) ne $_[1]; return $_[0]; $$; -SELECT roundtrip('null'); +SELECT roundtrip('null') is null; SELECT roundtrip('1'); SELECT roundtrip('1E+131071'); SELECT roundtrip('-1'); @@ -65,23 +71,24 @@ 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('[["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.40.0