[PATCH v4 03/14] box: introduce tuple_field_by_relative_path
Vladimir Davydov
vdavydov.dev at gmail.com
Mon Oct 15 20:46:33 MSK 2018
On Thu, Oct 11, 2018 at 10:58:48AM +0300, Kirill Shcherbatov wrote:
> The new tuple_field_by_relative_path routine is used in function
> tuple_field_raw_by_path to retrieve data by JSON path from field.
> We need this routine exported in future to access data by JSON
> path specified in key_part.
>
> Part of #1012.
> ---
> src/box/tuple_format.c | 60 ++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 43 insertions(+), 17 deletions(-)
>
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index b385c0d..5679cad 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -541,6 +541,42 @@ tuple_field_go_to_key(const char **field, const char *key, int len)
> return -1;
> }
>
> +/**
> + * Retrieve field data by JSON path.
> + * @param field 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 inline int
> +tuple_field_by_relative_path(const char **field, const char *path,
> + uint32_t path_len)
I kinda dislike the function name. See, we have tuple_field_by_path,
which takes a tuple struct, and tuple_field_by_relative_path, which
takes raw msgpack. Please think of a better name so that everything
looks consistent.
Also, this function doesn't (and won't looking at the further patches)
look up tuple_field/offset_slot, which is another unsettiling difference
from tuple_field_by_path, despite similar names. May be, it should
descend the JSON tree as well?
> +{
> + int rc;
> + struct json_path_parser parser;
> + struct json_path_node node;
> + json_path_parser_create(&parser, path, path_len);
I don't like that you create a parser even if this function is called
from tuple_field_raw_by_path: you could pass the parser created by the
latter. I mean, you'd have two functions, one takes a relative path,
it's going to be public. Another takes a parser. The latter would be
called by the former and by tuple_field_raw_by_path.
> + while ((rc = json_path_next(&parser, &node)) == 0) {
> + switch (node.type) {
> + case JSON_PATH_NUM:
> + rc = tuple_field_go_to_index(field, node.num);
> + break;
> + case JSON_PATH_STR:
> + rc = tuple_field_go_to_key(field, node.str, node.len);
> + break;
> + default:
> + assert(node.type == JSON_PATH_END);
> + return 0;
> + }
> + if (rc != 0) {
> + *field = NULL;
> + return 0;
> + }
> + }
> + return rc;
> +}
More information about the Tarantool-patches
mailing list