[PATCH 1/1] json: prefer tuple index as field name instead of json

[PATCH 1/1] json: prefer tuple index as field name instead of json

[Vladislav Shpilevoy]

Now when a tuple is indexed in lua, the index is interpreted as
a JSON path at first, and only then as a field name. It breaks
compatibility. Lets swap priority.

Follow up #1285
---
Branch: https://github.com/tarantool/tarantool/tree/gh-1285-json-fix

 src/box/tuple_format.c     | 54 +++++++++++++++++++++++++---------------------
 test/engine/tuple.result   |  9 +++++++-
 test/engine/tuple.test.lua |  4 +++-
 3 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 39770412e..d21477f89 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -547,21 +547,35 @@ tuple_field_raw_by_path(struct tuple_format *format, const char *tuple,
                         const char **field)
 {
 	assert(path_len > 0);
+	uint32_t fieldno;
+	/*
+	 * It is possible, that a field has a name as
+	 * well-formatted JSON. For example 'a.b.c.d' or '[1]' can
+	 * be field name. To save compatibility at first try to
+	 * use the path as a field name.
+	 */
+	if (tuple_fieldno_by_name(format->dict, path, path_len, path_hash,
+				  &fieldno) == 0) {
+		*field = tuple_field_raw(format, tuple, field_map, fieldno);
+		return 0;
+	}
 	struct json_path_parser parser;
 	struct json_path_node node;
 	json_path_parser_create(&parser, path, path_len);
 	int rc = json_path_next(&parser, &node);
 	if (rc != 0)
-		goto best_effort;
+		goto error;
 	switch(node.type) {
 	case JSON_PATH_NUM: {
 		int index = node.num;
-		if (index == 0)
-			goto best_effort;
+		if (index == 0) {
+			*field = NULL;
+			return 0;
+		}
 		index -= TUPLE_INDEX_BASE;
 		*field = tuple_field_raw(format, tuple, field_map, index);
 		if (*field == NULL)
-			goto best_effort;
+			return 0;
 		break;
 	}
 	case JSON_PATH_STR: {
@@ -581,7 +595,7 @@ tuple_field_raw_by_path(struct tuple_format *format, const char *tuple,
 		*field = tuple_field_raw_by_name(format, tuple, field_map,
 						 node.str, node.len, name_hash);
 		if (*field == NULL)
-			goto best_effort;
+			return 0;
 		break;
 	}
 	default:
@@ -589,7 +603,7 @@ tuple_field_raw_by_path(struct tuple_format *format, const char *tuple,
 		*field = NULL;
 		return 0;
 	}
-	while (rc == 0 && (rc = json_path_next(&parser, &node)) == 0) {
+	while ((rc = json_path_next(&parser, &node)) == 0) {
 		switch(node.type) {
 		case JSON_PATH_NUM:
 			rc = tuple_field_go_to_index(field, node.num);
@@ -601,24 +615,14 @@ tuple_field_raw_by_path(struct tuple_format *format, const char *tuple,
 			assert(node.type == JSON_PATH_END);
 			return 0;
 		}
+		if (rc != 0) {
+			*field = NULL;
+			return 0;
+		}
 	}
-	assert(rc != 0);
-	/*
-	 * It is possible, that a field has a name as
-	 * well-formatted JSON. For example 'a.b.c.d' can be field
-	 * name. If a data was not found by such path, then try
-	 * to interpret the whole path as a field name.
-	 * The same is true for field names, that are not valid
-	 * JSON.
-	 */
-best_effort:
-	*field = tuple_field_raw_by_name(format, tuple, field_map, path,
-					 path_len, path_hash);
-	if (rc > 0 && *field == NULL) {
-		diag_set(ClientError, ER_ILLEGAL_PARAMS,
-			 tt_sprintf("error in path on position %d", rc));
-		return -1;
-	} else {
-		return 0;
-	}
+error:
+	assert(rc > 0);
+	diag_set(ClientError, ER_ILLEGAL_PARAMS,
+		 tt_sprintf("error in path on position %d", rc));
+	return -1;
 }
diff --git a/test/engine/tuple.result b/test/engine/tuple.result
index 7a6dcba9e..e8a50a0b1 100644
--- a/test/engine/tuple.result
+++ b/test/engine/tuple.result
@@ -677,6 +677,9 @@ format[4] = {name = 'field4', type = 'string' }
 format[5] = {name = "[2][6]['привет中国world']['中国a']", type = 'string'}
 ---
 ...
+format[6] = {name = '[1]', type = 'unsigned'}
+---
+...
 s = box.schema.space.create('test', {format = format})
 ---
 ...
@@ -689,7 +692,7 @@ field2 = {1, 2, 3, "4", {5,6,7}, {привет中国world={中国="привет"
 field3 = {[10] = 100, k1 = 100, k2 = {1,2,3}, k3 = { {a=1, b=2}, {c=3, d=4} }, [-1] = 200}
 ---
 ...
-t = s:replace{1, field2, field3, "123456", "yes, this"}
+t = s:replace{1, field2, field3, "123456", "yes, this", 100}
 ---
 ...
 t[1]
@@ -805,6 +808,10 @@ t["[3][10]"]
 ---
 - 100
 ...
+t["[1]"]
+---
+- 100
+...
 -- Not found.
 t[0]
 ---
diff --git a/test/engine/tuple.test.lua b/test/engine/tuple.test.lua
index f3997aa19..bfb9a96c1 100644
--- a/test/engine/tuple.test.lua
+++ b/test/engine/tuple.test.lua
@@ -227,11 +227,12 @@ format[2] = {name = 'field2', type = 'array'}
 format[3] = {name = 'field3', type = 'map'}
 format[4] = {name = 'field4', type = 'string' }
 format[5] = {name = "[2][6]['привет中国world']['中国a']", type = 'string'}
+format[6] = {name = '[1]', type = 'unsigned'}
 s = box.schema.space.create('test', {format = format})
 pk = s:create_index('pk')
 field2 = {1, 2, 3, "4", {5,6,7}, {привет中国world={中国="привет"}, key="value1", value="key1"}}
 field3 = {[10] = 100, k1 = 100, k2 = {1,2,3}, k3 = { {a=1, b=2}, {c=3, d=4} }, [-1] = 200}
-t = s:replace{1, field2, field3, "123456", "yes, this"}
+t = s:replace{1, field2, field3, "123456", "yes, this", 100}
 t[1]
 t[2]
 t[3]
@@ -260,6 +261,7 @@ t.field2[5]
 t[".field1"]
 t["field1"]
 t["[3][10]"]
+t["[1]"]
 
 -- Not found.
 t[0]
-- 
2.15.1 (Apple Git-101)