From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladislav Shpilevoy Subject: [PATCH 1/1] json: prefer tuple index as field name instead of json Date: Tue, 24 Apr 2018 23:08:59 +0300 Message-Id: <9fc9b43a9ddb90b95b06f7a5f10c6bb97186029e.1524600459.git.v.shpilevoy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com List-ID: 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)