Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Teodor Sigaev]

Thank you, pushed with Petr's notice

Dmitry Dolgov wrote:


On 6 April 2016 at 03:29, Andrew Dunstan > wrote:


Yeah, keeping it but rejecting update of an existing key is my preference 
too.

cheers

andrew


Yes, it sounds quite reasonable. Here is a new version of patch (it will throw
an error for an existing key). Is it better now?





--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Petr Jelinek]

On 06/04/16 06:13, Dmitry Dolgov wrote:


On 6 April 2016 at 03:29, Andrew Dunstan > wrote:


Yeah, keeping it but rejecting update of an existing key is my
preference too.

cheers

andrew


Yes, it sounds quite reasonable. Here is a new version of patch (it will
throw
an error for an existing key). Is it better now?


This seems like reasonable compromise to me. I wonder if the errcode 
should be ERRCODE_INVALID_PARAMETER_VALUE but don't feel too strongly 
about that.


--
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

On 6 April 2016 at 03:29, Andrew Dunstan  wrote:

>
> Yeah, keeping it but rejecting update of an existing key is my preference
> too.
>
> cheers
>
> andrew
>

Yes, it sounds quite reasonable. Here is a new version of patch (it will
throw
an error for an existing key). Is it better now?
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 1bc9fbc..0d4b3a1 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10903,6 +10903,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11184,6 +11187,39 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, insert_after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.  If
+ target section designated by
+ path is in a JSONB array,
+ new_value will be inserted before target or
+ after if insert_after is true (default is
+ false). If target section
+ designated by path is in JSONB object,
+ new_value will be inserted only if
+ target does not exist. As with the path
+ orientated operators, negative integers that appear in
+ path count from the end of JSON arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
@@ -11235,10 +11271,11 @@ table2-mapping
   
 
   All the items of the path parameter of jsonb_set
-  must be present in the target, unless
-  create_missing is true, in which case all but the last item
-  must be present. If these conditions are not met the target
-  is returned unchanged.
+  as well as jsonb_insert except the last item must be present
+  in the target. If create_missing is false, all
+  items of the path parameter of jsonb_set must be
+  present. If these conditions are not met the target is
+  returned unchanged.
 
 
   If the last path item is an object key, it will be created if it
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 9ae1ef4..a6e661c 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -997,3 +997,11 @@ RETURNS text[]
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'parse_ident';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 97e0e8e..c1b8041 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,15 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+#define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER \
+		| JB_PATH_CREATE)
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +139,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3553,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-  0, newval, create);
+  0, newval, create ? JB_PATH_CREATE : JB_PATH_NOOP);
 
 	Assert(res != NULL);
 
@@ -3588,7 +3597,52 @@ jsonb_delete_path(PG_FUNCTION_ARGS)
 
 	it = JsonbIteratorInit(>root);
 
-	res = setPath(, path_elems, path_nulls, path_len, , 0, NULL, false);
+	res = setPath(, 

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Andrew Dunstan]

On 04/05/2016 03:08 PM, Tom Lane wrote:


I think there is potentially some use-case for insert-only semantics
for an object target, if you want to be sure you're not overwriting
data.  So I think "throw an error on duplicate key" is marginally better
than "reject object target altogether".  But I could live with either.




Yeah, keeping it but rejecting update of an existing key is my 
preference 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

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Tom Lane]

Andrew Dunstan  writes:
> On 04/05/2016 12:42 PM, Teodor Sigaev wrote:
>> I'm agree about covering this case by tests, but I think it should be 
>> allowed.
>> In this case it will work exactly as jsbonb_set

> It seems to me a violation of POLA to allow something called "insert" to 
> do a "replace" instead.

I agree with Andrew's point here.  If the target is an array it would
never replace an entry, so why would it do so for an object?

I think there is potentially some use-case for insert-only semantics
for an object target, if you want to be sure you're not overwriting
data.  So I think "throw an error on duplicate key" is marginally better
than "reject object target altogether".  But I could live with either.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Andrew Dunstan]

On 04/05/2016 12:42 PM, Teodor Sigaev wrote:
I've been asked to look at and comment on the SQL API of the feature. 
I think
it's basically sound, although there is one thing that's not clear 
from the
regression tests: what happens if we're inserting into an object and 
the key

already exists? e.g.:

select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"');

I think this should be forbidden, i.e. the function shouldn't ever 
overwrite an
existing value. If that's not handled it should be, and either way 
there should

be a regression test for it.


I'm agree about covering this case by tests, but I think it should be 
allowed.

In this case it will work exactly as jsbonb_set



It seems to me a violation of POLA to allow something called "insert" to 
do a "replace" instead.


cheers

andre



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Alvaro Herrera]

Andrew Dunstan wrote:

> I haven't been following this thread due to pressure of time, so my
> apologies in advance if these comments have already been covered.
> 
> I've been asked to look at and comment on the SQL API of the feature. I
> think it's basically sound, although there is one thing that's not clear
> from the regression tests: what happens if we're inserting into an object
> and the key already exists? e.g.:
> 
> select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"');

It has been covered: Petr said, and I agree with him, that this new
interface is for arrays, not objects, and so the above example should be
rejected altogether.  For objects we already have jsonb_set(), so what
is the point of having this work there?  It feels too much like
TIMTOWTDI, which isn't a principle we adhere to.

I think it'd be much more sensible if we were just to make this function
work on arrays only.  There, the solution to Andrew's question is
trivial: if you insert a value in a position that's already occupied,
the elements to its right are "shifted" one position upwards in the
array (this is why this is an "insert" operation and not "replace" or
"set").

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Teodor Sigaev]

I've been asked to look at and comment on the SQL API of the feature. I think
it's basically sound, although there is one thing that's not clear from the
regression tests: what happens if we're inserting into an object and the key
already exists? e.g.:

select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"');

I think this should be forbidden, i.e. the function shouldn't ever overwrite an
existing value. If that's not handled it should be, and either way there should
be a regression test for it.


I'm agree about covering this case by tests, but I think it should be allowed.
In this case it will work exactly as jsbonb_set
--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Andrew Dunstan]

On 03/31/2016 09:00 AM, Dmitry Dolgov wrote:
On 31 March 2016 at 17:31, Vitaly Burovoy > wrote:


it is logical to insert new value if "before", then current value,
then new
value if "after".


Oh, I see now. There is a slightly different logic: `v` is a current 
value and `newval` is a new value.
So basically we insert a current item in case of "after", then a new 
value (if it's not a delete operation),
then a current item in case of "before". But I agree, this code can be 
more straightforward. I've attached
a new version, pls take a look (it contains the same logic that you've 
mentioned).




I haven't been following this thread due to pressure of time, so my 
apologies in advance if these comments have already been covered.


I've been asked to look at and comment on the SQL API of the feature. I 
think it's basically sound, although there is one thing that's not clear 
from the regression tests: what happens if we're inserting into an 
object and the key already exists? e.g.:


select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"');

I think this should be forbidden, i.e. the function shouldn't ever 
overwrite an existing value. If that's not handled it should be, and 
either way there should be a regression test for it.


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

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

On 31 March 2016 at 17:31, Vitaly Burovoy  wrote:

> it is logical to insert new value if "before", then current value, then new
> value if "after".
>

Oh, I see now. There is a slightly different logic: `v` is a current value
and `newval` is a new value.
So basically we insert a current item in case of "after", then a new value
(if it's not a delete operation),
then a current item in case of "before". But I agree, this code can be more
straightforward. I've attached
a new version, pls take a look (it contains the same logic that you've
mentioned).
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 1bc9fbc..8fb2624 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10903,6 +10903,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11184,6 +11187,40 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, insert_after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.
+ If target section designated by
+ path is in a JSONB array,
+ new_value will be inserted before target or
+ after if insert_after is true (default is
+ false). If target section
+ designated by path is in JSONB object,
+ jsonb_insert behaves as
+ jsonb_set function where
+ create_missing is true. As with the
+ path orientated operators, negative integers that appear in
+ path count from the end of JSON arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
@@ -11235,10 +11272,11 @@ table2-mapping
   
 
   All the items of the path parameter of jsonb_set
-  must be present in the target, unless
-  create_missing is true, in which case all but the last item
-  must be present. If these conditions are not met the target
-  is returned unchanged.
+  as well as jsonb_insert except the last item must be present
+  in the target. If create_missing is false, all
+  items of the path parameter of jsonb_set must be
+  present. If these conditions are not met the target is
+  returned unchanged.
 
 
   If the last path item is an object key, it will be created if it
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 9ae1ef4..a6e661c 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -997,3 +997,11 @@ RETURNS text[]
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'parse_ident';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 97e0e8e..97bb08e 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,15 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+#define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER \
+		| JB_PATH_CREATE)
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +139,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3553,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-	

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Vitaly Burovoy]

On 3/30/16, Dmitry Dolgov <9erthali...@gmail.com> wrote:
> On 31 March 2016 at 05:04, Vitaly Burovoy  wrote:
>> The documentation changes still has to be fixed.
> Thanks for help. Looks like I'm not so good at text formulation. Fixed.
Never mind. I'm also not so good at it. It is still should be
rewritten by a native English speaker, but it is better to give him
good source for it.

===
I'm almost ready to mark it as "Ready for committer".

Few notes again.
1.
> a current item was pushed to parse state after JB_PATH_INSERT_BEFORE

I paid attention that the function's name 'pushJsonbValue' looks as
working with stack (push/pop), but there is no function "pop*" neither
in jsonfuncs.c nor jsonb_util.c.
That's why I wrote "it seems the logic in the code is correct" - it is
logical to insert new value if "before", then current value, then new
value if "after".
But yes, following by executing path to the "pushState" function
anyone understands "JsonbParseState" is constructed as a stack, not a
queue. So additional comment is needed why "JB_PATH_INSERT_AFTER"
check is placed before inserting curent value and
JB_PATH_INSERT_BEFORE check.

The comment can be located just before "if (op_type &
JB_PATH_INSERT_AFTER)" (line 3986) and look similar to:

/*
 * JsonbParseState struct behaves as a stack -- see the "pushState" function,
 * so check for JB_PATH_INSERT_BEFORE/JB_PATH_INSERT_AFTER in a reverse order.
 */

2.
A comment for the function "setPath" is obsolete: "If newval is null"
check and "If create is true" name do not longer exist.
Since I answered so late last time I propose the next formulating to
avoid one (possible) round:

/*
 * Do most of the heavy work for jsonb_set/jsonb_insert
 *
 * If JB_PATH_DELETE bit is set in op_type, the element is to be removed.
 *
 * If any bit mentioned in JB_PATH_CREATE_OR_INSERT is set in op_type,
 * we create the new value if the key or array index does not exist.
 *
 * Bits JB_PATH_INSERT_BEFORE and JB_PATH_INSERT_AFTER in op_type
 * behave as JB_PATH_CREATE if new value is inserted in JsonbObject.
 *
 * All path elements before the last must already exist
 * whatever bits in op_type are set, or nothing is done.
 */

===
I hope that notes are final (additional check in formulating is appreciated).

P.S.: if it is not hard for you, please rebase the patch to the
current master to avoid offset in func.sgml.

-- 
Best regards,
Vitaly Burovoy


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

On 31 March 2016 at 05:04, Vitaly Burovoy  wrote:

> The documentation changes still has to be fixed.
>

Thanks for help. Looks like I'm not so good at text formulation. Fixed.


> Moreover it seems the logic in the code is correct


No - I see now, that I made the same mistake in two places (e.g. in case of
`setPathArray` a current item was pushed to parse state after
`JB_PATH_INSERT_BEFORE`, not a new one), so they were annihilated. Fixed.
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index ae93e69..3c3ff7a 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10904,6 +10904,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11185,6 +11188,40 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, insert_after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.
+ If target section designated by
+ path is in a JSONB array,
+ new_value will be inserted before target or
+ after if insert_after is true (default is
+ false). If target section
+ designated by path is in JSONB object,
+ jsonb_insert behaves as
+ jsonb_set function where
+ create_missing is true. As with the
+ path orientated operators, negative integers that appear in
+ path count from the end of JSON arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
@@ -11236,10 +11273,11 @@ table2-mapping
   
 
   All the items of the path parameter of jsonb_set
-  must be present in the target, unless
-  create_missing is true, in which case all but the last item
-  must be present. If these conditions are not met the target
-  is returned unchanged.
+  as well as jsonb_insert except the last item must be present
+  in the target. If create_missing is false, all
+  items of the path parameter of jsonb_set must be
+  present. If these conditions are not met the target is
+  returned unchanged.
 
 
   If the last path item is an object key, it will be created if it
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 9ae1ef4..a6e661c 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -997,3 +997,11 @@ RETURNS text[]
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'parse_ident';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 97e0e8e..df9fa5d 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,15 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+#define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER \
+		| JB_PATH_CREATE)
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +139,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3553,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-  0, newval, create);
+  0, newval, create ? JB_PATH_CREATE : JB_PATH_NOOP);
 
 	Assert(res != NULL);

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Vitaly Burovoy]

On 3/25/16, Dmitry Dolgov <9erthali...@gmail.com> wrote:
> Here is a new version of path, I hope I didn't miss anything. Few notes:
>
>> 4.
>> or even create a new constant (there can be better name for it):
>> #define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE |
>> JB_PATH_INSERT_AFTER | JB_PATH_CREATE)
>
> Good idea, thanks.

You're welcome.
===

I'm sorry for the late letter.

Unfortunately, it seems one more round is necessary.
The documentation changes still has to be fixed.

The main description points to a "target section designated by path is
a JSONB array". It is a copy-paste from jsonb_set, but here it has
wrong context.
The main idea in jsonb_set is replacing any object which is pointed
(designated) by "path" no matter where (array or object) it is
located.
But in the phrase for jsonb_insert above you want to point to an upper
object _where_ "section designated by path" is located.

I'd write something like "If target section designated by path is _IN_
a JSONB array, ...". The same thing with "a JSONB object".

Also I'd rewrite "will act like" as "behaves as".

Last time I missed the argument's name "insert_before_after". It is
better to replace it as just "insert_after". Also reflect that name in
the documentation properly.

To be consistent change the constant "JB_PATH_NOOP" as "0x"
instead of just "0x0".

Also the variable's name "bool before" is incorrect because it
contradicts with the SQL function's argument "after" (from the
documentation) or "insert_after" (proposed above) or tests (de-facto
behavior). Moreover it seems the logic in the code is correct, so I
have no idea why "before ? JB_PATH_INSERT_BEFORE :
JB_PATH_INSERT_AFTER" works.
I think either proper comment should be added or lack in the code must be found.
Anyway the variable's name must reflect the SQL argument's name.

-- 
Best regards,
Vitaly Burovoy


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

Here is a new version of path, I hope I didn't miss anything. Few notes:

> 4.
> or even create a new constant (there can be better name for it):
> #define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE |
> JB_PATH_INSERT_AFTER | JB_PATH_CREATE)

Good idea, thanks.

> 5.
> > if (op_type != JB_PATH_DELETE)

Yes, I just missed that in previous patch.

> 7.
> Please, return the "skip" comment.

Well, I'm still not so sure about that, but if you're so confident then ok
=)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index ae93e69..dfed589 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10904,6 +10904,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11185,6 +11188,40 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.
+ If target section designated by
+ path is a JSONB array,
+ new_value will be inserted before target or
+ after if after is true (default is
+ false). If target section
+ designated by path is a JSONB object,
+ jsonb_insert will act like
+ jsonb_set function where
+ create_missing is true. As with the
+ path orientated operators, negative integers that appear in
+ path count from the end of JSON arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
@@ -11236,10 +11273,11 @@ table2-mapping
   
 
   All the items of the path parameter of jsonb_set
-  must be present in the target, unless
-  create_missing is true, in which case all but the last item
-  must be present. If these conditions are not met the target
-  is returned unchanged.
+  as well as jsonb_insert except the last item must be present
+  in the target. If create_missing is false, all
+  items of the path parameter of jsonb_set must be
+  present. If these conditions are not met the target is
+  returned unchanged.
 
 
   If the last path item is an object key, it will be created if it
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 9ae1ef4..58ed3c3 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -997,3 +997,11 @@ RETURNS text[]
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'parse_ident';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_before_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 97e0e8e..5bd2165 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,15 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x0
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+#define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER \
+		| JB_PATH_CREATE)
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +139,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3553,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-  0, newval, create);
+  0, newval, create ? JB_PATH_CREATE : JB_PATH_NOOP);
 
 	Assert(res != NULL);
 
@@ 

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Vitaly Burovoy]

On 2016-03-16, Dmitry Dolgov <9erthali...@gmail.com> wrote:
> Hi Vitaly, thanks for the review. I've attached a new version of path with
> improvements. Few notes:
>
>> 7. Why did you remove "skip"? It is a comment what "true" means...
>
> Actually, I thought that this comment was about skipping an element from
> jsonb in order to change/delete it,

As I got it, it is "skipNested", skipping iterating over nested
containers, not skipping an element.

> not about the last argument.  E.g. you can find several occurrences of
> `JsonbIteratorNext` with `true` as the last
> argument but without a "last argument is about skip" comment.

I think it is not a reason for deleting this comment. ;-)

> And there is a piece of code in the function `jsonb_delete` with a "skip
> element" commentary:
>
> ```
> /* skip corresponding value as well */
> if (r == WJB_KEY)
> JsonbIteratorNext(, , true);
> ```

The comment in your example is for the extra "JsonbIteratorNext" call
(the first one is in a "while" statement outside your example, but
over it in the code), not for the "skip" parameter in the
"JsonbIteratorNext" call here.
===

Notes for the jsonb_insert_v3.patch applied on the f6bd0da:

1a. Please, rebase your patch to the current master (it is easy to
resolve conflict, but still necessary).

1b. Now OID=3318 is "pg_stat_get_progress_info" (Hint: 3324 is free now).

2.
The documentation, func.sgml:
> Iftarget
Here must be a space after the "If" word.

3.
There is a little odd formulating me in the documentation part
(without formatting for better readability):
> If target section designated by path is a JSONB array, new_value will be 
> inserted before it, or after if after is true (defailt is false).

a. "new_value" is not inserted before "it" (a JSONB array), it is
inserted before target;
b. s/or after if/or after target if/;
c. s/defailt/default/

> If ... is a JSONB object, new_value will be added just like a regular key.

d. "new_value" is not a regular key: key is the final part of the "target";
e. "new_value" replaces target if it exists;
f. "new_value" is added if target is not exists as if jsonb_set is
called with create_missing=true.
g. "will" is about possibility. "be" is about an action.

4.
function setPathObject, block with "op_type = JB_PATH_CREATE"
(jsonfuncs.c, lines 3833..3840).
It seems it is not necessary now since you can easily check for all
three options like:
op_type & (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER | JB_PATH_CREATE)

or even create a new constant (there can be better name for it):
#define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE |
JB_PATH_INSERT_AFTER | JB_PATH_CREATE)

and use it like:
op_type & JB_PATH_CREATE_OR_INSERT

all occurrences of JB_PATH_CREATE in the function are already with the
"(level == path_len - 1)" condition, there is no additional check
needed.

also the new constant JB_PATH_CREATE_OR_INSERT can be used at lines 3969, 4025.

5.
> if (op_type != JB_PATH_DELETE)
It seems strange direct comparison among bitwise operators (lines 3987..3994)

You can leave it as is, but I'd change it (and similar one at the line
3868) to a bitwise operator:
if (!op_type & JB_PATH_DELETE)

6.
I'd like to see a test with big negative index as a reverse for big
positive index:
select jsonb_insert('{"a": [0,1,2]}', '{a, -10}', '"new_value"');

I know now it works as expected, it is just as a defense against
further changes that can be destructive for this special case.

7.
Please, return the "skip" comment.

8.
The documentation: add "jsonb_insert" to the note about importance of
existing intermediate keys. Try to reword it since the function
doesn't have a "create_missing" parameter support.
> All the items of the path parameter of jsonb_set must be present in the 
> target,
> ... in which case all but the last item must be present.


Currently I can't break the code, so I think it is close to the final state. ;-)

--
Best regards,
Vitaly Burovoy


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

> I still don't like that this works on path leading to an object given
that we can't fulfill the promise of inserting to an arbitrary position
there.

I'm thinking about this following way - `jsonb_insert` is just a function
to insert a new value, which has specific options to enforce arbitrary
position in case of jsonb array. I don't think this can confuse someone
since it's not something like `jsonb_insert_at_position` function. But of
course, if I'm wrong we can easy change the function name and make it
available only for arrays.

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

Hi Vitaly, thanks for the review. I've attached a new version of path with
improvements. Few notes:

> 7. Why did you remove "skip"? It is a comment what "true" means...

Actually, I thought that this comment was about skipping an element from
jsonb in order to change/delete it,
not about the last argument.  E.g. you can find several occurrences of
`JsonbIteratorNext` with `true` as the last
argument but without a "last argument is about skip" comment.
And there is a piece of code in the function `jsonb_delete` with a "skip
element" commentary:

```
/* skip corresponding value as well */
if (r == WJB_KEY)
JsonbIteratorNext(, , true);
```

So since in this patch it's not a simple skipping for setPathArray, I
removed that commentary. Am I wrong?

> 9. And finally... it does not work as expected in case of:

Yes, good catch, thanks.
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c0b94bc..158e7fb 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10791,6 +10791,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11072,6 +11075,39 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.
+ Iftarget section designated by
+ path is a JSONB array,
+ new_value will be inserted before it, or
+ after if after is true (defailt is
+ false).  If target section
+ designated by path is a JSONB object,
+ new_value will be added just like a regular
+ key.  As with the path orientated operators, negative integers that
+ appear in path count from the end of JSON
+ arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index abf9a70..b1281e7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -971,3 +971,11 @@ RETURNS jsonb
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'jsonb_set';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_before_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 88225aa..1c1da7c 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,13 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x0
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +137,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3551,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-  0, newval, create);
+  0, newval, create ? JB_PATH_CREATE : JB_PATH_NOOP);
 
 	Assert(res != NULL);
 
@@ -3588,7 +3595,52 @@ jsonb_delete_path(PG_FUNCTION_ARGS)
 
 	it = JsonbIteratorInit(>root);
 
-	res = setPath(, path_elems, path_nulls, path_len, , 0, NULL, false);
+	res = setPath(, path_elems, path_nulls, path_len, ,
+  0, NULL, JB_PATH_DELETE);
+
+	Assert(res != NULL);
+
+	PG_RETURN_JSONB(JsonbValueToJsonb(res));
+}
+
+/*
+ * SQL function jsonb_insert(jsonb, text[], jsonb, boolean)
+ *
+ */
+Datum
+jsonb_insert(PG_FUNCTION_ARGS)
+{
+	

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Vitaly Burovoy]

On 2016-02-29 17:19+00, Dmitry Dolgov <9erthali...@gmail.com> wrote:
On 2016-02-24 19:37+00, Petr Jelinek  wrote:
>> Also this patch needs documentation.
> I've added new version in attachments, thanks.

Hello! The first pass of a review is below.

1. Rename the "flag" variable to something more meaningful. (e.g.
"op_type" - "operation_type")


2. There is two extra spaces after the "_DELETE" word
+#define JB_PATH_DELETE 0x0002


3.
res = setPath(, path_elems, path_nulls, path_len, ,
- 0, newval, create);
+ 0, newval, create ? JB_PATH_CREATE : 0x0);

What is the magic constant "0x0"? If not "create", then what?
Introduce something like JB_PATH_NOOP = 0x0...


4. In the "setPathArray" the block:
if (newval != NULL)
"newval == NULL" is considered as "to be deleted" from the previous
convention and from the comments for the "setPath" function.
I think since you've introduced special constants one of which is
JB_PATH_DELETE (which is not used now) you should replace convention
from checking for a value to checking for a constant:
if (flag != JB_PATH_DELETE)
or even better:
if (!flag & JB_PATH_DELETE)


5. Change checking for equality (to bit constants) to bit operations:
(flag == JB_PATH_INSERT_BEFORE || flag == JB_PATH_INSERT_AFTER)
which can be replaced to:
(flag & (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER))

also here:
(flag == JB_PATH_CREATE || flag == JB_PATH_INSERT_BEFORE || flag
== JB_PATH_INSERT_AFTER))
can be:
(flag & (JB_PATH_CREATE | JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER))


6. Pay attention to parenthesises to make the code looks like similar
one around:
+   if ((npairs == 0) && flag == JB_PATH_CREATE && (level == path_len - 1))
should be:
+   if ((npairs == 0) && (flag == JB_PATH_CREATE) && (level == path_len - 
1))


7. Why did you remove "skip"? It is a comment what "true" means...
-   r = JsonbIteratorNext(it, , true);/* skip 
*/
+   r = JsonbIteratorNext(it, , true);


8. After your changes some statements exceed 80-column threshold...
The same rules for the documentation.


9. And finally... it does not work as expected in case of:
postgres=# select jsonb_insert('{"a":[0,1,2,3]}', '{"a", 10}', '"4"');
jsonb_insert
-
 {"a": [0, 1, 2, 3]}
(1 row)

wheras jsonb_set works:
postgres=# select jsonb_set('{"a":[0,1,2,3]}', '{"a", 10}', '"4"');
jsonb_set
--
 {"a": [0, 1, 2, 3, "4"]}
(1 row)

-- 
Best regards,
Vitaly Burovoy


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Petr Jelinek]

On 29/02/16 18:19, Dmitry Dolgov wrote:

 > I'd strongly prefer the jsonb_array_insert naming though
 > I don't think it's a good idea to use set when this is used on
object, I think that we should throw error in that case.

Well, I thought it's wrong to introduce different functions and
behaviour patterns for objects and arrays, because it's the one data
type after all. But it's just my opinion of course.



The problem I see with that logic is because we don't keep order of keys 
in the jsonb object the "insert" in the name will have misleading 
implications from the point of the user. Also it does not do anything 
for object that "set" doesn't do already, so why have two interfaces for 
doing same thing.


--
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

> I'd strongly prefer the jsonb_array_insert naming though
> I don't think it's a good idea to use set when this is used on object, I
think that we should throw error in that case.

Well, I thought it's wrong to introduce different functions and behaviour
patterns for objects and arrays, because it's the one data type after all.
But it's just my opinion of course.

> Also this patch needs documentation.

I've added new version in attachments, thanks.


jsonb_insert_v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Petr Jelinek]

On 18/02/16 15:38, Dmitry Dolgov wrote:

Hi

As far as I see there is one basic update function for jsonb, that can't be
covered by `jsonb_set` - insert a new value into an array at arbitrary
position.
Using `jsonb_set` function we can only append into array at the end/at the
beginning, and it looks more like a hack:

```
=# select jsonb_set('{"a": [1, 2, 3]}', '{a, 100}', '4');
   jsonb_set
-
  {"a": [1, 2, 3, 4]}
(1 row)


=# select jsonb_set('{"a": [1, 2, 3]}', '{a, -100}', '4');
   jsonb_set
-
  {"a": [4, 1, 2, 3]}
(1 row)
```

I think the possibility to insert into arbitrary position will be quite
useful,
something like `json_array_insert` in MySql:

```
mysql> set @j = '["a", {"b": [1, 2]}, [3, 4]]';
mysql> select json_array_insert(@j, '$[1].b[0]', 'x');

  json_array_insert(@j, '$[1].b[0]', 'x')
+-+
  ["a", {"b": ["x", 1, 2]}, [3, 4]]
```

It can look like `jsonb_insert` function in our case:

```
=# select jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"');
  jsonb_insert
---
  {"a": [0, "new_value", 1, 2]}
(1 row)
```



I think it makes sense to have interface like this, I'd strongly prefer 
the jsonb_array_insert naming though.




I attached possible implementation, which is basically quite small (all
logic-related
modifications is only about 4 lines in `setPath` function). This
implementation
assumes a flag to separate "insert before"/"insert after" operations, and an
alias to `jsonb_set` in case if we working with a jsonb object, not an
array.



I don't think it's a good idea to use set when this is used on object, I 
think that we should throw error in that case.


Also this patch needs documentation.

--
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

Hi

As far as I see there is one basic update function for jsonb, that can't be
covered by `jsonb_set` - insert a new value into an array at arbitrary
position.
Using `jsonb_set` function we can only append into array at the end/at the
beginning, and it looks more like a hack:

```
=# select jsonb_set('{"a": [1, 2, 3]}', '{a, 100}', '4');
  jsonb_set
-
 {"a": [1, 2, 3, 4]}
(1 row)


=# select jsonb_set('{"a": [1, 2, 3]}', '{a, -100}', '4');
  jsonb_set
-
 {"a": [4, 1, 2, 3]}
(1 row)
```

I think the possibility to insert into arbitrary position will be quite
useful,
something like `json_array_insert` in MySql:

```
mysql> set @j = '["a", {"b": [1, 2]}, [3, 4]]';
mysql> select json_array_insert(@j, '$[1].b[0]', 'x');

 json_array_insert(@j, '$[1].b[0]', 'x')
+-+
 ["a", {"b": ["x", 1, 2]}, [3, 4]]
```

It can look like `jsonb_insert` function in our case:

```
=# select jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"');
 jsonb_insert
---
 {"a": [0, "new_value", 1, 2]}
(1 row)
```

I attached possible implementation, which is basically quite small (all
logic-related
modifications is only about 4 lines in `setPath` function). This
implementation
assumes a flag to separate "insert before"/"insert after" operations, and an
alias to `jsonb_set` in case if we working with a jsonb object, not an
array.

What do you think about this?


jsonb_insert.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers