jsonb array-style subscripting

Started by Dmitry Dolgovover 10 years ago22 messages
#1Dmitry Dolgov
9erthalion6@gmail.com
1 attachment(s)

Hi,

Some time ago the array-style subscripting for the jsonb data type was
discussed in this mailing list. I think it will be quite convenient to have
a such nice syntax to update jsonb objects, so I'm trying to implement
this. I created a patch, that allows doing something like this:

=# create TEMP TABLE test_jsonb_subscript (
id int,
test_json jsonb
);

=# insert into test_jsonb_subscript values
(1, '{}'),
(2, '{}');

=# update test_jsonb_subscript set test_json['a']['a1']['a2'] = 42;
=# select * from test_jsonb_subscript;
id | test_json
----+--------------------------
1 | {"a": {"a1": {"a2": 42}}}
2 | {"a": {"a1": {"a2": 42}}}
(2 rows)

=# select test_json['a']['a1'] from test_jsonb_subscript;
test_json
------------
{"a2": 42}
{"a2": 42}
(2 rows)

This patch has a status "work in progress" of course. Generally speaking,
this implementation extends the `ArrayRef` usage for the jsonb.
And I need some sort of advice about several questions:

* is it interesting for the community?
* is that a good idea to extend the `ArrayRef` for jsonb? If it's
appropriate, probably we can rename it to `ArrayJsonbRef` of something.
* what can be improved in the code at the top level (function placement,
probably, functionality duplication, etc.)?
* are there any special cases, that I should take care of in this
implementation?

Attachments:

jsonb_subscript2.patchapplication/octet-stream; name=jsonb_subscript2.patchDownload
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 29f058c..eca1684 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -57,6 +57,7 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 #include "utils/xml.h"
+#include "utils/jsonb.h"
 
 
 /* static function decls */
@@ -272,6 +273,7 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 	IntArray	upper,
 				lower;
 	int		   *lIndex;
+	text	   **path;
 
 	array_source = ExecEvalExpr(astate->refexpr,
 								econtext,
@@ -290,6 +292,11 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 			return (Datum) NULL;
 	}
 
+	if (arrayRef->refarraytype == JSONBOID)
+	{
+		path = (text **) palloc(astate->refupperindexpr->length * sizeof(text*));
+	}
+
 	foreach(l, astate->refupperindexpr)
 	{
 		ExprState  *eltstate = (ExprState *) lfirst(l);
@@ -300,10 +307,21 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 					 errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
 							i + 1, MAXDIM)));
 
-		upper.indx[i++] = DatumGetInt32(ExecEvalExpr(eltstate,
-													 econtext,
-													 &eisnull,
-													 NULL));
+		if (arrayRef->refarraytype == JSONBOID)
+		{
+			path[i++] = cstring_to_text((char *)DatumGetPointer(ExecEvalExpr(eltstate,
+						econtext,
+						&eisnull,
+						NULL)));
+		}
+		else
+		{
+			upper.indx[i++] = DatumGetInt32(ExecEvalExpr(eltstate,
+						econtext,
+						&eisnull,
+						NULL));
+		}
+
 		/* If any index expr yields NULL, result is NULL or error */
 		if (eisnull)
 		{
@@ -445,14 +463,24 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 		}
 
 		if (lIndex == NULL)
-			return array_set_element(array_source, i,
-									 upper.indx,
-									 sourceData,
-									 eisnull,
-									 astate->refattrlength,
-									 astate->refelemlength,
-									 astate->refelembyval,
-									 astate->refelemalign);
+		{
+			if (arrayRef->refarraytype == JSONBOID && path != NULL)
+			{
+				return jsonb_set_element(array_source, path, i, sourceData,
+						((const ArrayRef *) astate->xprstate.expr)->refelemtype);
+			}
+			else
+			{
+				return array_set_element(array_source, i,
+						upper.indx,
+						sourceData,
+						eisnull,
+						astate->refattrlength,
+						astate->refelemlength,
+						astate->refelembyval,
+						astate->refelemalign);
+			}
+		}
 		else
 			return array_set_slice(array_source, i,
 								   upper.indx, lower.indx,
@@ -465,13 +493,22 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 	}
 
 	if (lIndex == NULL)
-		return array_get_element(array_source, i,
-								 upper.indx,
-								 astate->refattrlength,
-								 astate->refelemlength,
-								 astate->refelembyval,
-								 astate->refelemalign,
-								 isNull);
+	{
+		if (arrayRef->refarraytype == JSONBOID && path != NULL)
+		{
+			return jsonb_get_element(array_source, path, i, isNull);
+		}
+		else
+		{
+			return array_get_element(array_source, i,
+					upper.indx,
+					astate->refattrlength,
+					astate->refelemlength,
+					astate->refelembyval,
+					astate->refelemalign,
+					isNull);
+		}
+	}
 	else
 		return array_get_slice(array_source, i,
 							   upper.indx, lower.indx,
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index fa77ef1..a08eaab 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -470,13 +470,29 @@ transformIndirection(ParseState *pstate, Node *basenode, List *indirection)
 	}
 	/* process trailing subscripts, if any */
 	if (subscripts)
-		result = (Node *) transformArraySubscripts(pstate,
-												   result,
-												   exprType(result),
-												   InvalidOid,
-												   exprTypmod(result),
-												   subscripts,
-												   NULL);
+	{
+		if (exprType(result) == JSONBOID)
+		{
+			result = (Node *) transformJsonbSubscripts(pstate,
+					result,
+					exprType(result),
+					JSONBOID,
+					exprTypmod(result),
+					subscripts,
+					NULL);
+		}
+		else
+		{
+			result = (Node *) transformArraySubscripts(pstate,
+					result,
+					exprType(result),
+					InvalidOid,
+					exprTypmod(result),
+					subscripts,
+					NULL);
+		}
+
+	}
 
 	return result;
 }
diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index 4130cbf..3433532 100644
--- a/src/backend/parser/parse_node.c
+++ b/src/backend/parser/parse_node.c
@@ -258,6 +258,51 @@ transformArrayType(Oid *arrayType, int32 *arrayTypmod)
 	return elementType;
 }
 
+ArrayRef *
+transformJsonbSubscripts(ParseState *pstate,
+						 Node *arrayBase,
+						 Oid arrayType,
+						 Oid elementType,
+						 int32 arrayTypMod,
+						 List *indirection,
+						 Node *assignFrom)
+{
+	List	   *upperIndexpr = NIL;
+	List	   *lowerIndexpr = NIL;
+	ListCell   *idx;
+	ArrayRef   *aref;
+
+	/*
+	 * Transform the subscript expressions.
+	 */
+	foreach(idx, indirection)
+	{
+		A_Indices  *ai = (A_Indices *) lfirst(idx);
+		Node	   *subexpr;
+
+		Assert(IsA(ai, A_Indices));
+		subexpr = transformExpr(pstate, ai->uidx, pstate->p_expr_kind);
+		upperIndexpr = lappend(upperIndexpr, subexpr);
+	}
+
+	/*
+	 * Ready to build the ArrayRef node.
+	 */
+	aref = makeNode(ArrayRef);
+	aref->refarraytype = arrayType;
+	aref->refelemtype = elementType;
+	aref->reftypmod = arrayTypMod;
+	/* refcollid will be set by parse_collate.c */
+	aref->refupperindexpr = upperIndexpr;
+	aref->reflowerindexpr = lowerIndexpr;
+	aref->refexpr = (Expr *) arrayBase;
+	aref->refassgnexpr = (Expr *) assignFrom;
+
+	return aref;
+}
+
+
+
 /*
  * transformArraySubscripts()
  *		Transform array subscripting.  This is used for both
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 1b3fcd6..5f72a66 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -45,6 +45,19 @@ static Node *transformAssignmentIndirection(ParseState *pstate,
 							   ListCell *indirection,
 							   Node *rhs,
 							   int location);
+
+static Node *transformJsonbAssignmentSubscripts(ParseState *pstate,
+							  Node *basenode,
+							  const char *targetName,
+							  Oid targetTypeId,
+							  int32 targetTypMod,
+							  Oid targetCollation,
+							  List *subscripts,
+							  bool isSlice,
+							  ListCell *next_indirection,
+							  Node *rhs,
+							  int location);
+
 static Node *transformAssignmentSubscripts(ParseState *pstate,
 							  Node *basenode,
 							  const char *targetName,
@@ -744,27 +757,52 @@ transformAssignmentIndirection(ParseState *pstate,
 	if (subscripts)
 	{
 		/* recurse, and then return because we're done */
-		return transformAssignmentSubscripts(pstate,
-											 basenode,
-											 targetName,
-											 targetTypeId,
-											 targetTypMod,
-											 targetCollation,
-											 subscripts,
-											 isSlice,
-											 NULL,
-											 rhs,
-											 location);
+		if (exprType(basenode) == JSONBOID)
+		{
+			return transformJsonbAssignmentSubscripts(pstate,
+					basenode,
+					targetName,
+					targetTypeId,
+					targetTypMod,
+					targetCollation,
+					subscripts,
+					isSlice,
+					i,
+					rhs,
+					location);
+		}
+		else
+		{
+			return transformAssignmentSubscripts(pstate,
+					basenode,
+					targetName,
+					targetTypeId,
+					targetTypMod,
+					targetCollation,
+					subscripts,
+					isSlice,
+					NULL,
+					rhs,
+					location);
+		}
 	}
 
 	/* base case: just coerce RHS to match target type ID */
 
-	result = coerce_to_target_type(pstate,
-								   rhs, exprType(rhs),
-								   targetTypeId, targetTypMod,
-								   COERCION_ASSIGNMENT,
-								   COERCE_IMPLICIT_CAST,
-								   -1);
+	if (targetTypeId != InvalidOid)
+	{
+		result = coerce_to_target_type(pstate,
+									   rhs, exprType(rhs),
+									   targetTypeId, targetTypMod,
+									   COERCION_ASSIGNMENT,
+									   COERCE_IMPLICIT_CAST,
+									   -1);
+	}
+	else
+	{
+		result = rhs;
+	}
+
 	if (result == NULL)
 	{
 		if (targetIsArray)
@@ -792,6 +830,92 @@ transformAssignmentIndirection(ParseState *pstate,
 	return result;
 }
 
+static Node *
+transformJsonbAssignmentSubscripts(ParseState *pstate,
+							  Node *basenode,
+							  const char *targetName,
+							  Oid targetTypeId,
+							  int32 targetTypMod,
+							  Oid targetCollation,
+							  List *subscripts,
+							  bool isSlice,
+							  ListCell *next_indirection,
+							  Node *rhs,
+							  int location)
+{
+	Node	   *result;
+	Oid			arrayType;
+	int32		arrayTypMod;
+	Oid			elementTypeId;
+	Oid			typeNeeded;
+	Oid			collationNeeded;
+
+	Assert(subscripts != NIL);
+
+	/* Identify the actual array type and element type involved */
+	arrayType = targetTypeId;
+	arrayTypMod = targetTypMod;
+	elementTypeId = InvalidOid;
+
+	/* Identify type that RHS must provide */
+	typeNeeded = isSlice ? arrayType : elementTypeId;
+
+	/*
+	 * Array normally has same collation as elements, but there's an
+	 * exception: we might be subscripting a domain over an array type. In
+	 * that case use collation of the base type.
+	 */
+	if (arrayType == targetTypeId)
+		collationNeeded = targetCollation;
+	else
+		collationNeeded = get_typcollation(arrayType);
+
+	/* recurse to create appropriate RHS for array assign */
+	rhs = transformAssignmentIndirection(pstate,
+										 NULL,
+										 targetName,
+										 true,
+										 typeNeeded,
+										 arrayTypMod,
+										 collationNeeded,
+										 next_indirection,
+										 rhs,
+										 location);
+
+	/* process subscripts */
+	result = (Node *) transformJsonbSubscripts(pstate,
+											   basenode,
+											   arrayType,
+											   exprType(rhs),
+											   arrayTypMod,
+											   subscripts,
+											   rhs);
+
+	/* If target was a domain over array, need to coerce up to the domain */
+	if (arrayType != targetTypeId)
+	{
+		Oid			resulttype = exprType(result);
+
+		result = coerce_to_target_type(pstate,
+									   result, resulttype,
+									   targetTypeId, targetTypMod,
+									   COERCION_ASSIGNMENT,
+									   COERCE_IMPLICIT_CAST,
+									   -1);
+		/* can fail if we had int2vector/oidvector, but not for true domains */
+		if (result == NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_CANNOT_COERCE),
+					 errmsg("cannot cast type %s to %s",
+							format_type_be(resulttype),
+							format_type_be(targetTypeId)),
+					 parser_errposition(pstate, location)));
+	}
+
+	return result;
+}
+
+
 /*
  * helper for transformAssignmentIndirection: process array assignment
  */
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 154bc36..523a0ca 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -1943,3 +1943,45 @@ jsonb_object_agg_finalfn(PG_FUNCTION_ARGS)
 
 	PG_RETURN_POINTER(out);
 }
+
+
+Datum
+jsonb_set_element(Datum jsonbdatum,
+		text **path, int path_len, Datum sourceData, Oid source_type)
+{
+	Jsonb				*jb = DatumGetJsonb(jsonbdatum);
+	JsonbInState		result;
+	JsonbTypeCategory	tcategory;
+	Oid					outfuncoid;
+	JsonbValue			v, newkey, newval;
+	JsonbParseState 	*state = NULL;
+	JsonbIterator 		*it;
+	uint32 				r;
+	JsonbValue 			*res = NULL;
+	int					i, level = 0;
+	bool				*path_nulls = palloc(path_len * sizeof(bool));
+
+	jsonb_categorize_type(source_type,
+						  &tcategory, &outfuncoid);
+	memset(&result, 0, sizeof(JsonbInState));
+	result.parseState = NULL;
+	datum_to_jsonb(sourceData, false, &result, tcategory, outfuncoid, false);
+
+	it = JsonbIteratorInit(&jb->root);
+
+	newval = *result.res;
+
+	if (newval.type == jbvArray && newval.val.array.rawScalar == true)
+	{
+		newval = newval.val.array.elems[0];
+	}
+
+	for(i = 0; i < path_len; i++)
+	{
+		path_nulls[i]= false;
+	}
+
+	res = setPath(&it, path, path_nulls, path_len, &state, 0, (void *)&newval, true, true);
+
+	PG_RETURN_JSONB(JsonbValueToJsonb(res));
+}
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index 4d73315..aa2384b 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -408,7 +408,6 @@ findJsonbValueFromContainer(JsonbContainer *container, uint32 flags,
 	pfree(result);
 	return NULL;
 }
-
 /*
  * Get i-th value of a Jsonb array.
  *
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 3b8d42e..1b4074e 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,12 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+#define add_newval(state, newval, unpacked) \
+	if (unpacked) \
+		(void) pushJsonbValue(st, WJB_VALUE, (JsonbValue *)newval); \
+	else  \
+		addJsonbToParseState(st, (Jsonb *)newval);
+
 /* 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);
@@ -127,17 +133,13 @@ static JsonbValue *findJsonbValueFromContainerLen(JsonbContainer *container,
 /* functions supporting jsonb_delete, jsonb_set and jsonb_concat */
 static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 			   JsonbParseState **state);
-static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
-		bool *path_nulls, int path_len,
-		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  void *newval, bool unpacked, uint32 npairs, bool create);
 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, void *newval, bool unpacked, uint32 nelems, bool create);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3538,7 +3540,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(&in->root);
 
 	res = setPath(&it, path_elems, path_nulls, path_len, &st,
-				  0, newval, create);
+				  0, (void *)newval, false, create);
 
 	Assert(res != NULL);
 
@@ -3582,7 +3584,7 @@ jsonb_delete_path(PG_FUNCTION_ARGS)
 
 	it = JsonbIteratorInit(&in->root);
 
-	res = setPath(&it, path_elems, path_nulls, path_len, &st, 0, NULL, false);
+	res = setPath(&it, path_elems, path_nulls, path_len, &st, 0, NULL, false, false);
 
 	Assert(res != NULL);
 
@@ -3709,10 +3711,10 @@ IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
  * does not exist. All path elements before the last must already exist
  * whether or not create is true, or nothing is done.
  */
-static JsonbValue *
+JsonbValue *
 setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
-		JsonbParseState **st, int level, Jsonb *newval, bool create)
+		JsonbParseState **st, int level, void *newval, bool unpacked, bool create)
 {
 	JsonbValue	v;
 	JsonbValue *res = NULL;
@@ -3725,7 +3727,7 @@ setPath(JsonbIterator **it, Datum *path_elems,
 		case WJB_BEGIN_ARRAY:
 			(void) pushJsonbValue(st, r, NULL);
 			setPathArray(it, path_elems, path_nulls, path_len, st, level,
-						 newval, v.val.array.nElems, create);
+						 newval, unpacked, v.val.array.nElems, create);
 			r = JsonbIteratorNext(it, &v, false);
 			Assert(r == WJB_END_ARRAY);
 			res = pushJsonbValue(st, r, NULL);
@@ -3734,7 +3736,7 @@ setPath(JsonbIterator **it, Datum *path_elems,
 		case WJB_BEGIN_OBJECT:
 			(void) pushJsonbValue(st, r, NULL);
 			setPathObject(it, path_elems, path_nulls, path_len, st, level,
-						  newval, v.val.object.nPairs, create);
+						  newval, unpacked, v.val.object.nPairs, create);
 			r = JsonbIteratorNext(it, &v, true);
 			Assert(r == WJB_END_OBJECT);
 			res = pushJsonbValue(st, r, NULL);
@@ -3757,7 +3759,7 @@ setPath(JsonbIterator **it, Datum *path_elems,
 static void
 setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			  int path_len, JsonbParseState **st, int level,
-			  Jsonb *newval, uint32 npairs, bool create)
+			  void *newval, bool unpacked, uint32 npairs, bool create)
 {
 	JsonbValue	v;
 	int			i;
@@ -3767,17 +3769,34 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 	if (level >= path_len || path_nulls[level])
 		done = true;
 
-	/* empty object is a special case for create */
-	if ((npairs == 0) && create && (level == path_len - 1))
+	if ((npairs == 0) && create)
 	{
+		int		i;
+		for (i = 0; i < path_len - 1 - level; i++)
+		{
+			JsonbValue	newkey;
+
+			newkey.type = jbvString;
+			newkey.val.string.len = VARSIZE_ANY_EXHDR(path_elems[level + i]);
+			newkey.val.string.val = VARDATA_ANY(path_elems[level + i]);
+
+			(void) pushJsonbValue(st, WJB_KEY, &newkey);
+			(void) pushJsonbValue(st, WJB_BEGIN_OBJECT, NULL);
+		}
+
 		JsonbValue	newkey;
 
 		newkey.type = jbvString;
-		newkey.val.string.len = VARSIZE_ANY_EXHDR(path_elems[level]);
-		newkey.val.string.val = VARDATA_ANY(path_elems[level]);
+		newkey.val.string.len = VARSIZE_ANY_EXHDR(path_elems[level + i]);
+		newkey.val.string.val = VARDATA_ANY(path_elems[level + i]);
 
 		(void) pushJsonbValue(st, WJB_KEY, &newkey);
-		addJsonbToParseState(st, newval);
+		add_newval(st, newval, unpacked);
+
+		for (i = 0; i < path_len - 1 - level; i++)
+		{
+			(void) pushJsonbValue(st, WJB_END_OBJECT, NULL);
+		}
 	}
 
 	for (i = 0; i < npairs; i++)
@@ -3797,7 +3816,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 				if (newval != NULL)
 				{
 					(void) pushJsonbValue(st, WJB_KEY, &k);
-					addJsonbToParseState(st, newval);
+					add_newval(st, newval, unpacked);
 				}
 				done = true;
 			}
@@ -3805,7 +3824,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			{
 				(void) pushJsonbValue(st, r, &k);
 				setPath(it, path_elems, path_nulls, path_len,
-						st, level + 1, newval, create);
+						st, level + 1, newval, unpacked, create);
 			}
 		}
 		else
@@ -3819,7 +3838,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 				newkey.val.string.val = VARDATA_ANY(path_elems[level]);
 
 				(void) pushJsonbValue(st, WJB_KEY, &newkey);
-				addJsonbToParseState(st, newval);
+				add_newval(st, newval, unpacked);
 			}
 
 			(void) pushJsonbValue(st, r, &k);
@@ -3851,7 +3870,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 static void
 setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			 int path_len, JsonbParseState **st, int level,
-			 Jsonb *newval, uint32 nelems, bool create)
+			 void *newval, bool unpacked, uint32 nelems, bool create)
 {
 	JsonbValue	v;
 	int			idx,
@@ -3896,7 +3915,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 	if ((idx == INT_MIN || nelems == 0) && create && (level == path_len - 1))
 	{
 		Assert(newval != NULL);
-		addJsonbToParseState(st, newval);
+		add_newval(st, newval, unpacked);
 		done = true;
 	}
 
@@ -3911,13 +3930,13 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			{
 				r = JsonbIteratorNext(it, &v, true);	/* skip */
 				if (newval != NULL)
-					addJsonbToParseState(st, newval);
+					add_newval(st, newval, unpacked);
 
 				done = true;
 			}
 			else
 				(void) setPath(it, path_elems, path_nulls, path_len,
-							   st, level + 1, newval, create);
+							   st, level + 1, newval, unpacked, create);
 		}
 		else
 		{
@@ -3944,9 +3963,46 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 
 			if (create && !done && level == path_len - 1 && i == nelems - 1)
 			{
-				addJsonbToParseState(st, newval);
+				add_newval(st, newval, unpacked);
 			}
 
 		}
 	}
 }
+
+
+Datum
+jsonb_get_element(Datum jsonbdatum,
+		text **path, int path_len, bool *isNull)
+{
+	Jsonb	   *jb = DatumGetJsonb(jsonbdatum);
+	JsonbValue *v;
+	int			level = 1;
+
+	if (!JB_ROOT_IS_OBJECT(jb))
+	{
+		*isNull = true;
+		return (Datum) 0;
+	}
+
+	v = findJsonbValueFromContainerLen(&jb->root, JB_FOBJECT,
+									   VARDATA_ANY(path[0]),
+									   VARSIZE_ANY_EXHDR(path[0]));
+
+	while (v != NULL &&
+		   v->type == jbvBinary && level < path_len)
+	{
+		v = findJsonbValueFromContainerLen(v->val.binary.data, JB_FOBJECT,
+									       VARDATA_ANY(path[level]),
+									       VARSIZE_ANY_EXHDR(path[level]));
+		level++;
+	}
+
+	if (v != NULL && level == path_len)
+	{
+		PG_RETURN_JSONB(JsonbValueToJsonb(v));
+	}
+
+	*isNull = true;
+	return (Datum) 0;
+}
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 5249945..8c50e2b 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -224,6 +224,13 @@ extern void cancel_parser_errposition_callback(ParseCallbackState *pcbstate);
 extern Var *make_var(ParseState *pstate, RangeTblEntry *rte, int attrno,
 		 int location);
 extern Oid	transformArrayType(Oid *arrayType, int32 *arrayTypmod);
+extern ArrayRef *transformJsonbSubscripts(ParseState *pstate,
+						 Node *arrayBase,
+						 Oid arrayType,
+						 Oid elementType,
+						 int32 arrayTypMod,
+						 List *indirection,
+						 Node *assignFrom);
 extern ArrayRef *transformArraySubscripts(ParseState *pstate,
 						 Node *arrayBase,
 						 Oid arrayType,
diff --git a/src/include/utils/jsonb.h b/src/include/utils/jsonb.h
index 3049a87..124b6b4 100644
--- a/src/include/utils/jsonb.h
+++ b/src/include/utils/jsonb.h
@@ -433,5 +433,11 @@ extern char *JsonbToCString(StringInfo out, JsonbContainer *in,
 extern char *JsonbToCStringIndent(StringInfo out, JsonbContainer *in,
 					 int estimated_len);
 
+extern Datum jsonb_set_element(Datum datum, text **path, int path_len, Datum sourceData, Oid source_type);
+extern Datum jsonb_get_element(Datum datum, text **path, int path_len, bool *isNull);
+extern JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
+		bool *path_nulls, int path_len,
+		JsonbParseState **st, int level, void *newval,
+		bool unpacked, bool create);
 
 #endif   /* __JSONB_H__ */
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 17656d4..915f97b 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -3295,3 +3295,97 @@ select jsonb_set('[]','{-99}','{"foo":123}');
  [{"foo": 123}]
 (1 row)
 
+-- jsonb subscript
+select ('{"a": 1}'::jsonb)['a'];
+ jsonb 
+-------
+ 1
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['b'];
+ jsonb 
+-------
+ "c"
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'];
+   jsonb   
+-----------
+ [1, 2, 3]
+(1 row)
+
+select ('{"a": 1}'::jsonb)['not_exist'];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1'];
+     jsonb     
+---------------
+ {"a2": "aaa"}
+(1 row)
+
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2'];
+ jsonb 
+-------
+ "aaa"
+(1 row)
+
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2']['a3'];
+ jsonb 
+-------
+ 
+(1 row)
+
+create TEMP TABLE test_jsonb_subscript (
+       id int,
+       test_json jsonb
+);
+insert into test_jsonb_subscript values
+(1, '{}'),
+(2, '{}');
+update test_jsonb_subscript set test_json['a'] = 1;
+select * from test_jsonb_subscript;
+ id | test_json 
+----+-----------
+  1 | {"a": 1}
+  2 | {"a": 1}
+(2 rows)
+
+update test_jsonb_subscript set test_json['a'] = 'test';
+select * from test_jsonb_subscript;
+ id |   test_json   
+----+---------------
+  1 | {"a": "test"}
+  2 | {"a": "test"}
+(2 rows)
+
+update test_jsonb_subscript set test_json['a'] = '{"b": 1}'::jsonb;
+select * from test_jsonb_subscript;
+ id |    test_json    
+----+-----------------
+  1 | {"a": {"b": 1}}
+  2 | {"a": {"b": 1}}
+(2 rows)
+
+update test_jsonb_subscript set test_json['a'] = '[1, 2, 3]'::jsonb;
+select * from test_jsonb_subscript;
+ id |    test_json     
+----+------------------
+  1 | {"a": [1, 2, 3]}
+  2 | {"a": [1, 2, 3]}
+(2 rows)
+
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values
+(1, '{}'),
+(2, '{}');
+update test_jsonb_subscript set test_json['b']['b1']['b2'] = 1;
+select * from test_jsonb_subscript;
+ id |        test_json         
+----+--------------------------
+  1 | {"b": {"b1": {"b2": 1}}}
+  2 | {"b": {"b1": {"b2": 1}}}
+(2 rows)
+
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 83ed4eb..d7e3aa7 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -809,3 +809,41 @@ select jsonb_set('{}','{x}','{"foo":123}');
 select jsonb_set('[]','{0}','{"foo":123}');
 select jsonb_set('[]','{99}','{"foo":123}');
 select jsonb_set('[]','{-99}','{"foo":123}');
+
+-- jsonb subscript
+select ('{"a": 1}'::jsonb)['a'];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['b'];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'];
+select ('{"a": 1}'::jsonb)['not_exist'];
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1'];
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2'];
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2']['a3'];
+
+create TEMP TABLE test_jsonb_subscript (
+       id int,
+       test_json jsonb
+);
+
+insert into test_jsonb_subscript values
+(1, '{}'),
+(2, '{}');
+
+update test_jsonb_subscript set test_json['a'] = 1;
+select * from test_jsonb_subscript;
+
+update test_jsonb_subscript set test_json['a'] = 'test';
+select * from test_jsonb_subscript;
+
+update test_jsonb_subscript set test_json['a'] = '{"b": 1}'::jsonb;
+select * from test_jsonb_subscript;
+
+update test_jsonb_subscript set test_json['a'] = '[1, 2, 3]'::jsonb;
+select * from test_jsonb_subscript;
+
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values
+(1, '{}'),
+(2, '{}');
+
+update test_jsonb_subscript set test_json['b']['b1']['b2'] = 1;
+select * from test_jsonb_subscript;
#2Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Dmitry Dolgov (#1)
Re: jsonb array-style subscripting

On 8/17/15 12:57 PM, Dmitry Dolgov wrote:

* is it interesting for the community?

We definitely need better ways to manipulate JSON.

* is that a good idea to extend the `ArrayRef` for jsonb? If it's
appropriate, probably we can rename it to `ArrayJsonbRef` of something.
* what can be improved in the code at the top level (function placement,
probably, functionality duplication, etc.)?
* are there any special cases, that I should take care of in this
implementation?

How would this work when you have a JSON array? Postgres array syntax
suddenly becoming key/value syntax for JSON seems like a pretty bad idea
to me. Could a different syntax (maybe {}) be used instead?

I'm not sure having the UPDATE you show cause objects to spring to life
is so great either.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com

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

#3Merlin Moncure
mmoncure@gmail.com
In reply to: Dmitry Dolgov (#1)
Re: jsonb array-style subscripting

On Mon, Aug 17, 2015 at 12:57 PM, Dmitry Dolgov <9erthalion6@gmail.com> wrote:

Hi,

Some time ago the array-style subscripting for the jsonb data type was
discussed in this mailing list. I think it will be quite convenient to have
a such nice syntax to update jsonb objects, so I'm trying to implement this.
I created a patch, that allows doing something like this:

=# create TEMP TABLE test_jsonb_subscript (
id int,
test_json jsonb
);

=# insert into test_jsonb_subscript values
(1, '{}'),
(2, '{}');

=# update test_jsonb_subscript set test_json['a']['a1']['a2'] = 42;
=# select * from test_jsonb_subscript;
id | test_json
----+--------------------------
1 | {"a": {"a1": {"a2": 42}}}
2 | {"a": {"a1": {"a2": 42}}}
(2 rows)

=# select test_json['a']['a1'] from test_jsonb_subscript;
test_json
------------
{"a2": 42}
{"a2": 42}
(2 rows)

This patch has a status "work in progress" of course. Generally speaking,
this implementation extends the `ArrayRef` usage for the jsonb.
And I need some sort of advice about several questions:

* is it interesting for the community?
* is that a good idea to extend the `ArrayRef` for jsonb? If it's
appropriate, probably we can rename it to `ArrayJsonbRef` of something.
* what can be improved in the code at the top level (function placement,
probably, functionality duplication, etc.)?
* are there any special cases, that I should take care of in this
implementation?

I'm not sure if this:
update test_jsonb_subscript set test_json['a']['a1']['a2'] = 42;

...is a good idea. postgres operators tend to return immutable copies
of the item they are referring to. In other words, you'd never see a
column operator on the 'left' side of the equals in an update
statement. I think you need to look at a function to get the behavior
you want:

update test_jsonb_subscript set test_json = jsonb_modify(test_json,
'[a][a1][a2] = 42');]

...as a hypothetical example. The idea is you need to make a
function that provides the ability to make the complete json you want.
Update statements always make a copy of the record anyways.

merlin

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

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Dmitry Dolgov (#1)
Re: jsonb array-style subscripting

Dmitry Dolgov wrote:

Some time ago the array-style subscripting for the jsonb data type was
discussed in this mailing list. I think it will be quite convenient to have
a such nice syntax to update jsonb objects, so I'm trying to implement
this. I created a patch, that allows doing something like this:

=# update test_jsonb_subscript set test_json['a']['a1']['a2'] = 42;

FWIW we discussed exactly this during the unconference
https://wiki.postgresql.org/wiki/PgCon_2015_Developer_Unconference#Direction_of_json_and_jsonb
and Andrew and Tom seemed okay with this syntax.

--
�lvaro Herrera http://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

#5Peter Geoghegan
pg@heroku.com
In reply to: Merlin Moncure (#3)
Re: jsonb array-style subscripting

On Mon, Aug 17, 2015 at 12:26 PM, Merlin Moncure <mmoncure@gmail.com> wrote:

I'm not sure if this:
update test_jsonb_subscript set test_json['a']['a1']['a2'] = 42;

...is a good idea.

This kind of "targetlist indirection" is already possible with arrays
and composite types.

--
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

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Merlin Moncure (#3)
Re: jsonb array-style subscripting

On 08/17/2015 03:26 PM, Merlin Moncure wrote:

I'm not sure if this:
update test_jsonb_subscript set test_json['a']['a1']['a2'] = 42;

...is a good idea. postgres operators tend to return immutable copies
of the item they are referring to. In other words, you'd never see a
column operator on the 'left' side of the equals in an update
statement. I think you need to look at a function to get the behavior
you want:

update test_jsonb_subscript set test_json = jsonb_modify(test_json,
'[a][a1][a2] = 42');]

...as a hypothetical example. The idea is you need to make a
function that provides the ability to make the complete json you want.
Update statements always make a copy of the record anyways.

Why should jsonb be different from an array? You can assign to an array
element, using exactly this syntax, except that the index expressions
have to be integers.

This was discussed at pgcon and generally met with approval. There is
some demand for it. If Dmitry hadn't done this I would probably have
done it myself.

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

#7Peter Geoghegan
pg@heroku.com
In reply to: Merlin Moncure (#3)
Re: jsonb array-style subscripting

On Mon, Aug 17, 2015 at 12:26 PM, Merlin Moncure <mmoncure@gmail.com> wrote:

...is a good idea. postgres operators tend to return immutable copies
of the item they are referring to.

This patch does not add an operator at all, actually. If feels like
there ought to be an operator, but in fact there is not. The parser is
hard-coded to recognize array-style subscripts, which this uses.

While I'm certainly glad that Dmitry took the time to work on this, I
think we will need an operator, too. Or, more accurately, there should
probably be a way to make something like this use some available GIN
index:

postgres=# explain analyze select * from testjsonb where p['a'] = '[1]';
QUERY PLAN
-----------------------------------------------------------------------------------------------------
Seq Scan on testjsonb (cost=0.00..27.00 rows=7 width=32) (actual
time=0.022..0.023 rows=1 loops=1)
Filter: (p['a'] = '[1]'::jsonb)
Planning time: 0.070 ms
Execution time: 0.054 ms
(4 rows)

This doesn't really matter with arrays, but ISTM that it matters here.
I have no strong feelings on how it should work, but certain things do
seem to suggest themselves. For example, maybe the parser can be made
to create a query tree that uses an indexable operator based on
special-case logic. Although maybe that's a kludge too far, since I
can imagine it breaking other legitimate things. My sense is that this
will need to be discussed.

--
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

#8Josh Berkus
josh@agliodbs.com
In reply to: Dmitry Dolgov (#1)
Re: jsonb array-style subscripting

On 08/17/2015 10:57 AM, Dmitry Dolgov wrote:

Hi,

Some time ago the array-style subscripting for the jsonb data type was
discussed in this mailing list. I think it will be quite convenient to
have a such nice syntax to update jsonb objects, so I'm trying to
implement this. I created a patch, that allows doing something like this:

Yaaay!

=# create TEMP TABLE test_jsonb_subscript (
id int,
test_json jsonb
);

=# insert into test_jsonb_subscript values
(1, '{}'),
(2, '{}');

=# update test_jsonb_subscript set test_json['a']['a1']['a2'] = 42;
=# select * from test_jsonb_subscript;
id | test_json
----+--------------------------
1 | {"a": {"a1": {"a2": 42}}}
2 | {"a": {"a1": {"a2": 42}}}
(2 rows)

So, both perl and python do not allow "deep nesting" of assignments.
For example:

d = { "a" : { } }
d["a"]["a1"]["a2"] = 42

Traceback (most recent call last):
File "<stdin>", line 1, in <module>
KeyError: 'a1'

... you have to append one key level at a time. Your approach, on the
other hand, feels more user-friendly to me; I can't tell you the number
of "if 'a2' in dic[key]" tests I've written.

So, is there any reason why consistency with perl/python behavior would
be more desirable than user-friendliness? I'm thinking no, but figured
that it's something which needs to come up.

There is one ambiguous case you need to address:

testjson = '{ "a" : { } }'

SET testjson['a']['a1']['1'] = 42

... so in this case, is '1' a key, or the first item of an array? how
do we determine that? How does the user assign something to an array?

=# select test_json['a']['a1'] from test_jsonb_subscript;
test_json
------------
{"a2": 42}
{"a2": 42}
(2 rows)

Again, how do we handle missing keys? Just return NULL? or ERROR? I'd
prefer the former, but there will be arguments the other way.

This patch has a status "work in progress" of course. Generally
speaking, this implementation extends the `ArrayRef` usage for the jsonb.
And I need some sort of advice about several questions:

* is it interesting for the community?
* is that a good idea to extend the `ArrayRef` for jsonb? If it's
appropriate, probably we can rename it to `ArrayJsonbRef` of something.
* what can be improved in the code at the top level (function placement,
probably, functionality duplication, etc.)?
* are there any special cases, that I should take care of in this
implementation?

array/key ambiguity is going to be painful.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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

#9Merlin Moncure
mmoncure@gmail.com
In reply to: Andrew Dunstan (#6)
Re: jsonb array-style subscripting

On Mon, Aug 17, 2015 at 2:44 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 08/17/2015 03:26 PM, Merlin Moncure wrote:

I'm not sure if this:
update test_jsonb_subscript set test_json['a']['a1']['a2'] = 42;

...is a good idea. postgres operators tend to return immutable copies
of the item they are referring to. In other words, you'd never see a
column operator on the 'left' side of the equals in an update
statement. I think you need to look at a function to get the behavior
you want:

update test_jsonb_subscript set test_json = jsonb_modify(test_json,
'[a][a1][a2] = 42');]

...as a hypothetical example. The idea is you need to make a
function that provides the ability to make the complete json you want.
Update statements always make a copy of the record anyways.

Why should jsonb be different from an array? You can assign to an array
element, using exactly this syntax, except that the index expressions have
to be integers.

This was discussed at pgcon and generally met with approval. There is some
demand for it. If Dmitry hadn't done this I would probably have done it
myself.

Yeah, this all makes sense. I withdraw the statement.

merlin

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

#10Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Josh Berkus (#8)
Re: jsonb array-style subscripting

On 8/17/15 3:33 PM, Josh Berkus wrote:

Again, how do we handle missing keys? Just return NULL? or ERROR? I'd
prefer the former, but there will be arguments the other way.

I've been wondering if we should add some kind of "strict" JSON. My big
concern is throwing an error if you try to provide duplicate keys, but
it seems reasonable that json_strict would throw an error if you try to
reference something that doesn't exist.

This patch has a status "work in progress" of course. Generally
speaking, this implementation extends the `ArrayRef` usage for the jsonb.
And I need some sort of advice about several questions:

* is it interesting for the community?
* is that a good idea to extend the `ArrayRef` for jsonb? If it's
appropriate, probably we can rename it to `ArrayJsonbRef` of something.
* what can be improved in the code at the top level (function placement,
probably, functionality duplication, etc.)?
* are there any special cases, that I should take care of in this
implementation?

array/key ambiguity is going to be painful.

JSON keys are required to be strings, so maybe it's OK to differentiate
based on whether the index is a string or a number. Or perhaps we use
different nomenclature (ie: {} for objects).

We should also think about what would work for hstore. Adding this for
hstore is clearly separate work, but it'd be bad to close that door here.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com

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

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dmitry Dolgov (#1)
Re: jsonb array-style subscripting

Dmitry Dolgov <9erthalion6@gmail.com> writes:

* is that a good idea to extend the `ArrayRef` for jsonb?

No. Make a new expression node type.

(Salesforce did something similar for an internal feature, and it was a
disaster both for code modularity and performance. We had to change it to
a separate node type, which I just got finished doing. Don't go down that
path. While you're at it, I'd advise that fetch and assignment be two
different node types rather than copying ArrayRef's bad precedent of using
only one.)

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

#12Josh Berkus
josh@agliodbs.com
In reply to: Dmitry Dolgov (#1)
Re: jsonb array-style subscripting

On 08/17/2015 02:18 PM, Jim Nasby wrote:

On 8/17/15 3:33 PM, Josh Berkus wrote:

Again, how do we handle missing keys? Just return NULL? or ERROR? I'd
prefer the former, but there will be arguments the other way.

I've been wondering if we should add some kind of "strict" JSON. My big
concern is throwing an error if you try to provide duplicate keys, but
it seems reasonable that json_strict would throw an error if you try to
reference something that doesn't exist.

Only if there's demand for it. Is there?

array/key ambiguity is going to be painful.

JSON keys are required to be strings, so maybe it's OK to differentiate
based on whether the index is a string or a number. Or perhaps we use
different nomenclature (ie: {} for objects).

Well, we did get rid of all of those implicit conversions for a reason.
So maybe that's good enough? i.e.

json['a']['b'][1] = 5
"assign 5 as the first element in the array 'b' of object 'a'"

json['a']['b']['1'] = 5
"assign 5 to key '1' of object 'b' of object 'a'"

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Geoghegan (#7)
Re: jsonb array-style subscripting

Peter Geoghegan <pg@heroku.com> writes:

This patch does not add an operator at all, actually. If feels like
there ought to be an operator, but in fact there is not. The parser is
hard-coded to recognize array-style subscripts, which this uses.

While I'm certainly glad that Dmitry took the time to work on this, I
think we will need an operator, too. Or, more accurately, there should
probably be a way to make something like this use some available GIN
index:

postgres=# explain analyze select * from testjsonb where p['a'] = '[1]';

Hm. There is definitely attraction to the idea that x[y] is some sort of
operator-like syntactic sugar for invocation of an underlying function,
rather than necessarily a hard-coded behavior. That would provide a
framework for extending the feature to all container-like datatypes,
whereas the approach Dimitry has prototyped doesn't look like it offers
much help at all for other datatypes.

But I do not think that that will change things very much as far as
making it possible for things like the above to be indexed. You'd
still have an expression tree that's a nest of two operators, which
doesn't fit into our ideas about index opclasses. (That is, it still
has to be a special case, so it doesn't matter that much if one of
the nodes is some kind of NotAnArrayRef rather than a FuncExpr with
some new CoercionForm value to make it print differently.)

Also, the syntactic sugar would have to be able to deal with multiple
subscripts, which makes things a lot more complicated.

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

#14Kaare Rasmussen
kaare@jasonic.dk
In reply to: Josh Berkus (#8)
Re: jsonb array-style subscripting

On 2015-08-17 22:33, Josh Berkus wrote:

So, both perl and python do not allow "deep nesting" of assignments.
For example:

d = { "a" : { } }
d["a"]["a1"]["a2"] = 42

Traceback (most recent call last):
File "<stdin>", line 1, in <module>
KeyError: 'a1'

Not sure I understand what you mean. In Perl you'd do

$ perl -e '%d = (a => {}); $d{a}{a1}{a2} = 42; print $d{a}{a1}{a2}'
42

which looks pretty much like what's proposed.

/kaare

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

#15Pavel Stehule
pavel.stehule@gmail.com
In reply to: Jim Nasby (#2)
Re: jsonb array-style subscripting

Hi

2015-08-17 21:12 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

On 8/17/15 12:57 PM, Dmitry Dolgov wrote:

* is it interesting for the community?

We definitely need better ways to manipulate JSON.

* is that a good idea to extend the `ArrayRef` for jsonb? If it's

appropriate, probably we can rename it to `ArrayJsonbRef` of something.
* what can be improved in the code at the top level (function placement,
probably, functionality duplication, etc.)?
* are there any special cases, that I should take care of in this
implementation?

How would this work when you have a JSON array? Postgres array syntax
suddenly becoming key/value syntax for JSON seems like a pretty bad idea to
me. Could a different syntax (maybe {}) be used instead?

I don't understand why '{}' should be better than '[]' ?

The lot of modern languages doesn't different between arrays and hash.

Regards

Pavel

Show quoted text

I'm not sure having the UPDATE you show cause objects to spring to life is
so great either.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com

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

#16Oleg Bartunov
obartunov@gmail.com
In reply to: Peter Geoghegan (#7)
Re: jsonb array-style subscripting

On Mon, Aug 17, 2015 at 11:26 PM, Peter Geoghegan <pg@heroku.com> wrote:

On Mon, Aug 17, 2015 at 12:26 PM, Merlin Moncure <mmoncure@gmail.com>
wrote:

...is a good idea. postgres operators tend to return immutable copies
of the item they are referring to.

This patch does not add an operator at all, actually. If feels like
there ought to be an operator, but in fact there is not. The parser is
hard-coded to recognize array-style subscripts, which this uses.

While I'm certainly glad that Dmitry took the time to work on this, I
think we will need an operator, too. Or, more accurately, there should
probably be a way to make something like this use some available GIN
index:

postgres=# explain analyze select * from testjsonb where p['a'] = '[1]';
QUERY PLAN

-----------------------------------------------------------------------------------------------------
Seq Scan on testjsonb (cost=0.00..27.00 rows=7 width=32) (actual
time=0.022..0.023 rows=1 loops=1)
Filter: (p['a'] = '[1]'::jsonb)
Planning time: 0.070 ms
Execution time: 0.054 ms
(4 rows)

This doesn't really matter with arrays, but ISTM that it matters here.
I have no strong feelings on how it should work, but certain things do
seem to suggest themselves. For example, maybe the parser can be made
to create a query tree that uses an indexable operator based on
special-case logic. Although maybe that's a kludge too far, since I
can imagine it breaking other legitimate things. My sense is that this
will need to be discussed.

Peter, we are thinking about better indexing of subselects, let's first
have the syntax sugar in core, which Dmitry implemented.

Show quoted text

--
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

#17Andrew Dunstan
andrew@dunslane.net
In reply to: Pavel Stehule (#15)
Re: jsonb array-style subscripting

On 08/18/2015 01:32 AM, Pavel Stehule wrote:

Hi

2015-08-17 21:12 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com
<mailto:Jim.Nasby@bluetreble.com>>:

On 8/17/15 12:57 PM, Dmitry Dolgov wrote:

* is it interesting for the community?

We definitely need better ways to manipulate JSON.

* is that a good idea to extend the `ArrayRef` for jsonb? If it's
appropriate, probably we can rename it to `ArrayJsonbRef` of
something.
* what can be improved in the code at the top level (function
placement,
probably, functionality duplication, etc.)?
* are there any special cases, that I should take care of in this
implementation?

How would this work when you have a JSON array? Postgres array
syntax suddenly becoming key/value syntax for JSON seems like a
pretty bad idea to me. Could a different syntax (maybe {}) be used
instead?

I don't understand why '{}' should be better than '[]' ?

The lot of modern languages doesn't different between arrays and hash.

What is more, it would be a lot more intrusive to the parser, I suspect.
We currently allow json path expressions to contain both object key and
array index values, and I don't see any reason to do this differently.
This is the syntax that was discussed at pgcon.

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

#18Andrew Dunstan
andrew@dunslane.net
In reply to: Kaare Rasmussen (#14)
Re: jsonb array-style subscripting

On 08/18/2015 01:11 AM, Kaare Rasmussen wrote:

On 2015-08-17 22:33, Josh Berkus wrote:

So, both perl and python do not allow "deep nesting" of assignments.
For example:

d = { "a" : { } }
d["a"]["a1"]["a2"] = 42

Traceback (most recent call last):
File "<stdin>", line 1, in <module>
KeyError: 'a1'

Not sure I understand what you mean. In Perl you'd do

$ perl -e '%d = (a => {}); $d{a}{a1}{a2} = 42; print $d{a}{a1}{a2}'
42

which looks pretty much like what's proposed.

Indeed, I mentioned recently that perl auto-vivifies intermediate paths
like this.

But we don't do that in jsonb_set, as was discussed back in May, and as
JS doesn't either I think that decision is sound.

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

#19Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Josh Berkus (#12)
Re: jsonb array-style subscripting

On 8/17/15 4:25 PM, Josh Berkus wrote:

On 08/17/2015 02:18 PM, Jim Nasby wrote:

On 8/17/15 3:33 PM, Josh Berkus wrote:

Again, how do we handle missing keys? Just return NULL? or ERROR? I'd
prefer the former, but there will be arguments the other way.

I've been wondering if we should add some kind of "strict" JSON. My big
concern is throwing an error if you try to provide duplicate keys, but
it seems reasonable that json_strict would throw an error if you try to
reference something that doesn't exist.

Only if there's demand for it. Is there?

I'm certainly worried (paranoid?) about it. Postgres is very good about
not silently dropping data and this seems a glaring departure from that.
I haven't looked yet but I'm hoping this could at least be added as an
extension without duplicating a bunch of the existing JSON stuff.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com

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

#20Andrew Dunstan
andrew@dunslane.net
In reply to: Jim Nasby (#19)
Re: jsonb array-style subscripting

On 08/20/2015 03:24 PM, Jim Nasby wrote:

On 8/17/15 4:25 PM, Josh Berkus wrote:

On 08/17/2015 02:18 PM, Jim Nasby wrote:

On 8/17/15 3:33 PM, Josh Berkus wrote:

Again, how do we handle missing keys? Just return NULL? or

ERROR? I'd

prefer the former, but there will be arguments the other way.

I've been wondering if we should add some kind of "strict" JSON. My

big

concern is throwing an error if you try to provide duplicate keys, but
it seems reasonable that json_strict would throw an error if you

try to

reference something that doesn't exist.

Only if there's demand for it. Is there?

I'm certainly worried (paranoid?) about it. Postgres is very good
about not silently dropping data and this seems a glaring departure
from that. I haven't looked yet but I'm hoping this could at least be
added as an extension without duplicating a bunch of the existing JSON
stuff.

What on earth does this have to do with duplicate keys? The jsonb input
rules on duplicate keys are very clear, incidentally, and follow the
practice of most JSON processors (duplicates are discarded, last one
wins.) But that has nothing at all to do with this feature, really.

This feature can NOT be added as an extension, since it requires grammar
processing modification.

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

#21Josh Berkus
josh@agliodbs.com
In reply to: Dmitry Dolgov (#1)
Re: jsonb array-style subscripting

On 08/20/2015 12:24 PM, Jim Nasby wrote:

On 8/17/15 4:25 PM, Josh Berkus wrote:

On 08/17/2015 02:18 PM, Jim Nasby wrote:

On 8/17/15 3:33 PM, Josh Berkus wrote:

Again, how do we handle missing keys? Just return NULL? or

ERROR? I'd

prefer the former, but there will be arguments the other way.

I've been wondering if we should add some kind of "strict" JSON. My big
concern is throwing an error if you try to provide duplicate keys, but
it seems reasonable that json_strict would throw an error if you try to
reference something that doesn't exist.

Only if there's demand for it. Is there?

I'm certainly worried (paranoid?) about it. Postgres is very good about
not silently dropping data and this seems a glaring departure from that.
I haven't looked yet but I'm hoping this could at least be added as an
extension without duplicating a bunch of the existing JSON stuff.

There's a big difference between silently dropping data and implicitly
creating it. As I said, the only reason I could see wanting a strict
mode is because AppDev users expect it to be consistent with their
programming languages. Otherwise, from a user perspective, being able
to create a whole nested chain in one statement is a big win.

What could be added as an extension?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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

#22Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Josh Berkus (#21)
Re: jsonb array-style subscripting

On 8/20/15 3:44 PM, Josh Berkus wrote:

What could be added as an extension?

A method for preventing duplicate object keys.

Since I'm in the minority here lets just drop it. :)
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com

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