From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 18 Dec 2018 23:58:42 +0300 From: Vladimir Davydov Subject: Re: [PATCH v6 8/8] box: specify indexes in user-friendly form Message-ID: <20181218205842.utabeqkxyt2mpl7f@esperanza> References: <0cab9a9f155f9b2b3bf982acd083c4db132ba4a4.1544995259.git.kshcherbatov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0cab9a9f155f9b2b3bf982acd083c4db132ba4a4.1544995259.git.kshcherbatov@tarantool.org> To: Kirill Shcherbatov Cc: tarantool-patches@freelists.org, kostja@tarantool.org List-ID: 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 >