From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Kirill Shcherbatov Subject: [PATCH v8 2/5] box: refactor tuple_field_raw_by_path routine Date: Wed, 16 Jan 2019 16:44:40 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: tarantool-patches@freelists.org, vdavydov.dev@gmail.com Cc: kostja@tarantool.org, Kirill Shcherbatov List-ID: Refactored tuple_field_raw_by_path routine as function tuple_field_raw_by_full_path returning field data if the field exists or NULL, instead of error index in path. The *_by_full_path is a better name for routine because we are going to introduce *_by_path routine in future that would work with relative paths. const char * returning valiue is more common result type for such routines. Got rid of invalid path error handling in routine lbox_tuple_field_by_path. Needed for #1012 --- src/box/lua/tuple.c | 9 ++---- src/box/tuple.h | 51 +++++++++++++++------------------ src/box/tuple_format.c | 61 +++++++++++----------------------------- src/box/tuple_format.h | 53 ++++++++++++++++++---------------- test/engine/tuple.result | 16 +++++------ 5 files changed, 79 insertions(+), 111 deletions(-) diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c index 1867f810f..7d377b69e 100644 --- a/src/box/lua/tuple.c +++ b/src/box/lua/tuple.c @@ -463,13 +463,10 @@ lbox_tuple_field_by_path(struct lua_State *L) const char *field = NULL, *path = lua_tolstring(L, 2, &len); if (len == 0) return 0; - if (tuple_field_by_path(tuple, path, (uint32_t) len, - lua_hashstring(L, 2), &field) != 0) { - return luaT_error(L); - } else if (field == NULL) { + field = tuple_field_by_full_path(tuple, path, (uint32_t)len, + lua_hashstring(L, 2)); + if (field == NULL) return 0; - } - assert(field != NULL); luamp_decode(L, luaL_msgpack_default, &field); return 1; } diff --git a/src/box/tuple.h b/src/box/tuple.h index 83e5b7013..4368dac4e 100644 --- a/src/box/tuple.h +++ b/src/box/tuple.h @@ -513,6 +513,23 @@ tuple_field(const struct tuple *tuple, uint32_t fieldno) tuple_field_map(tuple), fieldno); } +/** + * Get a field refereed by index @part in tuple. + * @param tuple Tuple to get the field from. + * @param part Index part to use. + * @param path Relative JSON path to field data. + * @param path_len The length of the @path. + * @retval Field data if the field exists or NULL. + */ +static inline const char * +tuple_field_by_path(const struct tuple *tuple, uint32_t fieldno, + const char *path, uint32_t path_len) +{ + return tuple_field_raw_by_path(tuple_format(tuple), tuple_data(tuple), + tuple_field_map(tuple), fieldno, + path, path_len); +} + /** * Get a field refereed by index @part in tuple. * @param tuple Tuple to get the field from. @@ -532,38 +549,16 @@ tuple_field_by_part(const struct tuple *tuple, struct key_part *part) * @param path Field JSON path. * @param path_len Length of @a path. * @param path_hash Hash of @a path. - * @param[out] field Found field, or NULL, if not found. - * - * @retval 0 Success. - * @retval -1 Error in JSON path. - */ -static inline int -tuple_field_by_path(const struct tuple *tuple, const char *path, - uint32_t path_len, uint32_t path_hash, - const char **field) -{ - return tuple_field_raw_by_path(tuple_format(tuple), tuple_data(tuple), - tuple_field_map(tuple), path, path_len, - path_hash, field); -} - -/** - * Get tuple field by its name. - * @param tuple Tuple to get field from. - * @param name Field name. - * @param name_len Length of @a name. - * @param name_hash Hash of @a name. * - * @retval not NULL MessagePack field. - * @retval NULL No field with @a name. + * @retval field data if field exists or NULL */ static inline const char * -tuple_field_by_name(const struct tuple *tuple, const char *name, - uint32_t name_len, uint32_t name_hash) +tuple_field_by_full_path(const struct tuple *tuple, const char *path, + uint32_t path_len, uint32_t path_hash) { - return tuple_field_raw_by_name(tuple_format(tuple), tuple_data(tuple), - tuple_field_map(tuple), name, name_len, - name_hash); + return tuple_field_raw_by_full_path(tuple_format(tuple), tuple_data(tuple), + tuple_field_map(tuple), + path, path_len, path_hash); } /** diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c index e11b4e6f3..61b4c9734 100644 --- a/src/box/tuple_format.c +++ b/src/box/tuple_format.c @@ -713,15 +713,7 @@ tuple_field_go_to_key(const char **field, const char *key, int len) return -1; } -/** - * Retrieve msgpack data by JSON path. - * @param data Pointer to msgpack with data. - * @param path The path to process. - * @param path_len The length of the @path. - * @retval 0 On success. - * @retval >0 On path parsing error, invalid character position. - */ -static int +int tuple_field_go_to_path(const char **data, const char *path, uint32_t path_len) { int rc; @@ -748,11 +740,10 @@ tuple_field_go_to_path(const char **data, const char *path, uint32_t path_len) return rc; } -int -tuple_field_raw_by_path(struct tuple_format *format, const char *tuple, - const uint32_t *field_map, const char *path, - uint32_t path_len, uint32_t path_hash, - const char **field) +const char * +tuple_field_raw_by_full_path(struct tuple_format *format, const char *tuple, + const uint32_t *field_map, const char *path, + uint32_t path_len, uint32_t path_hash) { assert(path_len > 0); uint32_t fieldno; @@ -763,22 +754,16 @@ tuple_field_raw_by_path(struct tuple_format *format, const char *tuple, * 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; - } + &fieldno) == 0) + return tuple_field_raw(format, tuple, field_map, fieldno); struct json_lexer lexer; struct json_token token; json_lexer_create(&lexer, path, path_len, TUPLE_INDEX_BASE); - int rc = json_lexer_next_token(&lexer, &token); - if (rc != 0) - goto error; + if (json_lexer_next_token(&lexer, &token) != 0) + return NULL; switch(token.type) { case JSON_TOKEN_NUM: { - int index = token.num; - *field = tuple_field_raw(format, tuple, field_map, index); - if (*field == NULL) - return 0; + fieldno = token.num; break; } case JSON_TOKEN_STR: { @@ -795,28 +780,16 @@ tuple_field_raw_by_path(struct tuple_format *format, const char *tuple, */ name_hash = field_name_hash(token.str, token.len); } - *field = tuple_field_raw_by_name(format, tuple, field_map, - token.str, token.len, - name_hash); - if (*field == NULL) - return 0; + if (tuple_fieldno_by_name(format->dict, token.str, token.len, + name_hash, &fieldno) != 0) + return NULL; break; } default: assert(token.type == JSON_TOKEN_END); - *field = NULL; - return 0; + return NULL; } - rc = tuple_field_go_to_path(field, path + lexer.offset, - path_len - lexer.offset); - if (rc == 0) - return 0; - /* Setup absolute error position. */ - rc += lexer.offset; - -error: - assert(rc > 0); - diag_set(ClientError, ER_ILLEGAL_PARAMS, - tt_sprintf("error in path on position %d", rc)); - return -1; + return tuple_field_raw_by_path(format, tuple, field_map, fieldno, + path + lexer.offset, + path_len - lexer.offset); } diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h index 30b93b610..d32bb91f2 100644 --- a/src/box/tuple_format.h +++ b/src/box/tuple_format.h @@ -397,27 +397,33 @@ tuple_field_raw(struct tuple_format *format, const char *tuple, } /** - * Get tuple field by its name. - * @param format Tuple format. - * @param tuple MessagePack tuple's body. - * @param field_map Tuple field map. - * @param name Field name. - * @param name_len Length of @a name. - * @param name_hash Hash of @a name. - * - * @retval not NULL MessagePack field. - * @retval NULL No field with @a name. + * Retrieve msgpack data by JSON path. + * @param data Pointer to msgpack with data. + * @param path The path to process. + * @param path_len The length of the @path. + * @retval 0 On success. + * @retval >0 On path parsing error, invalid character position. + */ +int +tuple_field_go_to_path(const char **data, const char *path, uint32_t path_len); + +/** + * Get a field at the specific position in this MessagePack + * array by fieldno and path. */ static inline const char * -tuple_field_raw_by_name(struct tuple_format *format, const char *tuple, - const uint32_t *field_map, const char *name, - uint32_t name_len, uint32_t name_hash) +tuple_field_raw_by_path(struct tuple_format *format, const char *tuple, + const uint32_t *field_map, uint32_t fieldno, + const char *path, uint32_t path_len) { - uint32_t fieldno; - if (tuple_fieldno_by_name(format->dict, name, name_len, name_hash, - &fieldno) != 0) + tuple = tuple_field_raw(format, tuple, field_map, fieldno); + if (path == NULL) + return tuple; + if (unlikely(tuple == NULL)) return NULL; - return tuple_field_raw(format, tuple, field_map, fieldno); + if (unlikely(tuple_field_go_to_path(&tuple, path, path_len) != 0)) + return NULL; + return tuple; } /** @@ -428,16 +434,13 @@ tuple_field_raw_by_name(struct tuple_format *format, const char *tuple, * @param path Field path. * @param path_len Length of @a path. * @param path_hash Hash of @a path. - * @param[out] field Found field, or NULL, if not found. * - * @retval 0 Success. - * @retval -1 Error in JSON path. + * @retval field data if field exists or NULL */ -int -tuple_field_raw_by_path(struct tuple_format *format, const char *tuple, - const uint32_t *field_map, const char *path, - uint32_t path_len, uint32_t path_hash, - const char **field); +const char * +tuple_field_raw_by_full_path(struct tuple_format *format, const char *tuple, + const uint32_t *field_map, const char *path, + uint32_t path_len, uint32_t path_hash); /** * Get a tuple field pointed to by an index part. diff --git a/test/engine/tuple.result b/test/engine/tuple.result index 7ca3985c7..daf57d1f5 100644 --- a/test/engine/tuple.result +++ b/test/engine/tuple.result @@ -823,7 +823,7 @@ t[0] ... t["[0]"] --- -- error: Illegal parameters, error in path on position 2 +- null ... t["[1000]"] --- @@ -847,7 +847,7 @@ t["[2][6].key100"] ... t["[2][0]"] -- 0-based index in array. --- -- error: Illegal parameters, error in path on position 5 +- null ... t["[4][3]"] -- Can not index string. --- @@ -866,27 +866,27 @@ t["a.b.c d.e.f"] -- Sytax errors. t["[2].[5]"] --- -- error: Illegal parameters, error in path on position 5 +- null ... t["[-1]"] --- -- error: Illegal parameters, error in path on position 2 +- null ... t[".."] --- -- error: Illegal parameters, error in path on position 2 +- null ... t["[["] --- -- error: Illegal parameters, error in path on position 2 +- null ... t["]]"] --- -- error: Illegal parameters, error in path on position 1 +- null ... t["{"] --- -- error: Illegal parameters, error in path on position 1 +- null ... s:drop() --- -- 2.19.2