Fix broken jsonb_set() logic for replacing array elements.

Commit 0b62fd036 did a fairly sloppy job of refactoring setPath()
to support jsonb_insert() along with jsonb_set().  In its defense,
though, there was no regression test case exercising the case of
replacing an existing element in a jsonb array.

Per bug #14366 from Peng Sun.  Back-patch to 9.6 where bug was introduced.

Report: <20161012065349.1412.47858@wrigleys.postgresql.org>

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 996007d4837d93aaae23d7b2970822c5800d4546..059d570de7a69fced3f04c1f27349013092060d1 100644 (file)
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -34,11 +34,11 @@
 #include "utils/typcache.h"
 
 /* Operations available for setPath */
-#define JB_PATH_NOOP                                   0x0000
 #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_REPLACE                                        0x0004
+#define JB_PATH_INSERT_BEFORE                  0x0008
+#define JB_PATH_INSERT_AFTER                   0x0010
 #define JB_PATH_CREATE_OR_INSERT \
        (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER | JB_PATH_CREATE)
 
@@ -3545,7 +3545,7 @@ jsonb_set(PG_FUNCTION_ARGS)
        it = JsonbIteratorInit(&in->root);
 
        res = setPath(&it, path_elems, path_nulls, path_len, &st,
-                                 0, newval, create ? JB_PATH_CREATE : JB_PATH_NOOP);
+                                 0, newval, create ? JB_PATH_CREATE : JB_PATH_REPLACE);
 
        Assert(res != NULL);
 
@@ -4003,7 +4003,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
                                if (op_type & (JB_PATH_INSERT_AFTER | JB_PATH_INSERT_BEFORE))
                                        (void) pushJsonbValue(st, r, &v);
 
-                               if (op_type & JB_PATH_INSERT_AFTER)
+                               if (op_type & (JB_PATH_INSERT_AFTER | JB_PATH_REPLACE))
                                        addJsonbToParseState(st, newval);
 
                                done = true;
@@ -4035,7 +4035,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
                                }
                        }
 
-                       if (op_type & JB_PATH_CREATE_OR_INSERT && !done &&
+                       if ((op_type & JB_PATH_CREATE_OR_INSERT) && !done &&
                                level == path_len - 1 && i == nelems - 1)
                        {
                                addJsonbToParseState(st, newval);

diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index a6d25defb0d82e3e151675e7a6ccaa72b630a210..e2cb08a6fba244f8afad8419af5bb6ae272803eb 100644 (file)
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -3238,6 +3238,12 @@ select jsonb_set('[]','{1}','"b"', false);
  []
 (1 row)
 
+select jsonb_set('[{"f1":1,"f2":null},2,null,3]', '{0}','[2,3,4]', false);
+        jsonb_set        
+-------------------------
+ [[2, 3, 4], 2, null, 3]
+(1 row)
+
 -- jsonb_set adding instead of replacing
 -- prepend to array
 select jsonb_set('{"a":1,"b":[0,1,2],"c":{"d":4}}','{b,-33}','{"foo":123}');

diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index b84bd70a2992904f1520d897e5d7ae2ab8030d75..6b4c796992259a1324d71e13fce8fb7813f1b299 100644 (file)
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -814,6 +814,7 @@ select '[]'::jsonb #- '{a}';
 select jsonb_set('"a"','{a}','"b"'); --error
 select jsonb_set('{}','{a}','"b"', false);
 select jsonb_set('[]','{1}','"b"', false);
+select jsonb_set('[{"f1":1,"f2":null},2,null,3]', '{0}','[2,3,4]', false);
 
 -- jsonb_set adding instead of replacing