From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com Subject: [PATCH 1/1] json: prefer tuple index as field name instead of json Date: Tue, 24 Apr 2018 23:08:59 +0300 [thread overview] Message-ID: <9fc9b43a9ddb90b95b06f7a5f10c6bb97186029e.1524600459.git.v.shpilevoy@tarantool.org> (raw) 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)
reply other threads:[~2018-04-24 20:08 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=9fc9b43a9ddb90b95b06f7a5f10c6bb97186029e.1524600459.git.v.shpilevoy@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH 1/1] json: prefer tuple index as field name instead of json' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox