jsonb_set array append hack?
Hi,
I've noticed that if you use a string for an element key in jsonb_set with
create_missing set to true, you can use it to append to an array:
postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
'{vehicle_types,nonsense}',
'"motorcycle"', true);
jsonb_set
----------------------------------------------------------------
{"name": "Joe", "vehicle_types": ["car", "van", "motorcycle"]}
(1 row)
What this really should match is a nested element inside "vehicle_types"
called "nonsense". But this seems to be a hack to get an element added to
an array. To do it properly currently requires specifying an arbitrary
number in the hope that it will exceed the number of elements you have in
the array.
e.g.
postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
'{vehicle_types,100000}',
'"motorcycle"', true);
jsonb_set
----------------------------------------------------------------
{"name": "Joe", "vehicle_types": ["car", "van", "motorcycle"]}
(1 row)
But I'm guessing people shouldn't be relying on the hack in the first
example. Isn't this a bug? If so, wouldn't this also be a bug?:
postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
array['vehicle_types',NULL],
'"motorcycle"', true);
Thom
On 09/14/2015 01:29 PM, Thom Brown wrote:
Hi,
I've noticed that if you use a string for an element key in jsonb_set
with create_missing set to true, you can use it to append to an array:postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
'{vehicle_types,nonsense}',
'"motorcycle"', true);
jsonb_set
----------------------------------------------------------------
{"name": "Joe", "vehicle_types": ["car", "van", "motorcycle"]}
(1 row)What this really should match is a nested element inside
"vehicle_types" called "nonsense". But this seems to be a hack to get
an element added to an array. To do it properly currently requires
specifying an arbitrary number in the hope that it will exceed the
number of elements you have in the array.
That's a bug and we should fix it.
e.g.
postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
'{vehicle_types,100000}',
'"motorcycle"', true);
jsonb_set
----------------------------------------------------------------
{"name": "Joe", "vehicle_types": ["car", "van", "motorcycle"]}
(1 row)But I'm guessing people shouldn't be relying on the hack in the first
example. Isn't this a bug? If so, wouldn't this also be a bug?:postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
array['vehicle_types',NULL],
'"motorcycle"', true);
I think that's a bug too.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I'm sorry, but I'm not sure, what behavior is expected in this case?
Right now the following logic was implemented:
"we trying to set an element inside an array, but we've got a
non-integer path item
("nonsense" in this particular case), so we're going to add a new
element at the end of array by default"
If it's wrong, should we refuse to perform such kind of operations, or
should we replace
"vehicle_type": ["car", "van"]
to
"vehicle_type: {"nonsense": "motorcycle"}
?
On 15 September 2015 at 01:59, Andrew Dunstan <andrew@dunslane.net> wrote:
Show quoted text
On 09/14/2015 01:29 PM, Thom Brown wrote:
Hi,
I've noticed that if you use a string for an element key in jsonb_set
with create_missing set to true, you can use it to append to an array:postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
'{vehicle_types,nonsense}',
'"motorcycle"', true);
jsonb_set
----------------------------------------------------------------
{"name": "Joe", "vehicle_types": ["car", "van", "motorcycle"]}
(1 row)What this really should match is a nested element inside "vehicle_types"
called "nonsense". But this seems to be a hack to get an element added to
an array. To do it properly currently requires specifying an arbitrary
number in the hope that it will exceed the number of elements you have in
the array.That's a bug and we should fix it.
e.g.
postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
'{vehicle_types,100000}',
'"motorcycle"', true);
jsonb_set
----------------------------------------------------------------
{"name": "Joe", "vehicle_types": ["car", "van", "motorcycle"]}
(1 row)But I'm guessing people shouldn't be relying on the hack in the first
example. Isn't this a bug? If so, wouldn't this also be a bug?:postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
array['vehicle_types',NULL],
'"motorcycle"', true);I think that's a bug too.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 20 September 2015 at 16:17, Dmitry Dolgov <9erthalion6@gmail.com> wrote:
I'm sorry, but I'm not sure, what behavior is expected in this case?
Right now the following logic was implemented:
"we trying to set an element inside an array, but we've got a
non-integer path item
("nonsense" in this particular case), so we're going to add a new
element at the end of array by default"If it's wrong, should we refuse to perform such kind of operations, or
should we replace
"vehicle_type": ["car", "van"]
to
"vehicle_type: {"nonsense": "motorcycle"}
?
(please bottom-post)
I would expect some kind of error. We're trying to address a position in
an array, and we're instead passing a key. If it completes successfully,
the chances are it isn't what the user intended.
Thom
I would expect some kind of error. We're trying to address a position in
an array, and we're instead passing a key. If it completes successfully,
the chances are it isn't what the user intended.
Thanks for the explanation. So, basically, it should be like this, am I
right?
postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car", "van"]}'::jsonb,
'{vehicle_types, nonsense}',
'"motorcycle"', true);
ERROR: path element at the position 2 is not an integer
On 20 September 2015 at 23:50, Thom Brown <thom@linux.com> wrote:
Show quoted text
On 20 September 2015 at 16:17, Dmitry Dolgov <9erthalion6@gmail.com>
wrote:I'm sorry, but I'm not sure, what behavior is expected in this case?
Right now the following logic was implemented:
"we trying to set an element inside an array, but we've got a
non-integer path item
("nonsense" in this particular case), so we're going to add a new
element at the end of array by default"If it's wrong, should we refuse to perform such kind of operations, or
should we replace
"vehicle_type": ["car", "van"]
to
"vehicle_type: {"nonsense": "motorcycle"}
?(please bottom-post)
I would expect some kind of error. We're trying to address a position in
an array, and we're instead passing a key. If it completes successfully,
the chances are it isn't what the user intended.Thom
Attachments:
non_integer_in_path.patchapplication/octet-stream; name=non_integer_in_path.patchDownload
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 154a883..698024a 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3875,7 +3875,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
lindex = strtol(c, &badp, 10);
if (errno != 0 || badp == c || *badp != '\0' || lindex > INT_MAX ||
lindex < INT_MIN)
- idx = nelems;
+ elog(ERROR, "path element at the position %d is not an integer", level + 1);
else
idx = lindex;
}
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 82d1b69..63f6840 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -3199,11 +3199,7 @@ select '{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb #- '{
(1 row)
select '{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb #- '{b,-1e}'; -- invalid array subscript
- ?column?
----------------------------------------------------------------------
- {"a": 1, "b": [1, 2], "c": {"1": 2}, "d": {"1": [2, 3]}, "n": null}
-(1 row)
-
+ERROR: path element at the position 2 is not an integer
select '{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb #- '{d,1,0}';
?column?
------------------------------------------------------------------
@@ -3331,3 +3327,7 @@ select jsonb_set('[]','{-99}','{"foo":123}');
[{"foo": 123}]
(1 row)
+select jsonb_set('{"a": [1, 2, 3]}', '{a, non_integer}', '"new_value"');
+ERROR: path element at the position 2 is not an integer
+select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, non_integer}', '"new_value"');
+ERROR: path element at the position 3 is not an integer
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index cb03ada..2d4d666 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -816,3 +816,5 @@ select jsonb_set('{}','{x}','{"foo":123}');
select jsonb_set('[]','{0}','{"foo":123}');
select jsonb_set('[]','{99}','{"foo":123}');
select jsonb_set('[]','{-99}','{"foo":123}');
+select jsonb_set('{"a": [1, 2, 3]}', '{a, non_integer}', '"new_value"');
+select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, non_integer}', '"new_value"');
On 09/21/2015 12:13 PM, Dmitry Dolgov wrote:
I would expect some kind of error. We're trying to address a
position in an array, and we're instead passing a key. If it
completes successfully, the chances are it isn't what the user intended.Thanks for the explanation. So, basically, it should be like this, am
I right?postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car", "van"]}'::jsonb,
'{vehicle_types, nonsense}',
'"motorcycle"', true);
ERROR: path element at the position 2 is not an integer
That seems reasonable. For that matter, we should probably disallow NULL
path elements also, shouldn't we?
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 21 September 2015 at 22:21, Andrew Dunstan <andrew@dunslane.net> wrote:
On 09/21/2015 12:13 PM, Dmitry Dolgov wrote:
I would expect some kind of error. We're trying to address a position
in an array, and we're instead passing a key. If it completes
successfully, the chances are it isn't what the user intended.Thanks for the explanation. So, basically, it should be like this, am I
right?postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car", "van"]}'::jsonb,
'{vehicle_types, nonsense}',
'"motorcycle"', true);
ERROR: path element at the position 2 is not an integerThat seems reasonable. For that matter, we should probably disallow NULL
path elements also, shouldn't we?
I'd say yes. If someone really wants to name a field "null", they'll just
have to quote it in the path. (e.g. '{contact,"null"}')
--
Thom
For that matter, we should probably disallow NULL path elements also,
shouldn't we?
I'd say yes.
Well, here is the new `setPath` function with this modification. Is it what
did you mean?
Attachments:
non_integer_in_path2.patchapplication/octet-stream; name=non_integer_in_path2.patchDownload
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 154a883..01b6bb0 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3724,6 +3724,9 @@ setPath(JsonbIterator **it, Datum *path_elems,
JsonbValue *res = NULL;
int r;
+ if (path_nulls[level])
+ elog(ERROR, "path element at the position %d is NULL", level + 1);
+
r = JsonbIteratorNext(it, &v, false);
switch (r)
@@ -3875,7 +3878,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
lindex = strtol(c, &badp, 10);
if (errno != 0 || badp == c || *badp != '\0' || lindex > INT_MAX ||
lindex < INT_MIN)
- idx = nelems;
+ elog(ERROR, "path element at the position %d is not an integer", level + 1);
else
idx = lindex;
}
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 82d1b69..6da5a15 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -3127,11 +3127,7 @@ select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::j
(1 row)
select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb, '{d,NULL,0}', '[1,2,3]');
- jsonb_set
----------------------------------------------------------------------
- {"a": 1, "b": [1, 2], "c": {"1": 2}, "d": {"1": [2, 3]}, "n": null}
-(1 row)
-
+ERROR: path element at the position 2 is NULL
select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb, '{n}', '{"1": 2}');
jsonb_set
-------------------------------------------------------------------------
@@ -3151,11 +3147,7 @@ select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::j
(1 row)
select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb, '{d,NULL,0}', '{"1": 2}');
- jsonb_set
----------------------------------------------------------------------
- {"a": 1, "b": [1, 2], "c": {"1": 2}, "d": {"1": [2, 3]}, "n": null}
-(1 row)
-
+ERROR: path element at the position 2 is NULL
select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb, '{b,-1}', '"test"');
jsonb_set
--------------------------------------------------------------------------
@@ -3199,11 +3191,7 @@ select '{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb #- '{
(1 row)
select '{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb #- '{b,-1e}'; -- invalid array subscript
- ?column?
----------------------------------------------------------------------
- {"a": 1, "b": [1, 2], "c": {"1": 2}, "d": {"1": [2, 3]}, "n": null}
-(1 row)
-
+ERROR: path element at the position 2 is not an integer
select '{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb #- '{d,1,0}';
?column?
------------------------------------------------------------------
@@ -3331,3 +3319,9 @@ select jsonb_set('[]','{-99}','{"foo":123}');
[{"foo": 123}]
(1 row)
+select jsonb_set('{"a": [1, 2, 3]}', '{a, non_integer}', '"new_value"');
+ERROR: path element at the position 2 is not an integer
+select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, non_integer}', '"new_value"');
+ERROR: path element at the position 3 is not an integer
+select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, NULL}', '"new_value"');
+ERROR: path element at the position 3 is NULL
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index cb03ada..b1a0764 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -816,3 +816,6 @@ select jsonb_set('{}','{x}','{"foo":123}');
select jsonb_set('[]','{0}','{"foo":123}');
select jsonb_set('[]','{99}','{"foo":123}');
select jsonb_set('[]','{-99}','{"foo":123}');
+select jsonb_set('{"a": [1, 2, 3]}', '{a, non_integer}', '"new_value"');
+select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, non_integer}', '"new_value"');
+select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, NULL}', '"new_value"');
On Mon, Sep 21, 2015 at 2:21 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
Thanks for the explanation. So, basically, it should be like this, am I
right?postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car", "van"]}'::jsonb,
'{vehicle_types, nonsense}',
'"motorcycle"', true);
ERROR: path element at the position 2 is not an integerThat seems reasonable. For that matter, we should probably disallow NULL
path elements also, shouldn't we?
Are you planning on getting this in by Monday, Andrew? It would be
nice to have this fixed going into beta.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 10/03/2015 04:49 PM, Peter Geoghegan wrote:
On Mon, Sep 21, 2015 at 2:21 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
Thanks for the explanation. So, basically, it should be like this, am I
right?postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car", "van"]}'::jsonb,
'{vehicle_types, nonsense}',
'"motorcycle"', true);
ERROR: path element at the position 2 is not an integerThat seems reasonable. For that matter, we should probably disallow NULL
path elements also, shouldn't we?Are you planning on getting this in by Monday, Andrew? It would be
nice to have this fixed going into beta.
Yeah, will look at it tonight or tomorrow.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers