Peter Eisentraut <peter.eisentr...@2ndquadrant.com> 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
>From e0948ca29055241874ba455d39728da66fdf3fd5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
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
