Re: [PATCH v6 8/8] box: specify indexes in user-friendly form

[Vladimir Davydov <vdavydov.dev@gmail.com>]

https://www.freelists.org/post/tarantool-patches/PATCH-v5-99-box-specify-indexes-in-userfriendly-form,1

On Mon, Dec 17, 2018 at 09:52:52AM +0300, Kirill Shcherbatov wrote:
> Implemented a more convenient interface for creating an index
> by JSON path. Instead of specifying fieldno and relative path
> it comes possible to pass full JSON path to data.
> 
> Closes #1012
> 
> @TarantoolBot document
> Title: Indexes by JSON path
> Sometimes field data could have complex document structure.
> When this structure is consistent across whole document,
> you are able to create an index by JSON path.
> 
> Example:
> s = box.schema.space.create('sample')
> format = {{'id', 'unsigned'}, {'data', 'map'}}
> s:format(format)
> -- explicit JSON index creation
> age_idx = s:create_index('age', {{2, 'number', path = ".age"}})
> -- user-friendly syntax for JSON index creation
> parts = {{'data.FIO["fname"]', 'str'}, {'data.FIO["sname"]', 'str'},
>          {'data.age', 'number'}}
> info_idx = s:create_index('info', {parts = parts}})
> s:insert({1, {FIO={fname="James", sname="Bond"}, age=35}})
> ---
>  src/box/lua/index.c       | 64 +++++++++++++++++++++++++++++++++++++++
>  src/box/lua/schema.lua    | 22 +++++++-------
>  test/engine/json.result   | 28 +++++++++++++++++
>  test/engine/json.test.lua |  8 +++++
>  4 files changed, 111 insertions(+), 11 deletions(-)
> 
> diff --git a/src/box/lua/index.c b/src/box/lua/index.c
> index 6265c044a..9b04c5d9a 100644
> --- a/src/box/lua/index.c
> +++ b/src/box/lua/index.c
> @@ -35,6 +35,9 @@
>  #include "box/box.h"
>  #include "box/index.h"
>  #include "box/lua/tuple.h"
> +#include "box/schema.h"
> +#include "box/tuple_format.h"
> +#include "json/json.h"
>  #include "box/lua/misc.h" /* lbox_encode_tuple_on_gc() */
>  
>  /** {{{ box.index Lua library: access to spaces and indexes
> @@ -328,6 +331,66 @@ lbox_index_compact(lua_State *L)
>  	return 0;
>  }
>  
> +/**
> + * Resolve field index by absolute JSON path first component and
> + * return relative JSON path.
> + */
> +static int
> +lbox_index_path_resolve(struct lua_State *L)
> +{
> +	if (lua_gettop(L) != 3 ||
> +	    !lua_isnumber(L, 1) || !lua_isnumber(L, 2) || !lua_isstring(L, 3)) {
> +		return luaL_error(L, "Usage box.internal."
> +				     "path_resolve(part_id, space_id, path)");
> +	}
> +	uint32_t part_id = lua_tonumber(L, 1);
> +	uint32_t space_id = lua_tonumber(L, 2);
> +	size_t path_len;
> +	const char *path = lua_tolstring(L, 3, &path_len);
> +	struct space *space = space_cache_find(space_id);
> +	if (space == NULL)
> +		return luaT_error(L);
> +	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) {
> +		const char *err_msg =
> +			tt_sprintf("options.parts[%d]: error in path on "
> +				   "position %d", part_id, rc);
> +		diag_set(ClientError, ER_ILLEGAL_PARAMS, err_msg);
> +		return luaT_error(L);
> +	}
> +	assert(space->format != NULL && space->format->dict != NULL);
> +	uint32_t fieldno = token.num;
> +	uint32_t field_count = tuple_format_field_count(space->format);
> +	if (token.type == JSON_TOKEN_NUM && fieldno >= field_count) {
> +		const char *err_msg =
> +			tt_sprintf("options.parts[%d]: field '%d' referenced "
> +				   "in path is greater than format field "
> +				   "count %d", part_id,
> +				   fieldno + TUPLE_INDEX_BASE, field_count);
> +		diag_set(ClientError, ER_ILLEGAL_PARAMS, err_msg);
> +		return luaT_error(L);
> +	} else if (token.type == JSON_TOKEN_STR &&
> +		   tuple_fieldno_by_name(space->format->dict, token.str,
> +					 token.len,
> +					 field_name_hash(token.str, token.len),
> +					 &fieldno) != 0) {
> +		const char *err_msg =
> +			tt_sprintf("options.parts[%d]: field was not found by "
> +				   "name '%.*s'", part_id, token.len,
> +				   token.str);
> +		diag_set(ClientError, ER_ILLEGAL_PARAMS, err_msg);
> +		return luaT_error(L);
> +	}
> +	fieldno += TUPLE_INDEX_BASE;
> +	path += lexer.offset;
> +	lua_pushnumber(L, fieldno);
> +	lua_pushstring(L, path);
> +	return 2;
> +}
> +
>  /* }}} */
>  
>  void
> @@ -365,6 +428,7 @@ box_lua_index_init(struct lua_State *L)
>  		{"truncate", lbox_truncate},
>  		{"stat", lbox_index_stat},
>  		{"compact", lbox_index_compact},
> +		{"path_resolve", lbox_index_path_resolve},
>  		{NULL, NULL}
>  	};
>  
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index 8a804f0ba..497cf197c 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -575,7 +575,7 @@ local function update_index_parts_1_6_0(parts)
>      return result
>  end
>  
> -local function update_index_parts(format, parts)
> +local function update_index_parts(format, parts, space_id)
>      if type(parts) ~= "table" then
>          box.error(box.error.ILLEGAL_PARAMS,
>          "options.parts parameter should be a table")
> @@ -626,16 +626,16 @@ local function update_index_parts(format, parts)
>              box.error(box.error.ILLEGAL_PARAMS,
>                        "options.parts[" .. i .. "]: field (name or number) is expected")
>          elseif type(part.field) == 'string' then
> -            for k,v in pairs(format) do
> -                if v.name == part.field then
> -                    part.field = k
> -                    break
> -                end
> -            end
> -            if type(part.field) == 'string' then
> +            local idx, path = box.internal.path_resolve(i, space_id, part.field)
> +            if part.path ~= nil and part.path ~= path then
>                  box.error(box.error.ILLEGAL_PARAMS,
> -                          "options.parts[" .. i .. "]: field was not found by name '" .. part.field .. "'")
> +                          "options.parts[" .. i .. "]: field path '"..
> +                          part.path.." doesn't math the path '" ..
> +                          part.field .. "'")
>              end
> +            parts_can_be_simplified = parts_can_be_simplified and path == nil
> +            part.field = idx
> +            part.path = path or part.path
>          elseif part.field == 0 then
>              box.error(box.error.ILLEGAL_PARAMS,
>                        "options.parts[" .. i .. "]: field (number) must be one-based")
> @@ -792,7 +792,7 @@ box.schema.index.create = function(space_id, name, options)
>          end
>      end
>      local parts, parts_can_be_simplified =
> -        update_index_parts(format, options.parts)
> +        update_index_parts(format, options.parts, space_id)
>      -- create_index() options contains type, parts, etc,
>      -- stored separately. Remove these members from index_opts
>      local index_opts = {
> @@ -959,7 +959,7 @@ box.schema.index.alter = function(space_id, index_id, options)
>      if options.parts then
>          local parts_can_be_simplified
>          parts, parts_can_be_simplified =
> -            update_index_parts(format, options.parts)
> +            update_index_parts(format, options.parts, space_id)
>          -- save parts in old format if possible
>          if parts_can_be_simplified then
>              parts = simplify_index_parts(parts)
> diff --git a/test/engine/json.result b/test/engine/json.result
> index c33e568b3..60d71bdfa 100644
> --- a/test/engine/json.result
> +++ b/test/engine/json.result
> @@ -71,6 +71,34 @@ s:create_index('test2', {parts = {{2, 'number'}, {3, 'number', path = '.FIO.fnam
>  - error: Field ["data"]["FIO"]["fname"] has type 'string' in one index, but type 'number'
>      in another
>  ...
> +s:create_index('test3', {parts = {{2, 'number'}, {']sad.FIO["fname"]', 'str'}}})
> +---
> +- error: 'Illegal parameters, options.parts[2]: error in path on position 1'
> +...
> +s:create_index('test3', {parts = {{2, 'number'}, {'[666].FIO["fname"]', 'str'}}})
> +---
> +- error: 'Illegal parameters, options.parts[2]: field ''666'' referenced in path is
> +    greater than format field count 5'
> +...
> +s:create_index('test3', {parts = {{2, 'number'}, {'invalid.FIO["fname"]', 'str'}}})
> +---
> +- error: 'Illegal parameters, options.parts[2]: field was not found by name ''invalid'''
> +...
> +idx3 = s:create_index('test3', {parts = {{2, 'number'}, {'data.FIO["fname"]', 'str'}}})
> +---
> +...
> +assert(idx3 ~= nil)
> +---
> +- true
> +...
> +assert(idx3.parts[2].path == ".FIO[\"fname\"]")
> +---
> +- true
> +...
> +-- Vinyl has optimizations that omit index checks, so errors could differ.
> +idx3:drop()
> +---
> +...
>  s:insert{7, 7, {town = 'London', FIO = 666}, 4, 5}
>  ---
>  - error: 'Tuple field ["data"]["FIO"] type does not match one required by operation:
> diff --git a/test/engine/json.test.lua b/test/engine/json.test.lua
> index 45153743d..f91846d79 100644
> --- a/test/engine/json.test.lua
> +++ b/test/engine/json.test.lua
> @@ -19,6 +19,14 @@ s:format(format)
>  format = {{'id', 'unsigned'}, {'meta', 'unsigned'}, {'data', 'map'}, {'age', 'unsigned'}, {'level', 'unsigned'}}
>  s:format(format)
>  s:create_index('test2', {parts = {{2, 'number'}, {3, 'number', path = '.FIO.fname'}, {3, 'str', path = '["FIO"]["sname"]'}}})
> +s:create_index('test3', {parts = {{2, 'number'}, {']sad.FIO["fname"]', 'str'}}})
> +s:create_index('test3', {parts = {{2, 'number'}, {'[666].FIO["fname"]', 'str'}}})
> +s:create_index('test3', {parts = {{2, 'number'}, {'invalid.FIO["fname"]', 'str'}}})
> +idx3 = s:create_index('test3', {parts = {{2, 'number'}, {'data.FIO["fname"]', 'str'}}})
> +assert(idx3 ~= nil)
> +assert(idx3.parts[2].path == ".FIO[\"fname\"]")
> +-- Vinyl has optimizations that omit index checks, so errors could differ.
> +idx3:drop()
>  s:insert{7, 7, {town = 'London', FIO = 666}, 4, 5}
>  s:insert{7, 7, {town = 'London', FIO = {fname = 666, sname = 'Bond'}}, 4, 5}
>  s:insert{7, 7, {town = 'London', FIO = {fname = "James"}}, 4, 5}
> -- 
> 2.19.2
>