pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: ilmari@ilmari.org
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Transform for pl/perl
Date: Thu, 07 Jun 2018 14:54:10 +0100
Message-ID: <d8jlgbq66t9.fsf@dalvik.ping.uio.no> (raw )
In-Reply-To: <e5130c85-23da-5d11-ca74-6a050cc5fa85@2ndquadrant.com >
References: <20180606161453.tvp4ltavlcvejmmz@alvherre.pgsql >
<e5130c85-23da-5d11-ca74-6a050cc5fa85@2ndquadrant.com >
Peter Eisentraut <peter.eisentraut@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
Attachments:
[text/x-diff] 0001-Fix-excess-enreferencing-in-plperl-jsonb-transform.patch (11.3K, ../d8jlgbq66t9.fsf@dalvik.ping.uio.no/2-0001-Fix-excess-enreferencing-in-plperl-jsonb-transform.patch)
download | inline diff:
From e0948ca29055241874ba455d39728da66fdf3fd5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@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
view thread (62+ messages) latest in thread
Message-ID: <d8jlgbq66t9.fsf@dalvik.ping.uio.no>
Permalink: ../d8jlgbq66t9.fsf@dalvik.ping.uio.no/
Also on: postgresql.org/message-id/d8jlgbq66t9.fsf@dalvik.ping.uio.no
copy link · copy postgr.es · copy git am
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: ilmari@ilmari.org, peter.eisentraut@2ndquadrant.com, alvherre@2ndquadrant.com, tgl@sss.pgh.pa.us, pavel.stehule@gmail.com, n.gluhov@postgrespro.ru, pgsql-hackers@lists.postgresql.org
Subject: Re: Transform for pl/perl
In-Reply-To: <d8jlgbq66t9.fsf@dalvik.ping.uio.no>
* 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