> Attached is an implantation of jsonb_delete that instead of taking a
single key to remove accepts an array of keys

Since I already saw this patch, here is my small review.

Speaking about implementation of `jsonb_delete_array` - it's fine, but I
would like to suggest two modifications:

* create a separate helper function for jsonb delete operation, to use it
in both `jsonb_delete` and `jsonb_delete_array`. It will help to
concentrate related logic in one place.

* use variadic arguments for `jsonb_delete_array`. For rare cases, when
someone decides to use this function directly instead of corresponding
operator. It will be more consistent with `jsonb_delete` from my point of
view, because it's transition from `jsonb_delete(data, 'key')` to
`jsonb_delete(data, 'key1', 'key2')` is more smooth, than to
`jsonb_delete(data, '{key1, key2}')`.

I've attached a patch with these modifications. What do you think?
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 17ee4e4..aa8156e 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -149,6 +149,11 @@ static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
+/* common workers for jsonb_delete functions */
+static Datum jsonb_delete_worker(Jsonb *in, Datum *elems,
+								 bool *nulls, int len);
+
+
 /* state for json_object_keys */
 typedef struct OkeysState
 {
@@ -3395,14 +3400,8 @@ jsonb_delete(PG_FUNCTION_ARGS)
 {
 	Jsonb	   *in = PG_GETARG_JSONB(0);
 	text	   *key = PG_GETARG_TEXT_PP(1);
-	char	   *keyptr = VARDATA_ANY(key);
-	int			keylen = VARSIZE_ANY_EXHDR(key);
-	JsonbParseState *state = NULL;
-	JsonbIterator *it;
-	JsonbValue	v,
-			   *res = NULL;
-	bool		skipNested = false;
-	JsonbIteratorToken r;
+	Datum	   *keys = (Datum *) palloc(sizeof(Datum));
+	bool	   *nulls = (bool *) palloc(sizeof(bool));
 
 	if (JB_ROOT_IS_SCALAR(in))
 		ereport(ERROR,
@@ -3412,21 +3411,95 @@ jsonb_delete(PG_FUNCTION_ARGS)
 	if (JB_ROOT_COUNT(in) == 0)
 		PG_RETURN_JSONB(in);
 
+	keys[0] = PointerGetDatum(key);
+	nulls[0] = FALSE;
+
+	return jsonb_delete_worker(in, keys, nulls, 1);
+}
+
+/*
+ * SQL function jsonb_delete (jsonb, text[])
+ *
+ * return a copy of the jsonb with the indicated items
+ * removed.
+ */
+Datum
+jsonb_delete_array(PG_FUNCTION_ARGS)
+{
+	Jsonb	   *in = PG_GETARG_JSONB(0);
+	ArrayType  *keys = PG_GETARG_ARRAYTYPE_P(1);
+	Datum	   *keys_elems;
+	bool	   *keys_nulls;
+	int			keys_len;
+
+	if (ARR_NDIM(keys) > 1)
+		ereport(ERROR,
+				(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
+				 errmsg("wrong number of array subscripts")));
+
+	if (JB_ROOT_IS_SCALAR(in))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("cannot delete from scalar")));
+
+	if (JB_ROOT_COUNT(in) == 0)
+		PG_RETURN_JSONB(in);
+
+	deconstruct_array(keys, TEXTOID, -1, false, 'i',
+					  &keys_elems, &keys_nulls, &keys_len);
+
+	return jsonb_delete_worker(in, keys_elems, keys_nulls, keys_len);
+}
+
+Datum
+jsonb_delete_worker(Jsonb *in, Datum *elems, bool *nulls, int len)
+{
+	JsonbParseState *state = NULL;
+	JsonbIterator *it;
+	JsonbValue	v,
+			   *res = NULL;
+	bool		skipNested = false;
+	JsonbIteratorToken r;
+
+	if (len == 0)
+		PG_RETURN_JSONB(in);
+
 	it = JsonbIteratorInit(&in->root);
 
 	while ((r = JsonbIteratorNext(&it, &v, skipNested)) != 0)
 	{
 		skipNested = true;
 
-		if ((r == WJB_ELEM || r == WJB_KEY) &&
-			(v.type == jbvString && keylen == v.val.string.len &&
-			 memcmp(keyptr, v.val.string.val, keylen) == 0))
+		if ((r == WJB_ELEM || r == WJB_KEY) && v.type == jbvString)
 		{
-			/* skip corresponding value as well */
-			if (r == WJB_KEY)
-				JsonbIteratorNext(&it, &v, true);
+			int			i;
+			bool		found = false;
 
-			continue;
+			for (i = 0; i < len; i++)
+			{
+				char	   *keyptr;
+				int			keylen;
+
+				if (nulls[i])
+					continue;
+
+				keyptr = VARDATA_ANY(elems[i]);
+				keylen = VARSIZE_ANY_EXHDR(elems[i]);
+				if (keylen == v.val.string.len &&
+					memcmp(keyptr, v.val.string.val, keylen) == 0)
+				{
+					found = true;
+					break;
+				}
+			}
+			if (found)
+			{
+				/* skip corresponding value as well */
+				if (r == WJB_KEY)
+					JsonbIteratorNext(&it, &v, true);
+
+				continue;
+			}
 		}
 
 		res = pushJsonbValue(&state, r, r < WJB_BEGIN_ARRAY ? &v : NULL);
diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index 26fa618..347b2e1 100644
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -1820,6 +1820,8 @@ DATA(insert OID = 3284 (  "||"	   PGNSP PGUID b f f 3802 3802 3802 0 0 jsonb_con
 DESCR("concatenate");
 DATA(insert OID = 3285 (  "-"	   PGNSP PGUID b f f 3802 25 3802 0 0 3302 - - ));
 DESCR("delete object field");
+DATA(insert OID = 3344 (  "-"      PGNSP PGUID b f f 3802 1009 3802 0 0 3343 - -));
+DESCR("delete object fields");
 DATA(insert OID = 3286 (  "-"	   PGNSP PGUID b f f 3802 23 3802 0 0 3303 - - ));
 DESCR("delete array element");
 DATA(insert OID = 3287 (  "#-"	   PGNSP PGUID b f f 3802 1009 3802 0 0 jsonb_delete_path - - ));
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 047a1ce..535ce6e 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4897,6 +4897,7 @@ DESCR("GIN support");
 DATA(insert OID = 3301 (  jsonb_concat	   PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 3802 "3802 3802" _null_ _null_ _null_ _null_ _null_ jsonb_concat _null_ _null_ _null_ ));
 DATA(insert OID = 3302 (  jsonb_delete	   PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 3802 "3802 25" _null_ _null_ _null_ _null_ _null_ jsonb_delete _null_ _null_ _null_ ));
 DATA(insert OID = 3303 (  jsonb_delete	   PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 3802 "3802 23" _null_ _null_ _null_ _null_ _null_ jsonb_delete_idx _null_ _null_ _null_ ));
+DATA(insert OID = 3343 ( jsonb_delete      PGNSP PGUID 12 1 0 25 0 f f f f t f i s 2 0 3802 "3802 1009" "{3802,1009}" "{i,v}" "{from_json,path_elems}" _null_ _null_ jsonb_delete_array _null_ _null_ _null_ ));
 DATA(insert OID = 3304 (  jsonb_delete_path    PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 3802 "3802 1009" _null_ _null_ _null_ _null_ _null_ jsonb_delete_path _null_ _null_ _null_ ));
 DATA(insert OID = 3305 (  jsonb_set    PGNSP PGUID 12 1 0 0 0 f f f f t f i s 4 0 3802 "3802 1009 3802 16" _null_ _null_ _null_ _null_ _null_ jsonb_set _null_ _null_ _null_ ));
 DESCR("Set part of a jsonb");
diff --git a/src/include/utils/jsonb.h b/src/include/utils/jsonb.h
index 470d5b1..d6af415 100644
--- a/src/include/utils/jsonb.h
+++ b/src/include/utils/jsonb.h
@@ -404,6 +404,7 @@ extern Datum jsonb_concat(PG_FUNCTION_ARGS);
 
 /* deletion */
 extern Datum jsonb_delete(PG_FUNCTION_ARGS);
+extern Datum jsonb_delete_array(PG_FUNCTION_ARGS);
 extern Datum jsonb_delete_idx(PG_FUNCTION_ARGS);
 extern Datum jsonb_delete_path(PG_FUNCTION_ARGS);
 
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index e2cb08a..ba9b1d7 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -3095,6 +3095,24 @@ select '["a","b","c"]'::jsonb - -4;
  ["a", "b", "c"]
 (1 row)
 
+select '{"a":1 , "b":2, "c":3}'::jsonb - '{b}'::text[];
+     ?column?     
+------------------
+ {"a": 1, "c": 3}
+(1 row)
+
+select '{"a":1 , "b":2, "c":3}'::jsonb - '{c,b}'::text[];
+ ?column? 
+----------
+ {"a": 1}
+(1 row)
+
+select '{"a":1 , "b":2, "c":3}'::jsonb - '{}'::text[];
+         ?column?         
+--------------------------
+ {"a": 1, "b": 2, "c": 3}
+(1 row)
+
 select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb, '{n}', '[1,2,3]');
                                 jsonb_set                                 
 --------------------------------------------------------------------------
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 6b4c796..eb65a38 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -777,6 +777,10 @@ select '["a","b","c"]'::jsonb - -2;
 select '["a","b","c"]'::jsonb - -3;
 select '["a","b","c"]'::jsonb - -4;
 
+select '{"a":1 , "b":2, "c":3}'::jsonb - '{b}'::text[];
+select '{"a":1 , "b":2, "c":3}'::jsonb - '{c,b}'::text[];
+select '{"a":1 , "b":2, "c":3}'::jsonb - '{}'::text[];
+
 select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb, '{n}', '[1,2,3]');
 select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb, '{b,-1}', '[1,2,3]');
 select jsonb_set('{"n":null, "a":1, "b":[1,2], "c":{"1":2}, "d":{"1":[2,3]}}'::jsonb, '{d,1,0}', '[1,2,3]');
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to