* [tarantool-patches] [PATCH v2 0/2] lua: add key_def lua module @ 2019-03-27 14:29 Kirill Shcherbatov 2019-03-27 14:29 ` [tarantool-patches] [PATCH v2 1/2] lua: add luaT_tuple_new() Kirill Shcherbatov 2019-03-27 14:29 ` [tarantool-patches] [PATCH v2 2/2] lua: add key_def lua module Kirill Shcherbatov 0 siblings, 2 replies; 25+ messages in thread From: Kirill Shcherbatov @ 2019-03-27 14:29 UTC (permalink / raw) To: tarantool-patches, alexander.turenko; +Cc: Kirill Shcherbatov Introduced new LUA module key_def. There are several reasons to add this module: * Factor out key parts parsing code from the tuples merger (#3276). * Support comparing a tuple with a key / a tuple, support merging key_defs from Lua (#3398). * Support extracting a key from a tuple (#4025). Changes in version 2: - ability to pass tables, not only tuples as arguments - perform types and nullablility validations - fixed errors http://github.com/tarantool/tarantool/tree/kshch/gh-4025-lua-key-kef-methods https://github.com/tarantool/tarantool/issues/4025 Alexander Turenko (1): lua: add luaT_tuple_new() Kirill Shcherbatov (1): lua: add key_def lua module src/CMakeLists.txt | 1 + src/box/CMakeLists.txt | 2 + src/box/lua/init.c | 5 + src/box/lua/key_def.c | 441 ++++++++++++++++++++++++++++++++ src/box/lua/key_def.h | 63 +++++ src/box/lua/key_def.lua | 19 ++ src/box/lua/space.cc | 35 +-- src/box/lua/tuple.c | 58 +++-- src/box/lua/tuple.h | 15 +- src/box/tuple.h | 33 +++ test/box-tap/key_def.test.lua | 340 ++++++++++++++++++++++++ test/unit/CMakeLists.txt | 4 + test/unit/luaT_tuple_new.c | 178 +++++++++++++ test/unit/luaT_tuple_new.result | 22 ++ 14 files changed, 1167 insertions(+), 49 deletions(-) create mode 100644 src/box/lua/key_def.c create mode 100644 src/box/lua/key_def.h create mode 100644 src/box/lua/key_def.lua create mode 100755 test/box-tap/key_def.test.lua create mode 100644 test/unit/luaT_tuple_new.c create mode 100644 test/unit/luaT_tuple_new.result -- 2.21.0 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tarantool-patches] [PATCH v2 1/2] lua: add luaT_tuple_new() 2019-03-27 14:29 [tarantool-patches] [PATCH v2 0/2] lua: add key_def lua module Kirill Shcherbatov @ 2019-03-27 14:29 ` Kirill Shcherbatov 2019-03-28 9:01 ` [tarantool-patches] " Konstantin Osipov 2019-04-03 18:01 ` [tarantool-patches] " Vladimir Davydov 2019-03-27 14:29 ` [tarantool-patches] [PATCH v2 2/2] lua: add key_def lua module Kirill Shcherbatov 1 sibling, 2 replies; 25+ messages in thread From: Kirill Shcherbatov @ 2019-03-27 14:29 UTC (permalink / raw) To: tarantool-patches, alexander.turenko From: Alexander Turenko <alexander.turenko@tarantool.org> The function allows to create a tuple with specific tuple format in C code using a Lua table, another tuple, or objects on a Lua stack. Needed for #3276, #3398, #4025 --- src/box/lua/space.cc | 7 +- src/box/lua/tuple.c | 58 +++++++---- src/box/lua/tuple.h | 15 ++- test/unit/CMakeLists.txt | 4 + test/unit/luaT_tuple_new.c | 178 ++++++++++++++++++++++++++++++++ test/unit/luaT_tuple_new.result | 22 ++++ 6 files changed, 261 insertions(+), 23 deletions(-) create mode 100644 test/unit/luaT_tuple_new.c create mode 100644 test/unit/luaT_tuple_new.result diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc index 9dfc97b6a..ca793e423 100644 --- a/src/box/lua/space.cc +++ b/src/box/lua/space.cc @@ -463,6 +463,7 @@ lbox_space_frommap(struct lua_State *L) struct tuple_dictionary *dict = NULL; uint32_t id = 0; struct space *space = NULL; + struct tuple *tuple = NULL; int argc = lua_gettop(L); bool table = false; if (argc < 2 || argc > 3 || !lua_istable(L, 2)) @@ -510,7 +511,11 @@ lbox_space_frommap(struct lua_State *L) lua_replace(L, 1); lua_settop(L, 1); - return luaT_tuple_new(L, space->format); + tuple = luaT_tuple_new(L, -1, space->format); + if (tuple == NULL) + return luaT_error(L); + luaT_pushtuple(L, tuple); + return 1; usage_error: return luaL_error(L, "Usage: space:frommap(map, opts)"); } diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c index 60c1a3999..3f4e3dbf7 100644 --- a/src/box/lua/tuple.c +++ b/src/box/lua/tuple.c @@ -93,47 +93,65 @@ luaT_istuple(struct lua_State *L, int narg) return *(struct tuple **) data; } -int -luaT_tuple_new(struct lua_State *L, struct tuple_format *format) +struct tuple * +luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format) { - int argc = lua_gettop(L); - if (argc < 1) { - lua_newtable(L); /* create an empty tuple */ - ++argc; + if (idx != 0 && !lua_istable(L, idx) && !luaT_istuple(L, idx)) { + diag_set(IllegalParams, "A tuple or a table expected, got %s", + lua_typename(L, lua_type(L, idx))); + return NULL; } - struct ibuf *buf = tarantool_lua_ibuf; + struct ibuf *buf = tarantool_lua_ibuf; ibuf_reset(buf); struct mpstream stream; mpstream_init(&stream, buf, ibuf_reserve_cb, ibuf_alloc_cb, luamp_error, L); - - if (argc == 1 && (lua_istable(L, 1) || luaT_istuple(L, 1))) { - /* New format: box.tuple.new({1, 2, 3}) */ - luamp_encode_tuple(L, &tuple_serializer, &stream, 1); - } else { - /* Backward-compatible format: box.tuple.new(1, 2, 3). */ + if (idx == 0) { + /* + * Create the tuple from lua stack + * objects. + */ + int argc = lua_gettop(L); mpstream_encode_array(&stream, argc); for (int k = 1; k <= argc; ++k) { luamp_encode(L, luaL_msgpack_default, &stream, k); } + } else { + /* Create the tuple from a Lua table. */ + luamp_encode_tuple(L, &tuple_serializer, &stream, idx); } mpstream_flush(&stream); - struct tuple *tuple = box_tuple_new(format, buf->buf, - buf->buf + ibuf_used(buf)); + buf->buf + ibuf_used(buf)); if (tuple == NULL) - return luaT_error(L); - /* box_tuple_new() doesn't leak on exception, see public API doc */ - luaT_pushtuple(L, tuple); + return NULL; ibuf_reinit(tarantool_lua_ibuf); - return 1; + return tuple; } static int lbox_tuple_new(lua_State *L) { - return luaT_tuple_new(L, box_tuple_format_default()); + int argc = lua_gettop(L); + if (argc < 1) { + lua_newtable(L); /* create an empty tuple */ + ++argc; + } + /* + * Use backward-compatible parameters format: + * box.tuple.new(1, 2, 3) (idx == 0), or the new one: + * box.tuple.new({1, 2, 3}) (idx == 1). + */ + int idx = argc == 1 && (lua_istable(L, 1) || + luaT_istuple(L, 1)); + box_tuple_format_t *fmt = box_tuple_format_default(); + struct tuple *tuple = luaT_tuple_new(L, idx, fmt); + if (tuple == NULL) + return luaT_error(L); + /* box_tuple_new() doesn't leak on exception, see public API doc */ + luaT_pushtuple(L, tuple); + return 1; } static int diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h index ba7f01f4b..dd3aa0f08 100644 --- a/src/box/lua/tuple.h +++ b/src/box/lua/tuple.h @@ -42,6 +42,8 @@ typedef struct tuple box_tuple_t; struct lua_State; struct mpstream; struct luaL_serializer; +struct tuple_format; +typedef struct tuple_format box_tuple_format_t; /** \cond public */ @@ -67,8 +69,17 @@ luaT_istuple(struct lua_State *L, int idx); /** \endcond public */ -int -luaT_tuple_new(struct lua_State *L, struct tuple_format *format); +/** + * Create a new tuple with specific format from a Lua table, a + * tuple, or objects on the lua stack. + * + * Set idx to zero to create the new tuple from objects on the lua + * stack. + * + * In case of an error set a diag and return NULL. + */ +struct tuple * +luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format); static inline int luaT_pushtupleornil(struct lua_State *L, struct tuple *tuple) diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt index 6d5704349..2bcb6e0a8 100644 --- a/test/unit/CMakeLists.txt +++ b/test/unit/CMakeLists.txt @@ -140,6 +140,10 @@ add_executable(histogram.test histogram.c) target_link_libraries(histogram.test stat unit) add_executable(ratelimit.test ratelimit.c) target_link_libraries(ratelimit.test unit) +add_executable(luaT_tuple_new.test luaT_tuple_new.c) +target_link_libraries(luaT_tuple_new.test unit box server core misc + ${CURL_LIBRARIES} ${LIBYAML_LIBRARIES} ${READLINE_LIBRARIES} + ${ICU_LIBRARIES} ${LUAJIT_LIBRARIES}) add_executable(say.test say.c) target_link_libraries(say.test core unit) diff --git a/test/unit/luaT_tuple_new.c b/test/unit/luaT_tuple_new.c new file mode 100644 index 000000000..3486ed5d3 --- /dev/null +++ b/test/unit/luaT_tuple_new.c @@ -0,0 +1,178 @@ +#include <string.h> /* strncmp() */ +#include <lua.h> /* lua_*() */ +#include <lauxlib.h> /* luaL_*() */ +#include <lualib.h> /* luaL_openlibs() */ +#include "unit.h" /* plan, header, footer, is, ok */ +#include "memory.h" /* memory_init() */ +#include "fiber.h" /* fiber_init() */ +#include "small/ibuf.h" /* struct ibuf */ +#include "box/box.h" /* box_init() */ +#include "box/tuple.h" /* box_tuple_format_default() */ +#include "lua/msgpack.h" /* luaopen_msgpack() */ +#include "box/lua/tuple.h" /* luaT_tuple_new() */ +#include "diag.h" /* struct error, diag_*() */ +#include "exception.h" /* type_IllegalParams */ + +/* + * This test checks all usage cases of luaT_tuple_new(): + * + * * Use with idx == 0 and idx != 0. + * * Use with default and non-default formats. + * * Use a table and a tuple as an input. + * * Use with an unexpected lua type as an input. + * + * The test does not vary an input table/tuple. This is done in + * box/tuple.test.lua. + */ + +extern struct ibuf *tarantool_lua_ibuf; + +uint32_t +min_u32(uint32_t a, uint32_t b) +{ + return a < b ? a : b; +} + +void +check_tuple(const struct tuple *tuple, box_tuple_format_t *format, + int retvals, const char *case_name) +{ + uint32_t size; + const char *data = tuple_data_range(tuple, &size); + + ok(tuple != NULL, "%s: tuple != NULL", case_name); + is(tuple->format_id, tuple_format_id(format), + "%s: check tuple format id", case_name); + is(size, 4, "%s: check tuple size", case_name); + ok(!strncmp(data, "\x93\x01\x02\x03", min_u32(size, 4)), + "%s: check tuple data", case_name); + is(retvals, 0, "%s: check retvals count", case_name); +} + +void +check_error(struct lua_State *L, const struct tuple *tuple, int retvals, + const char *case_name) +{ + const char *exp_err = "A tuple or a table expected, got number"; + is(tuple, NULL, "%s: tuple == NULL", case_name); + is(retvals, 0, "%s: check retvals count", case_name); + struct error *e = diag_last_error(diag_get()); + is(e->type, &type_IllegalParams, "%s: check error type", case_name); + ok(!strcmp(e->errmsg, exp_err), "%s: check error message", case_name); +} + +int +test_basic(struct lua_State *L) +{ + plan(19); + header(); + + int top; + struct tuple *tuple; + box_tuple_format_t *default_format = box_tuple_format_default(); + + /* + * Case: a Lua table on idx == -2 as an input. + */ + + /* Prepare the Lua stack. */ + luaL_loadstring(L, "return {1, 2, 3}"); + lua_call(L, 0, 1); + lua_pushnil(L); + + /* Create and check a tuple. */ + top = lua_gettop(L); + tuple = luaT_tuple_new(L, -2, default_format); + check_tuple(tuple, default_format, lua_gettop(L) - top, "table"); + + /* Clean up. */ + lua_pop(L, 2); + assert(lua_gettop(L) == 0); + + /* + * Case: a tuple on idx == -1 as an input. + */ + + /* Prepare the Lua stack. */ + luaT_pushtuple(L, tuple); + + /* Create and check a tuple. */ + top = lua_gettop(L); + tuple = luaT_tuple_new(L, -1, default_format); + check_tuple(tuple, default_format, lua_gettop(L) - top, "tuple"); + + /* Clean up. */ + lua_pop(L, 1); + assert(lua_gettop(L) == 0); + + /* + * Case: elements on the stack (idx == 0) as an input and + * a non-default format. + */ + + /* Prepare the Lua stack. */ + lua_pushinteger(L, 1); + lua_pushinteger(L, 2); + lua_pushinteger(L, 3); + + /* Create a new format. */ + struct key_part_def part; + part.fieldno = 0; + part.type = FIELD_TYPE_INTEGER; + part.coll_id = COLL_NONE; + part.is_nullable = false; + part.nullable_action = ON_CONFLICT_ACTION_DEFAULT; + part.sort_order = SORT_ORDER_ASC; + part.path = NULL; + struct key_def *key_def = key_def_new(&part, 1); + box_tuple_format_t *another_format = box_tuple_format_new(&key_def, 1); + key_def_delete(key_def); + + /* Create and check a tuple. */ + top = lua_gettop(L); + tuple = luaT_tuple_new(L, 0, another_format); + check_tuple(tuple, another_format, lua_gettop(L) - top, "objects"); + + /* Clean up. */ + tuple_format_delete(another_format); + lua_pop(L, 3); + assert(lua_gettop(L) == 0); + + /* + * Case: a lua object of an unexpected type. + */ + + /* Prepare the Lua stack. */ + lua_pushinteger(L, 42); + + /* Try to create and check for the error. */ + top = lua_gettop(L); + tuple = luaT_tuple_new(L, -1, default_format); + check_error(L, tuple, lua_gettop(L) - top, "unexpected type"); + + /* Clean up. */ + lua_pop(L, 1); + assert(lua_gettop(L) == 0); + + footer(); + return check_plan(); +} + +int +main() +{ + memory_init(); + fiber_init(fiber_c_invoke); + + ibuf_create(tarantool_lua_ibuf, &cord()->slabc, 16000); + + struct lua_State *L = luaL_newstate(); + luaL_openlibs(L); + + box_init(); + box_lua_tuple_init(L); + luaopen_msgpack(L); + lua_pop(L, 1); + + return test_basic(L); +} diff --git a/test/unit/luaT_tuple_new.result b/test/unit/luaT_tuple_new.result new file mode 100644 index 000000000..110aa68c2 --- /dev/null +++ b/test/unit/luaT_tuple_new.result @@ -0,0 +1,22 @@ +1..19 + *** test_basic *** +ok 1 - table: tuple != NULL +ok 2 - table: check tuple format id +ok 3 - table: check tuple size +ok 4 - table: check tuple data +ok 5 - table: check retvals count +ok 6 - tuple: tuple != NULL +ok 7 - tuple: check tuple format id +ok 8 - tuple: check tuple size +ok 9 - tuple: check tuple data +ok 10 - tuple: check retvals count +ok 11 - objects: tuple != NULL +ok 12 - objects: check tuple format id +ok 13 - objects: check tuple size +ok 14 - objects: check tuple data +ok 15 - objects: check retvals count +ok 16 - unexpected type: tuple == NULL +ok 17 - unexpected type: check retvals count +ok 18 - unexpected type: check error type +ok 19 - unexpected type: check error message + *** test_basic: done *** -- 2.21.0 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/2] lua: add luaT_tuple_new() 2019-03-27 14:29 ` [tarantool-patches] [PATCH v2 1/2] lua: add luaT_tuple_new() Kirill Shcherbatov @ 2019-03-28 9:01 ` Konstantin Osipov 2019-03-28 9:18 ` Alexander Turenko 2019-04-03 18:01 ` [tarantool-patches] " Vladimir Davydov 1 sibling, 1 reply; 25+ messages in thread From: Konstantin Osipov @ 2019-03-28 9:01 UTC (permalink / raw) To: tarantool-patches; +Cc: alexander.turenko * Kirill Shcherbatov <kshcherbatov@tarantool.org> [19/03/27 17:30]: Wow, looks like this breaks compatibility and has no ticket in itself. Please file a ticket describing the new features and incompatible changes so that we can settle on public api changes before committing to the implementation. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/2] lua: add luaT_tuple_new() 2019-03-28 9:01 ` [tarantool-patches] " Konstantin Osipov @ 2019-03-28 9:18 ` Alexander Turenko 0 siblings, 0 replies; 25+ messages in thread From: Alexander Turenko @ 2019-03-28 9:18 UTC (permalink / raw) To: Konstantin Osipov; +Cc: tarantool-patches On Thu, Mar 28, 2019 at 12:01:29PM +0300, Konstantin Osipov wrote: > * Kirill Shcherbatov <kshcherbatov@tarantool.org> [19/03/27 17:30]: > > Wow, looks like this breaks compatibility and has no ticket in > itself. > > Please file a ticket describing the new features and incompatible > changes so that we can settle on public api changes before > committing to the implementation. > It is luaT_tuple_new(), not box_tuple_new(). It is not a part of the public API (I mean we don't export this function). > > -- > Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 > http://tarantool.io - www.twitter.com/kostja_osipov ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] [PATCH v2 1/2] lua: add luaT_tuple_new() 2019-03-27 14:29 ` [tarantool-patches] [PATCH v2 1/2] lua: add luaT_tuple_new() Kirill Shcherbatov 2019-03-28 9:01 ` [tarantool-patches] " Konstantin Osipov @ 2019-04-03 18:01 ` Vladimir Davydov 2019-04-04 2:51 ` Alexander Turenko 1 sibling, 1 reply; 25+ messages in thread From: Vladimir Davydov @ 2019-04-03 18:01 UTC (permalink / raw) To: Kirill Shcherbatov; +Cc: tarantool-patches, alexander.turenko On Wed, Mar 27, 2019 at 05:29:27PM +0300, Kirill Shcherbatov wrote: > From: Alexander Turenko <alexander.turenko@tarantool.org> > > The function allows to create a tuple with specific tuple format in C > code using a Lua table, another tuple, or objects on a Lua stack. > > Needed for #3276, #3398, #4025 > --- > src/box/lua/space.cc | 7 +- > src/box/lua/tuple.c | 58 +++++++---- > src/box/lua/tuple.h | 15 ++- > test/unit/CMakeLists.txt | 4 + > test/unit/luaT_tuple_new.c | 178 ++++++++++++++++++++++++++++++++ > test/unit/luaT_tuple_new.result | 22 ++++ > 6 files changed, 261 insertions(+), 23 deletions(-) > create mode 100644 test/unit/luaT_tuple_new.c > create mode 100644 test/unit/luaT_tuple_new.result Pushed to master. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] [PATCH v2 1/2] lua: add luaT_tuple_new() 2019-04-03 18:01 ` [tarantool-patches] " Vladimir Davydov @ 2019-04-04 2:51 ` Alexander Turenko 2019-04-04 8:14 ` Vladimir Davydov 0 siblings, 1 reply; 25+ messages in thread From: Alexander Turenko @ 2019-04-04 2:51 UTC (permalink / raw) To: Vladimir Davydov; +Cc: Kirill Shcherbatov, tarantool-patches On Wed, Apr 03, 2019 at 09:01:23PM +0300, Vladimir Davydov wrote: > On Wed, Mar 27, 2019 at 05:29:27PM +0300, Kirill Shcherbatov wrote: > > From: Alexander Turenko <alexander.turenko@tarantool.org> > > > > The function allows to create a tuple with specific tuple format in C > > code using a Lua table, another tuple, or objects on a Lua stack. > > > > Needed for #3276, #3398, #4025 > > --- > > src/box/lua/space.cc | 7 +- > > src/box/lua/tuple.c | 58 +++++++---- > > src/box/lua/tuple.h | 15 ++- > > test/unit/CMakeLists.txt | 4 + > > test/unit/luaT_tuple_new.c | 178 ++++++++++++++++++++++++++++++++ > > test/unit/luaT_tuple_new.result | 22 ++++ > > 6 files changed, 261 insertions(+), 23 deletions(-) > > create mode 100644 test/unit/luaT_tuple_new.c > > create mode 100644 test/unit/luaT_tuple_new.result > > Pushed to master. Thanks! Are there any problems with pushing to 2.1 and 1.10? I need this for merger, which is planned to land into 1.10. WBR, Alexander Turenko. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] [PATCH v2 1/2] lua: add luaT_tuple_new() 2019-04-04 2:51 ` Alexander Turenko @ 2019-04-04 8:14 ` Vladimir Davydov 0 siblings, 0 replies; 25+ messages in thread From: Vladimir Davydov @ 2019-04-04 8:14 UTC (permalink / raw) To: Alexander Turenko; +Cc: Kirill Shcherbatov, tarantool-patches On Thu, Apr 04, 2019 at 05:51:22AM +0300, Alexander Turenko wrote: > On Wed, Apr 03, 2019 at 09:01:23PM +0300, Vladimir Davydov wrote: > > On Wed, Mar 27, 2019 at 05:29:27PM +0300, Kirill Shcherbatov wrote: > > > From: Alexander Turenko <alexander.turenko@tarantool.org> > > > > > > The function allows to create a tuple with specific tuple format in C > > > code using a Lua table, another tuple, or objects on a Lua stack. > > > > > > Needed for #3276, #3398, #4025 > > > --- > > > src/box/lua/space.cc | 7 +- > > > src/box/lua/tuple.c | 58 +++++++---- > > > src/box/lua/tuple.h | 15 ++- > > > test/unit/CMakeLists.txt | 4 + > > > test/unit/luaT_tuple_new.c | 178 ++++++++++++++++++++++++++++++++ > > > test/unit/luaT_tuple_new.result | 22 ++++ > > > 6 files changed, 261 insertions(+), 23 deletions(-) > > > create mode 100644 test/unit/luaT_tuple_new.c > > > create mode 100644 test/unit/luaT_tuple_new.result > > > > Pushed to master. > > Thanks! You're welcome) > > Are there any problems with pushing to 2.1 and 1.10? I need this for > merger, which is planned to land into 1.10. Let's think about backporting this patch to 2.1 and 1.10 after we push the merger to master. ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tarantool-patches] [PATCH v2 2/2] lua: add key_def lua module 2019-03-27 14:29 [tarantool-patches] [PATCH v2 0/2] lua: add key_def lua module Kirill Shcherbatov 2019-03-27 14:29 ` [tarantool-patches] [PATCH v2 1/2] lua: add luaT_tuple_new() Kirill Shcherbatov @ 2019-03-27 14:29 ` Kirill Shcherbatov 2019-03-28 2:01 ` Alexander Turenko 1 sibling, 1 reply; 25+ messages in thread From: Kirill Shcherbatov @ 2019-03-27 14:29 UTC (permalink / raw) To: tarantool-patches, alexander.turenko; +Cc: Kirill Shcherbatov There are several reasons to add this module: * Factor out key parts parsing code from the tuples merger (#3276). * Support comparing a tuple with a key / a tuple, support merging key_defs from Lua (#3398). * Support extracting a key from a tuple (#4025). The format of `parts` parameter in the `key_def.new(parts)` call is compatible with the following structures: * box.space[...].index[...].parts; * net_box_conn.space[...].index[...].parts. A key_def instance has the following methods: * :extract_key(tuple) -> key (as tuple) * :compare(tuple_a, tuple_b) -> number * :compare_with_key(tuple, key) -> number * :merge(another_key_def) -> new key_def instance * :totable() -> table Note for compare_with_key(): `key` is tuple or table. Needed for #3276. Fixes #3398. Fixes #4025. @TarantoolBot document Title: lua: key_def module See the commit message for an API reference. Example for extract_key(): ```lua -- Remove values got from a secondary non-unique index. local key_def_lib = require('key_def') local s = box.schema.space.create('test') local pk = s:create_index('pk') local sk = s:create_index('test', {unique = false, parts = { {2, 'number', path = 'a'}, {2, 'number', path = 'b'}}}) s:insert{1, {a = 1, b = 1}} s:insert{2, {a = 1, b = 2}} local key_def = key_def_lib.new(pk.parts) for _, tuple in sk:pairs({1})) do local key = key_def:extract_key(tuple) pk:delete(key) end ``` --- src/CMakeLists.txt | 1 + src/box/CMakeLists.txt | 2 + src/box/lua/init.c | 5 + src/box/lua/key_def.c | 441 ++++++++++++++++++++++++++++++++++ src/box/lua/key_def.h | 63 +++++ src/box/lua/key_def.lua | 19 ++ src/box/lua/space.cc | 28 +-- src/box/tuple.h | 33 +++ test/box-tap/key_def.test.lua | 340 ++++++++++++++++++++++++++ 9 files changed, 906 insertions(+), 26 deletions(-) create mode 100644 src/box/lua/key_def.c create mode 100644 src/box/lua/key_def.h create mode 100644 src/box/lua/key_def.lua create mode 100755 test/box-tap/key_def.test.lua diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7c2395517..a6a18142b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -136,6 +136,7 @@ set(api_headers ${CMAKE_SOURCE_DIR}/src/lua/string.h ${CMAKE_SOURCE_DIR}/src/box/txn.h ${CMAKE_SOURCE_DIR}/src/box/key_def.h + ${CMAKE_SOURCE_DIR}/src/box/lua/key_def.h ${CMAKE_SOURCE_DIR}/src/box/field_def.h ${CMAKE_SOURCE_DIR}/src/box/tuple.h ${CMAKE_SOURCE_DIR}/src/box/tuple_format.h diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt index 59e91b65a..906ce22e6 100644 --- a/src/box/CMakeLists.txt +++ b/src/box/CMakeLists.txt @@ -12,6 +12,7 @@ lua_source(lua_sources lua/net_box.lua) lua_source(lua_sources lua/upgrade.lua) lua_source(lua_sources lua/console.lua) lua_source(lua_sources lua/xlog.lua) +lua_source(lua_sources lua/key_def.lua) set(bin_sources) bin_source(bin_sources bootstrap.snap bootstrap.h) @@ -139,6 +140,7 @@ add_library(box STATIC lua/net_box.c lua/xlog.c lua/sql.c + lua/key_def.c ${bin_sources}) target_link_libraries(box box_error tuple stat xrow xlog vclock crc32 scramble diff --git a/src/box/lua/init.c b/src/box/lua/init.c index 744b2c895..68ef58909 100644 --- a/src/box/lua/init.c +++ b/src/box/lua/init.c @@ -59,9 +59,11 @@ #include "box/lua/console.h" #include "box/lua/tuple.h" #include "box/lua/sql.h" +#include "box/lua/key_def.h" extern char session_lua[], tuple_lua[], + key_def_lua[], schema_lua[], load_cfg_lua[], xlog_lua[], @@ -80,6 +82,7 @@ static const char *lua_sources[] = { "box/console", console_lua, "box/load_cfg", load_cfg_lua, "box/xlog", xlog_lua, + "box/key_def", key_def_lua, NULL }; @@ -312,6 +315,8 @@ box_lua_init(struct lua_State *L) lua_pop(L, 1); tarantool_lua_console_init(L); lua_pop(L, 1); + luaopen_key_def(L); + lua_pop(L, 1); /* Load Lua extension */ for (const char **s = lua_sources; *s; s += 2) { diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c new file mode 100644 index 000000000..18d5d961d --- /dev/null +++ b/src/box/lua/key_def.c @@ -0,0 +1,441 @@ +/* + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include "box/lua/key_def.h" + +#include <lua.h> +#include <lauxlib.h> +#include "fiber.h" +#include "diag.h" +#include "tuple.h" +#include "box/key_def.h" +#include "box/box.h" +#include "box/tuple.h" +#include "box/coll_id_cache.h" +#include "lua/utils.h" +#include "box/tuple_format.h" + +static uint32_t key_def_type_id = 0; + +/** + * Set key_part_def from a table on top of a Lua stack. + * + * When successful return 0, otherwise return -1 and set a diag. + */ +static int +luaT_key_def_set_part(struct lua_State *L, struct key_part_def *parts, + int part_idx, struct region *region) +{ + struct key_part_def *part = &parts[part_idx]; + *part = key_part_def_default; + + /* Set part->fieldno. */ + lua_pushstring(L, "fieldno"); + lua_gettable(L, -2); + if (lua_isnil(L, -1)) { + diag_set(IllegalParams, "fieldno must not be nil"); + return -1; + } + /* + * Transform one-based Lua fieldno to zero-based + * fieldno to use in key_def_new(). + */ + part->fieldno = lua_tointeger(L, -1) - TUPLE_INDEX_BASE; + lua_pop(L, 1); + + /* Set part->type. */ + lua_pushstring(L, "type"); + lua_gettable(L, -2); + if (lua_isnil(L, -1)) { + diag_set(IllegalParams, "type must not be nil"); + return -1; + } + size_t type_len; + const char *type_name = lua_tolstring(L, -1, &type_len); + lua_pop(L, 1); + part->type = field_type_by_name(type_name, type_len); + switch (part->type) { + case FIELD_TYPE_ANY: + case FIELD_TYPE_ARRAY: + case FIELD_TYPE_MAP: + /* Tuple comparators don't support these types. */ + diag_set(IllegalParams, "Unsupported field type: %s", + type_name); + return -1; + case field_type_MAX: + diag_set(IllegalParams, "Unknown field type: %s", type_name); + return -1; + default: + /* Pass though. */ + break; + } + + /* Set part->is_nullable and part->nullable_action. */ + lua_pushstring(L, "is_nullable"); + lua_gettable(L, -2); + if (!lua_isnil(L, -1) && lua_toboolean(L, -1) != 0) { + part->is_nullable = true; + part->nullable_action = ON_CONFLICT_ACTION_NONE; + } + lua_pop(L, 1); + + /* + * Set part->coll_id using collation_id. + * + * The value will be checked in key_def_new(). + */ + lua_pushstring(L, "collation_id"); + lua_gettable(L, -2); + if (!lua_isnil(L, -1)) + part->coll_id = lua_tointeger(L, -1); + lua_pop(L, 1); + + /* Set part->coll_id using collation. */ + lua_pushstring(L, "collation"); + lua_gettable(L, -2); + if (!lua_isnil(L, -1)) { + /* Check for conflicting options. */ + if (part->coll_id != COLL_NONE) { + diag_set(IllegalParams, "Conflicting options: " + "collation_id and collation"); + return -1; + } + + size_t coll_name_len; + const char *coll_name = lua_tolstring(L, -1, &coll_name_len); + struct coll_id *coll_id = coll_by_name(coll_name, + coll_name_len); + if (coll_id == NULL) { + diag_set(IllegalParams, "Unknown collation: \"%s\"", + coll_name); + return -1; + } + part->coll_id = coll_id->id; + } + lua_pop(L, 1); + + /* Set part->path (JSON path). */ + lua_pushstring(L, "path"); + lua_gettable(L, -2); + if (!lua_isnil(L, -1)) { + size_t path_len; + const char *path = lua_tolstring(L, -1, &path_len); + if (json_path_validate(path, path_len, TUPLE_INDEX_BASE) != 0) { + diag_set(ClientError, ER_WRONG_INDEX_OPTIONS, + part_idx + TUPLE_INDEX_BASE, "invalid path"); + return -1; + } + char *tmp = region_alloc(region, path_len + 1); + if (tmp == NULL) { + diag_set(OutOfMemory, path_len + 1, "region", "path"); + return -1; + } + /* + * lua_tolstring() guarantees that a string have + * trailing '\0'. + */ + memcpy(tmp, path, path_len + 1); + part->path = tmp; + } else { + part->path = NULL; + } + lua_pop(L, 1); + return 0; +} + +void +lbox_push_key_part(struct lua_State *L, const struct key_part *part) +{ + lua_newtable(L); + + lua_pushstring(L, field_type_strs[part->type]); + lua_setfield(L, -2, "type"); + + lua_pushnumber(L, part->fieldno + TUPLE_INDEX_BASE); + lua_setfield(L, -2, "fieldno"); + + if (part->path != NULL) { + lua_pushlstring(L, part->path, part->path_len); + lua_setfield(L, -2, "path"); + } + + lua_pushboolean(L, key_part_is_nullable(part)); + lua_setfield(L, -2, "is_nullable"); + + if (part->coll_id != COLL_NONE) { + struct coll_id *coll_id = coll_by_id(part->coll_id); + assert(coll_id != NULL); + lua_pushstring(L, coll_id->name); + lua_setfield(L, -2, "collation"); + } +} + +struct key_def * +check_key_def(struct lua_State *L, int idx) +{ + if (lua_type(L, idx) != LUA_TCDATA) + return NULL; + + uint32_t cdata_type; + struct key_def **key_def_ptr = luaL_checkcdata(L, idx, &cdata_type); + if (key_def_ptr == NULL || cdata_type != key_def_type_id) + return NULL; + return *key_def_ptr; +} + +/** + * Free a key_def from a Lua code. + */ +static int +lbox_key_def_gc(struct lua_State *L) +{ + struct key_def *key_def = check_key_def(L, 1); + if (key_def == NULL) + return 0; + box_key_def_delete(key_def); + return 0; +} + +static int +lbox_key_def_extract_key(struct lua_State *L) +{ + struct key_def *key_def; + if (lua_gettop(L) != 2 || (key_def = check_key_def(L, 1)) == NULL) + return luaL_error(L, "Usage: key_def:extract_key(tuple)"); + + struct tuple_format *format = box_tuple_format_default(); + struct tuple *tuple; + if ((tuple = luaT_tuple_new(L, 2, format)) == NULL || + tuple_validate_parts(key_def, tuple) != 0) + return luaT_error(L); + tuple_ref(tuple); + + uint32_t key_size; + char *key = tuple_extract_key(tuple, key_def, &key_size); + tuple_unref(tuple); + if (key == NULL) + return luaT_error(L); + + struct tuple *ret = box_tuple_new(format, key, key + key_size); + if (ret == NULL) + return luaT_error(L); + luaT_pushtuple(L, ret); + return 1; +} + +static int +lbox_key_def_compare(struct lua_State *L) +{ + struct key_def *key_def; + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL) { + return luaL_error(L, "Usage: key_def:" + "compare(tuple_a, tuple_b)"); + } + + struct tuple *tuple_a, *tuple_b; + struct tuple_format *format = box_tuple_format_default(); + if ((tuple_a = luaT_tuple_new(L, 2, format)) == NULL || + tuple_validate_parts(key_def, tuple_a) != 0) + return luaT_error(L); + tuple_ref(tuple_a); + if ((tuple_b = luaT_tuple_new(L, 3, format)) == NULL || + tuple_validate_parts(key_def, tuple_b) != 0) { + tuple_unref(tuple_a); + return luaT_error(L); + } + tuple_ref(tuple_b); + + int rc = tuple_compare(tuple_a, tuple_b, key_def); + tuple_unref(tuple_a); + tuple_unref(tuple_b); + lua_pushinteger(L, rc); + return 1; +} + +static int +lbox_key_def_compare_with_key(struct lua_State *L) +{ + struct key_def *key_def; + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL) { + return luaL_error(L, "Usage: key_def:" + "compare_with_key(tuple, key)"); + } + + struct tuple *tuple, *key_tuple = NULL; + struct tuple_format *format = box_tuple_format_default(); + if ((tuple = luaT_tuple_new(L, 2, format)) == NULL || + tuple_validate_parts(key_def, tuple) != 0) + return luaT_error(L); + tuple_ref(tuple); + if ((key_tuple = luaT_tuple_new(L, 3, format)) == NULL) { + tuple_unref(tuple); + return luaT_error(L); + } + tuple_ref(key_tuple); + + const char *key = tuple_data(key_tuple); + assert(mp_typeof(*key) == MP_ARRAY); + uint32_t part_count = mp_decode_array(&key); + if (key_validate_parts(key_def, key, part_count, true) != 0) { + tuple_unref(tuple); + tuple_unref(key_tuple); + return luaT_error(L); + } + + int rc = tuple_compare_with_key(tuple, key, part_count, key_def); + tuple_unref(tuple); + tuple_unref(key_tuple); + lua_pushinteger(L, rc); + return 1; +} + +static int +lbox_key_def_merge(struct lua_State *L) +{ + struct key_def *key_def_a, *key_def_b; + if (lua_gettop(L) != 2 || (key_def_a = check_key_def(L, 1)) == NULL || + (key_def_b = check_key_def(L, 2)) == NULL) + return luaL_error(L, "Usage: key_def:merge(second_key_def)"); + + struct key_def *new_key_def = key_def_merge(key_def_a, key_def_b); + if (new_key_def == NULL) + return luaT_error(L); + + *(struct key_def **) luaL_pushcdata(L, key_def_type_id) = new_key_def; + lua_pushcfunction(L, lbox_key_def_gc); + luaL_setcdatagc(L, -2); + return 1; +} + +static int +lbox_key_def_to_table(struct lua_State *L) +{ + struct key_def *key_def; + if (lua_gettop(L) != 1 || (key_def = check_key_def(L, 1)) == NULL) + return luaL_error(L, "Usage: key_def:totable()"); + + lua_createtable(L, key_def->part_count, 0); + for (uint32_t i = 0; i < key_def->part_count; ++i) { + lbox_push_key_part(L, &key_def->parts[i]); + lua_rawseti(L, -2, i + 1); + } + return 1; +} + +/** + * Create a new key_def from a Lua table. + * + * Expected a table of key parts on the Lua stack. The format is + * the same as box.space.<...>.index.<...>.parts or corresponding + * net.box's one. + * + * Push the new key_def as cdata to a Lua stack. + */ +static int +lbox_key_def_new(struct lua_State *L) +{ + if (lua_gettop(L) != 1 || lua_istable(L, 1) != 1) + return luaL_error(L, "Bad params, use: key_def.new({" + "{fieldno = fieldno, type = type" + "[, is_nullable = <boolean>]" + "[, path = <string>]" + "[, collation_id = <number>]" + "[, collation = <string>]}, ...}"); + + uint32_t part_count = lua_objlen(L, 1); + const ssize_t parts_size = sizeof(struct key_part_def) * part_count; + + struct region *region = &fiber()->gc; + size_t region_svp = region_used(region); + struct key_part_def *parts = region_alloc(region, parts_size); + if (parts == NULL) { + diag_set(OutOfMemory, parts_size, "region", "parts"); + return luaT_error(L); + } + + for (uint32_t i = 0; i < part_count; ++i) { + lua_pushinteger(L, i + 1); + lua_gettable(L, 1); + if (luaT_key_def_set_part(L, parts, i, region) != 0) { + region_truncate(region, region_svp); + return luaT_error(L); + } + } + + struct key_def *key_def = key_def_new(parts, part_count); + region_truncate(region, region_svp); + if (key_def == NULL) + return luaT_error(L); + + /* + * Calculate minimal field count of tuples with specified + * key and update key_def optionality to use correct + * compare/extract functions. + */ + uint32_t min_field_count = + tuple_format_min_field_count(&key_def, 1, NULL, 0); + key_def_update_optionality(key_def, min_field_count); + + *(struct key_def **) luaL_pushcdata(L, key_def_type_id) = key_def; + lua_pushcfunction(L, lbox_key_def_gc); + luaL_setcdatagc(L, -2); + + return 1; +} + +LUA_API int +luaopen_key_def(struct lua_State *L) +{ + luaL_cdef(L, "struct key_def;"); + key_def_type_id = luaL_ctypeid(L, "struct key_def&"); + + /* Export C functions to Lua. */ + static const struct luaL_Reg meta[] = { + {"new", lbox_key_def_new}, + {NULL, NULL} + }; + luaL_register_module(L, "key_def", meta); + + lua_newtable(L); /* key_def.internal */ + lua_pushcfunction(L, lbox_key_def_extract_key); + lua_setfield(L, -2, "extract_key"); + lua_pushcfunction(L, lbox_key_def_compare); + lua_setfield(L, -2, "compare"); + lua_pushcfunction(L, lbox_key_def_compare_with_key); + lua_setfield(L, -2, "compare_with_key"); + lua_pushcfunction(L, lbox_key_def_merge); + lua_setfield(L, -2, "merge"); + lua_pushcfunction(L, lbox_key_def_to_table); + lua_setfield(L, -2, "totable"); + lua_setfield(L, -2, "internal"); + + return 1; +} diff --git a/src/box/lua/key_def.h b/src/box/lua/key_def.h new file mode 100644 index 000000000..7dc7158e8 --- /dev/null +++ b/src/box/lua/key_def.h @@ -0,0 +1,63 @@ +#ifndef TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED +#define TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED +/* + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY AUTHORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * AUTHORS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#if defined(__cplusplus) +extern "C" { +#endif /* defined(__cplusplus) */ + +struct lua_State; +struct key_part; + +/** + * Push a new table representing a key_part to a Lua stack. + */ +void +lbox_push_key_part(struct lua_State *L, const struct key_part *part); + +/** + * Extract a key_def object from a Lua stack. + */ +struct key_def * +check_key_def(struct lua_State *L, int idx); + +/** + * Register the module. + */ +int +luaopen_key_def(struct lua_State *L); + +#if defined(__cplusplus) +} /* extern "C" */ +#endif /* defined(__cplusplus) */ + +#endif /* TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED */ diff --git a/src/box/lua/key_def.lua b/src/box/lua/key_def.lua new file mode 100644 index 000000000..122243cc4 --- /dev/null +++ b/src/box/lua/key_def.lua @@ -0,0 +1,19 @@ +local ffi = require('ffi') +local key_def = require('key_def') +local key_def_t = ffi.typeof('struct key_def') + +local methods = { + ['extract_key'] = key_def.internal.extract_key, + ['compare'] = key_def.internal.compare, + ['compare_with_key'] = key_def.internal.compare_with_key, + ['merge'] = key_def.internal.merge, + ['totable'] = key_def.internal.totable, + ['__serialize'] = key_def.internal.totable, +} + +ffi.metatype(key_def_t, { + __index = function(self, key) + return methods[key] + end, + __tostring = function(key_def) return "<struct key_def &>" end, +}) diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc index ca793e423..6eecbc756 100644 --- a/src/box/lua/space.cc +++ b/src/box/lua/space.cc @@ -30,6 +30,7 @@ */ #include "box/lua/space.h" #include "box/lua/tuple.h" +#include "box/lua/key_def.h" #include "box/sql/sqlLimit.h" #include "lua/utils.h" #include "lua/trigger.h" @@ -286,32 +287,7 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i) for (uint32_t j = 0; j < index_def->key_def->part_count; j++) { lua_pushnumber(L, j + 1); - lua_newtable(L); - const struct key_part *part = - &index_def->key_def->parts[j]; - - lua_pushstring(L, field_type_strs[part->type]); - lua_setfield(L, -2, "type"); - - lua_pushnumber(L, part->fieldno + TUPLE_INDEX_BASE); - lua_setfield(L, -2, "fieldno"); - - if (part->path != NULL) { - lua_pushlstring(L, part->path, part->path_len); - lua_setfield(L, -2, "path"); - } - - lua_pushboolean(L, key_part_is_nullable(part)); - lua_setfield(L, -2, "is_nullable"); - - if (part->coll_id != COLL_NONE) { - struct coll_id *coll_id = - coll_by_id(part->coll_id); - assert(coll_id != NULL); - lua_pushstring(L, coll_id->name); - lua_setfield(L, -2, "collation"); - } - + lbox_push_key_part(L, &index_def->key_def->parts[j]); lua_settable(L, -3); /* index[k].parts[j] */ } diff --git a/src/box/tuple.h b/src/box/tuple.h index 8b12fd5a8..faa42fdf7 100644 --- a/src/box/tuple.h +++ b/src/box/tuple.h @@ -672,6 +672,39 @@ tuple_field_by_part(const struct tuple *tuple, struct key_part *part) tuple_field_map(tuple), part); } +/** + * Check that tuple match with the key definition. + * @param key_def Key definition. + * @param tuple Tuple for matching. + * @param allow_nullable True if nullable parts are allowed. + * + * @retval 0 The tuple is valid. + * @retval -1 The tuple is invalid. + */ +static inline int +tuple_validate_parts(struct key_def *key_def, struct tuple *tuple) +{ + uint32_t min_field_count = + tuple_format_min_field_count(&key_def, 1, NULL, 0); + uint32_t field_count = tuple_field_count(tuple); + if (field_count < min_field_count) { + diag_set(ClientError, ER_NO_SUCH_FIELD_NO, field_count + 1); + return -1; + } + for (uint32_t idx = 0; idx < key_def->part_count; idx++) { + struct key_part *part = &key_def->parts[idx]; + const char *field = tuple_field_by_part(tuple, part); + if (field == NULL) { + assert(key_def->has_optional_parts); + continue; + } + if (key_part_validate(part->type, field, idx, + key_part_is_nullable(part)) != 0) + return -1; + } + return 0; +} + /** * @brief Tuple Interator */ diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua new file mode 100755 index 000000000..fb2969a4d --- /dev/null +++ b/test/box-tap/key_def.test.lua @@ -0,0 +1,340 @@ +#!/usr/bin/env tarantool + +local tap = require('tap') +local ffi = require('ffi') +local json = require('json') +local fun = require('fun') +local key_def_lib = require('key_def') + +local usage_error = 'Bad params, use: key_def.new({' .. + '{fieldno = fieldno, type = type' .. + '[, is_nullable = <boolean>]' .. + '[, path = <string>]' .. + '[, collation_id = <number>]' .. + '[, collation = <string>]}, ...}' + +local function coll_not_found(fieldno, collation) + if type(collation) == 'number' then + return ('Wrong index options (field %d): ' .. + 'collation was not found by ID'):format(fieldno) + end + + return ('Unknown collation: "%s"'):format(collation) +end + +local function set_key_part_defaults(parts) + local res = {} + for i, part in ipairs(parts) do + res[i] = table.copy(part) + if res[i].is_nullable == nil then + res[i].is_nullable = false + end + end + return res +end + +local key_def_new_cases = { + -- Cases to call before box.cfg{}. + { + 'Pass a field on an unknown type', + parts = {{ + fieldno = 2, + type = 'unknown', + }}, + exp_err = 'Unknown field type: unknown', + }, + { + 'Try to use collation_id before box.cfg{}', + parts = {{ + fieldno = 1, + type = 'string', + collation_id = 2, + }}, + exp_err = coll_not_found(1, 2), + }, + { + 'Try to use collation before box.cfg{}', + parts = {{ + fieldno = 1, + type = 'string', + collation = 'unicode_ci', + }}, + exp_err = coll_not_found(1, 'unicode_ci'), + }, + function() + -- For collations. + box.cfg{} + end, + -- Cases to call after box.cfg{}. + { + 'Try to use both collation_id and collation', + parts = {{ + fieldno = 1, + type = 'string', + collation_id = 2, + collation = 'unicode_ci', + }}, + exp_err = 'Conflicting options: collation_id and collation', + }, + { + 'Unknown collation_id', + parts = {{ + fieldno = 1, + type = 'string', + collation_id = 42, + }}, + exp_err = coll_not_found(1, 42), + }, + { + 'Unknown collation name', + parts = {{ + fieldno = 1, + type = 'string', + collation = 'unknown', + }}, + exp_err = 'Unknown collation: "unknown"', + }, + { + 'Bad parts parameter type', + parts = 1, + exp_err = usage_error, + }, + { + 'No parameters', + params = {}, + exp_err = usage_error, + }, + { + 'Two parameters', + params = {{}, {}}, + exp_err = usage_error, + }, + { + 'Invalid JSON path', + parts = {{ + fieldno = 1, + type = 'string', + path = '[3[', + }}, + exp_err = 'Wrong index options (field 1): invalid path', + }, + { + 'Success case; zero parts', + parts = {}, + exp_err = nil, + }, + { + 'Success case; one part', + parts = {{ + fieldno = 1, + type = 'string', + }}, + exp_err = nil, + }, + { + 'Success case; one part with a JSON path', + parts = {{ + fieldno = 1, + type = 'string', + path = '[3]', + }}, + exp_err = nil, + }, +} + +local test = tap.test('key_def') + +test:plan(#key_def_new_cases - 1 + 7) +for _, case in ipairs(key_def_new_cases) do + if type(case) == 'function' then + case() + else + local ok, res + if case.params then + ok, res = pcall(key_def_lib.new, unpack(case.params)) + else + ok, res = pcall(key_def_lib.new, case.parts) + end + if case.exp_err == nil then + ok = ok and type(res) == 'cdata' and + ffi.istype('struct key_def', res) + test:ok(ok, case[1]) + else + local err = tostring(res) -- cdata -> string + test:is_deeply({ok, err}, {false, case.exp_err}, case[1]) + end + end +end + +-- Prepare source data for test cases. +local parts_a = { + {type = 'unsigned', fieldno = 1}, +} +local parts_b = { + {type = 'number', fieldno = 2}, + {type = 'number', fieldno = 3}, +} +local parts_c = { + {type = 'scalar', fieldno = 2}, + {type = 'scalar', fieldno = 1}, + {type = 'string', fieldno = 4, is_nullable = true}, +} +local key_def_a = key_def_lib.new(parts_a) +local key_def_b = key_def_lib.new(parts_b) +local key_def_c = key_def_lib.new(parts_c) +local tuple_a = box.tuple.new({1, 1, 22}) +local tuple_b = box.tuple.new({2, 1, 11}) +local tuple_c = box.tuple.new({3, 1, 22}) + +-- Case: extract_key(). +test:test('extract_key()', function(test) + test:plan(8) + + test:is_deeply(key_def_a:extract_key(tuple_a):totable(), {1}, 'case 1') + test:is_deeply(key_def_b:extract_key(tuple_a):totable(), {1, 22}, 'case 2') + + -- JSON path. + local res = key_def_lib.new({ + {type = 'string', fieldno = 1, path = 'a.b'}, + }):extract_key(box.tuple.new({{a = {b = 'foo'}}})):totable() + test:is_deeply(res, {'foo'}, 'JSON path (tuple argument)') + + local res = key_def_lib.new({ + {type = 'string', fieldno = 1, path = 'a.b'}, + }):extract_key({{a = {b = 'foo'}}}):totable() + test:is_deeply(res, {'foo'}, 'JSON path (table argument)') + + -- A key def has a **nullable** part with a field that is over + -- a tuple size. + test:is_deeply(key_def_c:extract_key(tuple_a):totable(), {1, 1, box.NULL}, + 'short tuple with a nullable part') + + -- A key def has a **non-nullable** part with a field that is + -- over a tuple size. + local exp_err = 'Field 2 was not found in the tuple' + local key_def = key_def_lib.new({ + {type = 'string', fieldno = 1}, + {type = 'string', fieldno = 2}, + }) + local ok, err = pcall(key_def.extract_key, key_def, + box.tuple.new({'foo'})) + test:is_deeply({ok, tostring(err)}, {false, exp_err}, + 'short tuple with a non-nullable part (case 1)') + + -- Same as before, but a max fieldno is over tuple:len() + 1. + local exp_err = 'Field 2 was not found in the tuple' + local key_def = key_def_lib.new({ + {type = 'string', fieldno = 1}, + {type = 'string', fieldno = 2}, + {type = 'string', fieldno = 3}, + }) + local ok, err = pcall(key_def.extract_key, key_def, + box.tuple.new({'foo'})) + test:is_deeply({ok, tostring(err)}, {false, exp_err}, + 'short tuple with a non-nullable part (case 2)') + + local exp_err = 'Supplied key type of part 2 does not match index ' .. + 'part type: expected string' + local ok, err = pcall(key_def.extract_key, key_def, + {"one", "two", 3}) + test:is_deeply({ok, tostring(err)}, {false, exp_err}, + 'short tuple with a non-nullable part (case 2)') +end) + +-- Case: compare(). +test:test('compare()', function(test) + test:plan(8) + + test:is(key_def_a:compare(tuple_b, tuple_a), 1, + 'case 1: great (tuple argument)') + test:is(key_def_a:compare(tuple_b, tuple_c), -1, + 'case 2: less (tuple argument)') + test:is(key_def_b:compare(tuple_b, tuple_a), -1, + 'case 3: less (tuple argument)') + test:is(key_def_b:compare(tuple_a, tuple_c), 0, + 'case 4: equal (tuple argument)') + + test:is(key_def_a:compare(tuple_b:totable(), tuple_a:totable()), 1, + 'case 1: great (table argument)') + test:is(key_def_a:compare(tuple_b:totable(), tuple_c:totable()), -1, + 'case 2: less (table argument)') + test:is(key_def_b:compare(tuple_b:totable(), tuple_a:totable()), -1, + 'case 3: less (table argument)') + test:is(key_def_b:compare(tuple_a:totable(), tuple_c:totable()), 0, + 'case 4: equal (table argument)') +end) + +-- Case: compare_with_key(). +test:test('compare_with_key()', function(test) + test:plan(2) + + local key = {1, 22} + test:is(key_def_b:compare_with_key(tuple_a:totable(), key), 0, 'table') + + local key = box.tuple.new({1, 22}) + test:is(key_def_b:compare_with_key(tuple_a, key), 0, 'tuple') +end) + +-- Case: totable(). +test:test('totable()', function(test) + test:plan(2) + + local exp = set_key_part_defaults(parts_a) + test:is_deeply(key_def_a:totable(), exp, 'case 1') + + local exp = set_key_part_defaults(parts_b) + test:is_deeply(key_def_b:totable(), exp, 'case 2') +end) + +-- Case: __serialize(). +test:test('__serialize()', function(test) + test:plan(2) + + local exp = set_key_part_defaults(parts_a) + test:is(json.encode(key_def_a), json.encode(exp), 'case 1') + + local exp = set_key_part_defaults(parts_b) + test:is(json.encode(key_def_b), json.encode(exp), 'case 2') +end) + +-- Case: tostring(). +test:test('tostring()', function(test) + test:plan(2) + + local exp = '<struct key_def &>' + test:is(tostring(key_def_a), exp, 'case 1') + test:is(tostring(key_def_b), exp, 'case 2') +end) + +-- Case: merge(). +test:test('merge()', function(test) + test:plan(6) + + local key_def_ab = key_def_a:merge(key_def_b) + local exp_parts = fun.iter(key_def_a:totable()) + :chain(fun.iter(key_def_b:totable())):totable() + test:is_deeply(key_def_ab:totable(), exp_parts, + 'case 1: verify with :totable()') + test:is_deeply(key_def_ab:extract_key(tuple_a):totable(), {1, 1, 22}, + 'case 1: verify with :extract_key()') + + local key_def_ba = key_def_b:merge(key_def_a) + local exp_parts = fun.iter(key_def_b:totable()) + :chain(fun.iter(key_def_a:totable())):totable() + test:is_deeply(key_def_ba:totable(), exp_parts, + 'case 2: verify with :totable()') + test:is_deeply(key_def_ba:extract_key(tuple_a):totable(), {1, 22, 1}, + 'case 2: verify with :extract_key()') + + -- Intersecting parts + NULL parts. + local key_def_cb = key_def_c:merge(key_def_b) + local exp_parts = key_def_c:totable() + exp_parts[#exp_parts + 1] = {type = 'number', fieldno = 3, + is_nullable = false} + test:is_deeply(key_def_cb:totable(), exp_parts, + 'case 3: verify with :totable()') + test:is_deeply(key_def_cb:extract_key(tuple_a):totable(), + {1, 1, box.NULL, 22}, 'case 3: verify with :extract_key()') +end) + +os.exit(test:check() and 0 or 1) -- 2.21.0 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] lua: add key_def lua module 2019-03-27 14:29 ` [tarantool-patches] [PATCH v2 2/2] lua: add key_def lua module Kirill Shcherbatov @ 2019-03-28 2:01 ` Alexander Turenko 2019-03-28 7:38 ` [tarantool-patches] " Kirill Shcherbatov 2019-03-28 8:41 ` Kirill Shcherbatov 0 siblings, 2 replies; 25+ messages in thread From: Alexander Turenko @ 2019-03-28 2:01 UTC (permalink / raw) To: Kirill Shcherbatov; +Cc: tarantool-patches, Vladimir Davydov Thank you, it works like a charm. I added a fixup commit on top of your patchset (added a test case, updated comments in the test a bit). Also please consider comments below. Vladimir, I CCed you to ask a question at end of the email (the code is on kshch/gh-4025-lua-key-kef-methods branch). WBR, Alexander Turenko. On Wed, Mar 27, 2019 at 05:29:28PM +0300, Kirill Shcherbatov wrote: > There are several reasons to add this module: > > * Factor out key parts parsing code from the tuples merger (#3276). > * Support comparing a tuple with a key / a tuple, support merging > key_defs from Lua (#3398). > * Support extracting a key from a tuple (#4025). > > The format of `parts` parameter in the `key_def.new(parts)` call is > compatible with the following structures: > > * box.space[...].index[...].parts; > * net_box_conn.space[...].index[...].parts. > > A key_def instance has the following methods: > > * :extract_key(tuple) -> key (as tuple) > * :compare(tuple_a, tuple_b) -> number > * :compare_with_key(tuple, key) -> number > * :merge(another_key_def) -> new key_def instance > * :totable() -> table > I would add here 'Functions that accept tuple(s) also allow to pass Lua table(s) instead'. > +static int > +lbox_key_def_compare(struct lua_State *L) > +{ > + struct key_def *key_def; > + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL) { > + return luaL_error(L, "Usage: key_def:" > + "compare(tuple_a, tuple_b)"); > + } > + > + struct tuple *tuple_a, *tuple_b; > + struct tuple_format *format = box_tuple_format_default(); > + if ((tuple_a = luaT_tuple_new(L, 2, format)) == NULL || > + tuple_validate_parts(key_def, tuple_a) != 0) > + return luaT_error(L); > + tuple_ref(tuple_a); > + if ((tuple_b = luaT_tuple_new(L, 3, format)) == NULL || > + tuple_validate_parts(key_def, tuple_b) != 0) { > + tuple_unref(tuple_a); > + return luaT_error(L); > + } > + tuple_ref(tuple_b); Consider the case when a user get tuples from a local space (or merger) and they have a format that allows to compare faster using precalculated offsets. I think we should not create a new tuple(s) in the case. Applicable for other functions too. > diff --git a/src/box/tuple.h b/src/box/tuple.h > index 8b12fd5a8..faa42fdf7 100644 > --- a/src/box/tuple.h > +++ b/src/box/tuple.h > @@ -672,6 +672,39 @@ tuple_field_by_part(const struct tuple *tuple, struct key_part *part) > tuple_field_map(tuple), part); > } > > +/** > + * Check that tuple match with the key definition. > + * @param key_def Key definition. > + * @param tuple Tuple for matching. > + * @param allow_nullable True if nullable parts are allowed. > + * > + * @retval 0 The tuple is valid. > + * @retval -1 The tuple is invalid. > + */ > +static inline int > +tuple_validate_parts(struct key_def *key_def, struct tuple *tuple) I don't sure it worth to inline this function: it is not so lightweight as, say, a structure field access. I'm tentative whether this function should be in tuple.[ch] or key_def.[ch]. What do you think? (If it is in tuple.[ch] maybe it is better to let a tuple being the first parameter?) Maybe we need to ask Vladimir (CCed). ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-03-28 2:01 ` Alexander Turenko @ 2019-03-28 7:38 ` Kirill Shcherbatov 2019-03-28 8:41 ` Kirill Shcherbatov 1 sibling, 0 replies; 25+ messages in thread From: Kirill Shcherbatov @ 2019-03-28 7:38 UTC (permalink / raw) To: tarantool-patches, Alexander Turenko; +Cc: Vladimir Davydov > Consider the case when a user get tuples from a local space (or merger) > and they have a format that allows to compare faster using precalculated > offsets. I think we should not create a new tuple(s) in the case. > > Applicable for other functions too. If we decide to support setting arguments in the form of tables, I propose to introduce the following function. In my opinion, it does not look unnatural. diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c index 18d5d961d..9cacf3cca 100644 --- a/src/box/lua/key_def.c +++ b/src/box/lua/key_def.c @@ -224,6 +224,23 @@ lbox_key_def_gc(struct lua_State *L) return 0; } +/** + * Take existent tuple from LUA stack or build a new tuple with + * default format from table, check for compatibility with a + * given key_def. + */ +static struct tuple * +lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) +{ + struct tuple *tuple = luaT_istuple(L, idx); + if (tuple == NULL) + tuple = luaT_tuple_new(L, 2, box_tuple_format_default()); + if (tuple == NULL || tuple_validate_parts(key_def, tuple) != 0) + return NULL; + tuple_ref(tuple); + return tuple; +} + static int lbox_key_def_extract_key(struct lua_State *L) { @@ -231,12 +248,9 @@ lbox_key_def_extract_key(struct lua_State *L) if (lua_gettop(L) != 2 || (key_def = check_key_def(L, 1)) == NULL) return luaL_error(L, "Usage: key_def:extract_key(tuple)"); - struct tuple_format *format = box_tuple_format_default(); struct tuple *tuple; - if ((tuple = luaT_tuple_new(L, 2, format)) == NULL || - tuple_validate_parts(key_def, tuple) != 0) + if ((tuple = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) return luaT_error(L); - tuple_ref(tuple); uint32_t key_size; char *key = tuple_extract_key(tuple, key_def, &key_size); @@ -244,7 +258,8 @@ lbox_key_def_extract_key(struct lua_State *L) if (key == NULL) return luaT_error(L); - struct tuple *ret = box_tuple_new(format, key, key + key_size); + struct tuple *ret = + box_tuple_new(box_tuple_format_default(), key, key + key_size); if (ret == NULL) return luaT_error(L); luaT_pushtuple(L, ret); @@ -261,17 +276,12 @@ lbox_key_def_compare(struct lua_State *L) } struct tuple *tuple_a, *tuple_b; - struct tuple_format *format = box_tuple_format_default(); - if ((tuple_a = luaT_tuple_new(L, 2, format)) == NULL || - tuple_validate_parts(key_def, tuple_a) != 0) + if ((tuple_a = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) return luaT_error(L); - tuple_ref(tuple_a); - if ((tuple_b = luaT_tuple_new(L, 3, format)) == NULL || - tuple_validate_parts(key_def, tuple_b) != 0) { + if ((tuple_b = lbox_key_def_check_tuple(L, key_def, 3)) == NULL) { tuple_unref(tuple_a); return luaT_error(L); } - tuple_ref(tuple_b); int rc = tuple_compare(tuple_a, tuple_b, key_def); tuple_unref(tuple_a); @@ -291,10 +301,8 @@ lbox_key_def_compare_with_key(struct lua_State *L) struct tuple *tuple, *key_tuple = NULL; struct tuple_format *format = box_tuple_format_default(); - if ((tuple = luaT_tuple_new(L, 2, format)) == NULL || - tuple_validate_parts(key_def, tuple) != 0) + if ((tuple = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) return luaT_error(L); - tuple_ref(tuple); if ((key_tuple = luaT_tuple_new(L, 3, format)) == NULL) { tuple_unref(tuple); return luaT_error(L); ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-03-28 2:01 ` Alexander Turenko 2019-03-28 7:38 ` [tarantool-patches] " Kirill Shcherbatov @ 2019-03-28 8:41 ` Kirill Shcherbatov [not found] ` <6d915212-e80f-4a6d-d884-b838bf25f8a7@tarantool.org> 1 sibling, 1 reply; 25+ messages in thread From: Kirill Shcherbatov @ 2019-03-28 8:41 UTC (permalink / raw) To: Alexander Turenko, Tarantool MailList On 28.03.2019 5:01, Alexander Turenko wrote: > Thank you, it works like a charm. > > I added a fixup commit on top of your patchset (added a test case, > updated comments in the test a bit). Also please consider comments > below. Thank you! > I would add here 'Functions that accept tuple(s) also allow to pass Lua > table(s) instead'. Included. > Consider the case when a user get tuples from a local space (or merger) > and they have a format that allows to compare faster using precalculated > offsets. I think we should not create a new tuple(s) in the case. > > Applicable for other functions too. Introduced a new wrapper that checks existent tuple from stack or build a new one if possible, perform validations. > I don't sure it worth to inline this function: it is not so lightweight > as, say, a structure field access. > > I'm tentative whether this function should be in tuple.[ch] or > key_def.[ch]. What do you think? > > (If it is in tuple.[ch] maybe it is better to let a tuple being the > first parameter?) It must be tuple.c because we need tuple_field_by_part and some other helper that are not defined in key_def.c ======================================================== There are several reasons to add this module: * Factor out key parts parsing code from the tuples merger (#3276). * Support comparing a tuple with a key / a tuple, support merging key_defs from Lua (#3398). * Support extracting a key from a tuple (#4025). The format of `parts` parameter in the `key_def.new(parts)` call is compatible with the following structures: * box.space[...].index[...].parts; * net_box_conn.space[...].index[...].parts. A key_def instance has the following methods: * :extract_key(tuple) -> key (as tuple) * :compare(tuple_a, tuple_b) -> number * :compare_with_key(tuple, key) -> number * :merge(another_key_def) -> new key_def instance * :totable() -> table Note functions that accept tuple(s) also allow to pass Lua table(s) instead. Needed for #3276. Fixes #3398. Fixes #4025. @TarantoolBot document Title: lua: key_def module See the commit message for an API reference. Example for extract_key(): ```lua -- Remove values got from a secondary non-unique index. local key_def_lib = require('key_def') local s = box.schema.space.create('test') local pk = s:create_index('pk') local sk = s:create_index('test', {unique = false, parts = { {2, 'number', path = 'a'}, {2, 'number', path = 'b'}}}) s:insert{1, {a = 1, b = 1}} s:insert{2, {a = 1, b = 2}} local key_def = key_def_lib.new(pk.parts) for _, tuple in sk:pairs({1})) do local key = key_def:extract_key(tuple) pk:delete(key) end ``` --- src/CMakeLists.txt | 1 + src/box/CMakeLists.txt | 2 + src/box/lua/init.c | 5 + src/box/lua/key_def.c | 443 ++++++++++++++++++++++++++++++++++ src/box/lua/key_def.h | 63 +++++ src/box/lua/key_def.lua | 19 ++ src/box/lua/space.cc | 28 +-- src/box/tuple.c | 24 ++ src/box/tuple.h | 12 + test/box-tap/key_def.test.lua | 370 ++++++++++++++++++++++++++++ 10 files changed, 941 insertions(+), 26 deletions(-) create mode 100644 src/box/lua/key_def.c create mode 100644 src/box/lua/key_def.h create mode 100644 src/box/lua/key_def.lua create mode 100755 test/box-tap/key_def.test.lua diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7c2395517..a6a18142b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -136,6 +136,7 @@ set(api_headers ${CMAKE_SOURCE_DIR}/src/lua/string.h ${CMAKE_SOURCE_DIR}/src/box/txn.h ${CMAKE_SOURCE_DIR}/src/box/key_def.h + ${CMAKE_SOURCE_DIR}/src/box/lua/key_def.h ${CMAKE_SOURCE_DIR}/src/box/field_def.h ${CMAKE_SOURCE_DIR}/src/box/tuple.h ${CMAKE_SOURCE_DIR}/src/box/tuple_format.h diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt index 59e91b65a..906ce22e6 100644 --- a/src/box/CMakeLists.txt +++ b/src/box/CMakeLists.txt @@ -12,6 +12,7 @@ lua_source(lua_sources lua/net_box.lua) lua_source(lua_sources lua/upgrade.lua) lua_source(lua_sources lua/console.lua) lua_source(lua_sources lua/xlog.lua) +lua_source(lua_sources lua/key_def.lua) set(bin_sources) bin_source(bin_sources bootstrap.snap bootstrap.h) @@ -139,6 +140,7 @@ add_library(box STATIC lua/net_box.c lua/xlog.c lua/sql.c + lua/key_def.c ${bin_sources}) target_link_libraries(box box_error tuple stat xrow xlog vclock crc32 scramble diff --git a/src/box/lua/init.c b/src/box/lua/init.c index 744b2c895..68ef58909 100644 --- a/src/box/lua/init.c +++ b/src/box/lua/init.c @@ -59,9 +59,11 @@ #include "box/lua/console.h" #include "box/lua/tuple.h" #include "box/lua/sql.h" +#include "box/lua/key_def.h" extern char session_lua[], tuple_lua[], + key_def_lua[], schema_lua[], load_cfg_lua[], xlog_lua[], @@ -80,6 +82,7 @@ static const char *lua_sources[] = { "box/console", console_lua, "box/load_cfg", load_cfg_lua, "box/xlog", xlog_lua, + "box/key_def", key_def_lua, NULL }; @@ -312,6 +315,8 @@ box_lua_init(struct lua_State *L) lua_pop(L, 1); tarantool_lua_console_init(L); lua_pop(L, 1); + luaopen_key_def(L); + lua_pop(L, 1); /* Load Lua extension */ for (const char **s = lua_sources; *s; s += 2) { diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c new file mode 100644 index 000000000..57c21db7b --- /dev/null +++ b/src/box/lua/key_def.c @@ -0,0 +1,443 @@ +/* + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include "box/coll_id_cache.h" +#include "box/lua/key_def.h" +#include "box/tuple.h" +#include "diag.h" +#include "fiber.h" +#include "lua/utils.h" +#include "tuple.h" + +static uint32_t key_def_type_id = 0; + +/** + * Set key_part_def from a table on top of a Lua stack. + * + * When successful return 0, otherwise return -1 and set a diag. + */ +static int +luaT_key_def_set_part(struct lua_State *L, struct key_part_def *parts, + int part_idx, struct region *region) +{ + struct key_part_def *part = &parts[part_idx]; + *part = key_part_def_default; + + /* Set part->fieldno. */ + lua_pushstring(L, "fieldno"); + lua_gettable(L, -2); + if (lua_isnil(L, -1)) { + diag_set(IllegalParams, "fieldno must not be nil"); + return -1; + } + /* + * Transform one-based Lua fieldno to zero-based + * fieldno to use in key_def_new(). + */ + part->fieldno = lua_tointeger(L, -1) - TUPLE_INDEX_BASE; + lua_pop(L, 1); + + /* Set part->type. */ + lua_pushstring(L, "type"); + lua_gettable(L, -2); + if (lua_isnil(L, -1)) { + diag_set(IllegalParams, "type must not be nil"); + return -1; + } + size_t type_len; + const char *type_name = lua_tolstring(L, -1, &type_len); + lua_pop(L, 1); + part->type = field_type_by_name(type_name, type_len); + switch (part->type) { + case FIELD_TYPE_ANY: + case FIELD_TYPE_ARRAY: + case FIELD_TYPE_MAP: + /* Tuple comparators don't support these types. */ + diag_set(IllegalParams, "Unsupported field type: %s", + type_name); + return -1; + case field_type_MAX: + diag_set(IllegalParams, "Unknown field type: %s", type_name); + return -1; + default: + /* Pass though. */ + break; + } + + /* Set part->is_nullable and part->nullable_action. */ + lua_pushstring(L, "is_nullable"); + lua_gettable(L, -2); + if (!lua_isnil(L, -1) && lua_toboolean(L, -1) != 0) { + part->is_nullable = true; + part->nullable_action = ON_CONFLICT_ACTION_NONE; + } + lua_pop(L, 1); + + /* + * Set part->coll_id using collation_id. + * + * The value will be checked in key_def_new(). + */ + lua_pushstring(L, "collation_id"); + lua_gettable(L, -2); + if (!lua_isnil(L, -1)) + part->coll_id = lua_tointeger(L, -1); + lua_pop(L, 1); + + /* Set part->coll_id using collation. */ + lua_pushstring(L, "collation"); + lua_gettable(L, -2); + if (!lua_isnil(L, -1)) { + /* Check for conflicting options. */ + if (part->coll_id != COLL_NONE) { + diag_set(IllegalParams, "Conflicting options: " + "collation_id and collation"); + return -1; + } + + size_t coll_name_len; + const char *coll_name = lua_tolstring(L, -1, &coll_name_len); + struct coll_id *coll_id = coll_by_name(coll_name, + coll_name_len); + if (coll_id == NULL) { + diag_set(IllegalParams, "Unknown collation: \"%s\"", + coll_name); + return -1; + } + part->coll_id = coll_id->id; + } + lua_pop(L, 1); + + /* Set part->path (JSON path). */ + lua_pushstring(L, "path"); + lua_gettable(L, -2); + if (!lua_isnil(L, -1)) { + size_t path_len; + const char *path = lua_tolstring(L, -1, &path_len); + if (json_path_validate(path, path_len, TUPLE_INDEX_BASE) != 0) { + diag_set(ClientError, ER_WRONG_INDEX_OPTIONS, + part_idx + TUPLE_INDEX_BASE, "invalid path"); + return -1; + } + char *tmp = region_alloc(region, path_len + 1); + if (tmp == NULL) { + diag_set(OutOfMemory, path_len + 1, "region", "path"); + return -1; + } + /* + * lua_tolstring() guarantees that a string have + * trailing '\0'. + */ + memcpy(tmp, path, path_len + 1); + part->path = tmp; + } else { + part->path = NULL; + } + lua_pop(L, 1); + return 0; +} + +void +lbox_push_key_part(struct lua_State *L, const struct key_part *part) +{ + lua_newtable(L); + + lua_pushstring(L, field_type_strs[part->type]); + lua_setfield(L, -2, "type"); + + lua_pushnumber(L, part->fieldno + TUPLE_INDEX_BASE); + lua_setfield(L, -2, "fieldno"); + + if (part->path != NULL) { + lua_pushlstring(L, part->path, part->path_len); + lua_setfield(L, -2, "path"); + } + + lua_pushboolean(L, key_part_is_nullable(part)); + lua_setfield(L, -2, "is_nullable"); + + if (part->coll_id != COLL_NONE) { + struct coll_id *coll_id = coll_by_id(part->coll_id); + assert(coll_id != NULL); + lua_pushstring(L, coll_id->name); + lua_setfield(L, -2, "collation"); + } +} + +struct key_def * +check_key_def(struct lua_State *L, int idx) +{ + if (lua_type(L, idx) != LUA_TCDATA) + return NULL; + + uint32_t cdata_type; + struct key_def **key_def_ptr = luaL_checkcdata(L, idx, &cdata_type); + if (key_def_ptr == NULL || cdata_type != key_def_type_id) + return NULL; + return *key_def_ptr; +} + +/** + * Free a key_def from a Lua code. + */ +static int +lbox_key_def_gc(struct lua_State *L) +{ + struct key_def *key_def = check_key_def(L, 1); + if (key_def == NULL) + return 0; + box_key_def_delete(key_def); + return 0; +} + +/** + * Take existent tuple from LUA stack or build a new tuple with + * default format from table, check for compatibility with a + * given key_def. + */ +static struct tuple * +lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) +{ + struct tuple *tuple = luaT_istuple(L, idx); + if (tuple == NULL) + tuple = luaT_tuple_new(L, idx, box_tuple_format_default()); + if (tuple == NULL || box_tuple_validate_parts(tuple, key_def) != 0) + return NULL; + tuple_ref(tuple); + return tuple; +} + +static int +lbox_key_def_extract_key(struct lua_State *L) +{ + struct key_def *key_def; + if (lua_gettop(L) != 2 || (key_def = check_key_def(L, 1)) == NULL) + return luaL_error(L, "Usage: key_def:extract_key(tuple)"); + + struct tuple *tuple; + if ((tuple = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) + return luaT_error(L); + + uint32_t key_size; + char *key = tuple_extract_key(tuple, key_def, &key_size); + tuple_unref(tuple); + if (key == NULL) + return luaT_error(L); + + struct tuple *ret = + box_tuple_new(box_tuple_format_default(), key, key + key_size); + if (ret == NULL) + return luaT_error(L); + luaT_pushtuple(L, ret); + return 1; +} + +static int +lbox_key_def_compare(struct lua_State *L) +{ + struct key_def *key_def; + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL) { + return luaL_error(L, "Usage: key_def:" + "compare(tuple_a, tuple_b)"); + } + + struct tuple *tuple_a, *tuple_b; + if ((tuple_a = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) + return luaT_error(L); + if ((tuple_b = lbox_key_def_check_tuple(L, key_def, 3)) == NULL) { + tuple_unref(tuple_a); + return luaT_error(L); + } + + int rc = tuple_compare(tuple_a, tuple_b, key_def); + tuple_unref(tuple_a); + tuple_unref(tuple_b); + lua_pushinteger(L, rc); + return 1; +} + +static int +lbox_key_def_compare_with_key(struct lua_State *L) +{ + struct key_def *key_def; + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL) { + return luaL_error(L, "Usage: key_def:" + "compare_with_key(tuple, key)"); + } + + struct tuple *tuple, *key_tuple = NULL; + struct tuple_format *format = box_tuple_format_default(); + if ((tuple = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) + return luaT_error(L); + if ((key_tuple = luaT_tuple_new(L, 3, format)) == NULL) { + tuple_unref(tuple); + return luaT_error(L); + } + tuple_ref(key_tuple); + + const char *key = tuple_data(key_tuple); + assert(mp_typeof(*key) == MP_ARRAY); + uint32_t part_count = mp_decode_array(&key); + if (key_validate_parts(key_def, key, part_count, true) != 0) { + tuple_unref(tuple); + tuple_unref(key_tuple); + return luaT_error(L); + } + + int rc = tuple_compare_with_key(tuple, key, part_count, key_def); + tuple_unref(tuple); + tuple_unref(key_tuple); + lua_pushinteger(L, rc); + return 1; +} + +static int +lbox_key_def_merge(struct lua_State *L) +{ + struct key_def *key_def_a, *key_def_b; + if (lua_gettop(L) != 2 || (key_def_a = check_key_def(L, 1)) == NULL || + (key_def_b = check_key_def(L, 2)) == NULL) + return luaL_error(L, "Usage: key_def:merge(second_key_def)"); + + struct key_def *new_key_def = key_def_merge(key_def_a, key_def_b); + if (new_key_def == NULL) + return luaT_error(L); + + *(struct key_def **) luaL_pushcdata(L, key_def_type_id) = new_key_def; + lua_pushcfunction(L, lbox_key_def_gc); + luaL_setcdatagc(L, -2); + return 1; +} + +static int +lbox_key_def_to_table(struct lua_State *L) +{ + struct key_def *key_def; + if (lua_gettop(L) != 1 || (key_def = check_key_def(L, 1)) == NULL) + return luaL_error(L, "Usage: key_def:totable()"); + + lua_createtable(L, key_def->part_count, 0); + for (uint32_t i = 0; i < key_def->part_count; ++i) { + lbox_push_key_part(L, &key_def->parts[i]); + lua_rawseti(L, -2, i + 1); + } + return 1; +} + +/** + * Create a new key_def from a Lua table. + * + * Expected a table of key parts on the Lua stack. The format is + * the same as box.space.<...>.index.<...>.parts or corresponding + * net.box's one. + * + * Push the new key_def as cdata to a Lua stack. + */ +static int +lbox_key_def_new(struct lua_State *L) +{ + if (lua_gettop(L) != 1 || lua_istable(L, 1) != 1) + return luaL_error(L, "Bad params, use: key_def.new({" + "{fieldno = fieldno, type = type" + "[, is_nullable = <boolean>]" + "[, path = <string>]" + "[, collation_id = <number>]" + "[, collation = <string>]}, ...}"); + + uint32_t part_count = lua_objlen(L, 1); + const ssize_t parts_size = sizeof(struct key_part_def) * part_count; + + struct region *region = &fiber()->gc; + size_t region_svp = region_used(region); + struct key_part_def *parts = region_alloc(region, parts_size); + if (parts == NULL) { + diag_set(OutOfMemory, parts_size, "region", "parts"); + return luaT_error(L); + } + + for (uint32_t i = 0; i < part_count; ++i) { + lua_pushinteger(L, i + 1); + lua_gettable(L, 1); + if (luaT_key_def_set_part(L, parts, i, region) != 0) { + region_truncate(region, region_svp); + return luaT_error(L); + } + } + + struct key_def *key_def = key_def_new(parts, part_count); + region_truncate(region, region_svp); + if (key_def == NULL) + return luaT_error(L); + + /* + * Calculate minimal field count of tuples with specified + * key and update key_def optionality to use correct + * compare/extract functions. + */ + uint32_t min_field_count = + tuple_format_min_field_count(&key_def, 1, NULL, 0); + key_def_update_optionality(key_def, min_field_count); + + *(struct key_def **) luaL_pushcdata(L, key_def_type_id) = key_def; + lua_pushcfunction(L, lbox_key_def_gc); + luaL_setcdatagc(L, -2); + + return 1; +} + +LUA_API int +luaopen_key_def(struct lua_State *L) +{ + luaL_cdef(L, "struct key_def;"); + key_def_type_id = luaL_ctypeid(L, "struct key_def&"); + + /* Export C functions to Lua. */ + static const struct luaL_Reg meta[] = { + {"new", lbox_key_def_new}, + {NULL, NULL} + }; + luaL_register_module(L, "key_def", meta); + + lua_newtable(L); /* key_def.internal */ + lua_pushcfunction(L, lbox_key_def_extract_key); + lua_setfield(L, -2, "extract_key"); + lua_pushcfunction(L, lbox_key_def_compare); + lua_setfield(L, -2, "compare"); + lua_pushcfunction(L, lbox_key_def_compare_with_key); + lua_setfield(L, -2, "compare_with_key"); + lua_pushcfunction(L, lbox_key_def_merge); + lua_setfield(L, -2, "merge"); + lua_pushcfunction(L, lbox_key_def_to_table); + lua_setfield(L, -2, "totable"); + lua_setfield(L, -2, "internal"); + + return 1; +} diff --git a/src/box/lua/key_def.h b/src/box/lua/key_def.h new file mode 100644 index 000000000..7dc7158e8 --- /dev/null +++ b/src/box/lua/key_def.h @@ -0,0 +1,63 @@ +#ifndef TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED +#define TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED +/* + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY AUTHORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * AUTHORS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#if defined(__cplusplus) +extern "C" { +#endif /* defined(__cplusplus) */ + +struct lua_State; +struct key_part; + +/** + * Push a new table representing a key_part to a Lua stack. + */ +void +lbox_push_key_part(struct lua_State *L, const struct key_part *part); + +/** + * Extract a key_def object from a Lua stack. + */ +struct key_def * +check_key_def(struct lua_State *L, int idx); + +/** + * Register the module. + */ +int +luaopen_key_def(struct lua_State *L); + +#if defined(__cplusplus) +} /* extern "C" */ +#endif /* defined(__cplusplus) */ + +#endif /* TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED */ diff --git a/src/box/lua/key_def.lua b/src/box/lua/key_def.lua new file mode 100644 index 000000000..122243cc4 --- /dev/null +++ b/src/box/lua/key_def.lua @@ -0,0 +1,19 @@ +local ffi = require('ffi') +local key_def = require('key_def') +local key_def_t = ffi.typeof('struct key_def') + +local methods = { + ['extract_key'] = key_def.internal.extract_key, + ['compare'] = key_def.internal.compare, + ['compare_with_key'] = key_def.internal.compare_with_key, + ['merge'] = key_def.internal.merge, + ['totable'] = key_def.internal.totable, + ['__serialize'] = key_def.internal.totable, +} + +ffi.metatype(key_def_t, { + __index = function(self, key) + return methods[key] + end, + __tostring = function(key_def) return "<struct key_def &>" end, +}) diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc index ca793e423..6eecbc756 100644 --- a/src/box/lua/space.cc +++ b/src/box/lua/space.cc @@ -30,6 +30,7 @@ */ #include "box/lua/space.h" #include "box/lua/tuple.h" +#include "box/lua/key_def.h" #include "box/sql/sqlLimit.h" #include "lua/utils.h" #include "lua/trigger.h" @@ -286,32 +287,7 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i) for (uint32_t j = 0; j < index_def->key_def->part_count; j++) { lua_pushnumber(L, j + 1); - lua_newtable(L); - const struct key_part *part = - &index_def->key_def->parts[j]; - - lua_pushstring(L, field_type_strs[part->type]); - lua_setfield(L, -2, "type"); - - lua_pushnumber(L, part->fieldno + TUPLE_INDEX_BASE); - lua_setfield(L, -2, "fieldno"); - - if (part->path != NULL) { - lua_pushlstring(L, part->path, part->path_len); - lua_setfield(L, -2, "path"); - } - - lua_pushboolean(L, key_part_is_nullable(part)); - lua_setfield(L, -2, "is_nullable"); - - if (part->coll_id != COLL_NONE) { - struct coll_id *coll_id = - coll_by_id(part->coll_id); - assert(coll_id != NULL); - lua_pushstring(L, coll_id->name); - lua_setfield(L, -2, "collation"); - } - + lbox_push_key_part(L, &index_def->key_def->parts[j]); lua_settable(L, -3); /* index[k].parts[j] */ } diff --git a/src/box/tuple.c b/src/box/tuple.c index 7f06d4053..350718eaa 100644 --- a/src/box/tuple.c +++ b/src/box/tuple.c @@ -725,6 +725,30 @@ box_tuple_new(box_tuple_format_t *format, const char *data, const char *end) return tuple_bless(ret); } +int +box_tuple_validate_parts(const box_tuple_t *tuple, struct key_def *key_def) +{ + uint32_t min_field_count = + tuple_format_min_field_count(&key_def, 1, NULL, 0); + uint32_t field_count = tuple_field_count(tuple); + if (field_count < min_field_count) { + diag_set(ClientError, ER_NO_SUCH_FIELD_NO, field_count + 1); + return -1; + } + for (uint32_t idx = 0; idx < key_def->part_count; idx++) { + struct key_part *part = &key_def->parts[idx]; + const char *field = tuple_field_by_part(tuple, part); + if (field == NULL) { + assert(key_def->has_optional_parts); + continue; + } + if (key_part_validate(part->type, field, idx, + key_part_is_nullable(part)) != 0) + return -1; + } + return 0; +} + /* }}} box_tuple_* */ int diff --git a/src/box/tuple.h b/src/box/tuple.h index 8b12fd5a8..32503b21c 100644 --- a/src/box/tuple.h +++ b/src/box/tuple.h @@ -284,6 +284,18 @@ box_tuple_t * box_tuple_upsert(const box_tuple_t *tuple, const char *expr, const char *expr_end); +/** + * Check that tuple match with the key definition. + * + * \param key_def Key definition. + * \param tuple Tuple for matching. + * \param allow_nullable True if nullable parts are allowed. + * \retval 0 The tuple is valid. + * \retval -1 The tuple is invalid. + */ +int +box_tuple_validate_parts(const box_tuple_t *tuple, struct key_def *key_def); + /** \endcond public */ /** diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua new file mode 100755 index 000000000..756520b8c --- /dev/null +++ b/test/box-tap/key_def.test.lua @@ -0,0 +1,370 @@ +#!/usr/bin/env tarantool + +local tap = require('tap') +local ffi = require('ffi') +local json = require('json') +local fun = require('fun') +local key_def_lib = require('key_def') + +local usage_error = 'Bad params, use: key_def.new({' .. + '{fieldno = fieldno, type = type' .. + '[, is_nullable = <boolean>]' .. + '[, path = <string>]' .. + '[, collation_id = <number>]' .. + '[, collation = <string>]}, ...}' + +local function coll_not_found(fieldno, collation) + if type(collation) == 'number' then + return ('Wrong index options (field %d): ' .. + 'collation was not found by ID'):format(fieldno) + end + + return ('Unknown collation: "%s"'):format(collation) +end + +local function set_key_part_defaults(parts) + local res = {} + for i, part in ipairs(parts) do + res[i] = table.copy(part) + if res[i].is_nullable == nil then + res[i].is_nullable = false + end + end + return res +end + +local key_def_new_cases = { + -- Cases to call before box.cfg{}. + { + 'Pass a field on an unknown type', + parts = {{ + fieldno = 2, + type = 'unknown', + }}, + exp_err = 'Unknown field type: unknown', + }, + { + 'Try to use collation_id before box.cfg{}', + parts = {{ + fieldno = 1, + type = 'string', + collation_id = 2, + }}, + exp_err = coll_not_found(1, 2), + }, + { + 'Try to use collation before box.cfg{}', + parts = {{ + fieldno = 1, + type = 'string', + collation = 'unicode_ci', + }}, + exp_err = coll_not_found(1, 'unicode_ci'), + }, + function() + -- For collations. + box.cfg{} + end, + -- Cases to call after box.cfg{}. + { + 'Try to use both collation_id and collation', + parts = {{ + fieldno = 1, + type = 'string', + collation_id = 2, + collation = 'unicode_ci', + }}, + exp_err = 'Conflicting options: collation_id and collation', + }, + { + 'Unknown collation_id', + parts = {{ + fieldno = 1, + type = 'string', + collation_id = 42, + }}, + exp_err = coll_not_found(1, 42), + }, + { + 'Unknown collation name', + parts = {{ + fieldno = 1, + type = 'string', + collation = 'unknown', + }}, + exp_err = 'Unknown collation: "unknown"', + }, + { + 'Bad parts parameter type', + parts = 1, + exp_err = usage_error, + }, + { + 'No parameters', + params = {}, + exp_err = usage_error, + }, + { + 'Two parameters', + params = {{}, {}}, + exp_err = usage_error, + }, + { + 'Invalid JSON path', + parts = {{ + fieldno = 1, + type = 'string', + path = '[3[', + }}, + exp_err = 'Wrong index options (field 1): invalid path', + }, + { + 'Success case; zero parts', + parts = {}, + exp_err = nil, + }, + { + 'Success case; one part', + parts = {{ + fieldno = 1, + type = 'string', + }}, + exp_err = nil, + }, + { + 'Success case; one part with a JSON path', + parts = {{ + fieldno = 1, + type = 'string', + path = '[3]', + }}, + exp_err = nil, + }, +} + +local test = tap.test('key_def') + +test:plan(#key_def_new_cases - 1 + 7) +for _, case in ipairs(key_def_new_cases) do + if type(case) == 'function' then + case() + else + local ok, res + if case.params then + ok, res = pcall(key_def_lib.new, unpack(case.params)) + else + ok, res = pcall(key_def_lib.new, case.parts) + end + if case.exp_err == nil then + ok = ok and type(res) == 'cdata' and + ffi.istype('struct key_def', res) + test:ok(ok, case[1]) + else + local err = tostring(res) -- cdata -> string + test:is_deeply({ok, err}, {false, case.exp_err}, case[1]) + end + end +end + +-- Prepare source data for test cases. +local parts_a = { + {type = 'unsigned', fieldno = 1}, +} +local parts_b = { + {type = 'number', fieldno = 2}, + {type = 'number', fieldno = 3}, +} +local parts_c = { + {type = 'scalar', fieldno = 2}, + {type = 'scalar', fieldno = 1}, + {type = 'string', fieldno = 4, is_nullable = true}, +} +local key_def_a = key_def_lib.new(parts_a) +local key_def_b = key_def_lib.new(parts_b) +local key_def_c = key_def_lib.new(parts_c) +local tuple_a = box.tuple.new({1, 1, 22}) +local tuple_b = box.tuple.new({2, 1, 11}) +local tuple_c = box.tuple.new({3, 1, 22}) + +-- Case: extract_key(). +test:test('extract_key()', function(test) + test:plan(9) + + test:is_deeply(key_def_a:extract_key(tuple_a):totable(), {1}, 'case 1') + test:is_deeply(key_def_b:extract_key(tuple_a):totable(), {1, 22}, 'case 2') + + -- JSON path. + local res = key_def_lib.new({ + {type = 'string', fieldno = 1, path = 'a.b'}, + }):extract_key(box.tuple.new({{a = {b = 'foo'}}})):totable() + test:is_deeply(res, {'foo'}, 'JSON path (tuple argument)') + + local res = key_def_lib.new({ + {type = 'string', fieldno = 1, path = 'a.b'}, + }):extract_key({{a = {b = 'foo'}}}):totable() + test:is_deeply(res, {'foo'}, 'JSON path (table argument)') + + -- A key def has a **nullable** part with a field that is over + -- a tuple size. + -- + -- The key def options are: + -- + -- * is_nullable = true; + -- * has_optional_parts = true. + test:is_deeply(key_def_c:extract_key(tuple_a):totable(), {1, 1, box.NULL}, + 'short tuple with a nullable part') + + -- A key def has a **non-nullable** part with a field that is + -- over a tuple size. + -- + -- The key def options are: + -- + -- * is_nullable = false; + -- * has_optional_parts = false. + local exp_err = 'Field 2 was not found in the tuple' + local key_def = key_def_lib.new({ + {type = 'string', fieldno = 1}, + {type = 'string', fieldno = 2}, + }) + local ok, err = pcall(key_def.extract_key, key_def, + box.tuple.new({'foo'})) + test:is_deeply({ok, tostring(err)}, {false, exp_err}, + 'short tuple with a non-nullable part (case 1)') + + -- Same as before, but a max fieldno is over tuple:len() + 1. + local exp_err = 'Field 2 was not found in the tuple' + local key_def = key_def_lib.new({ + {type = 'string', fieldno = 1}, + {type = 'string', fieldno = 2}, + {type = 'string', fieldno = 3}, + }) + local ok, err = pcall(key_def.extract_key, key_def, + box.tuple.new({'foo'})) + test:is_deeply({ok, tostring(err)}, {false, exp_err}, + 'short tuple with a non-nullable part (case 2)') + + -- Same as before, but with another key def options: + -- + -- * is_nullable = true; + -- * has_optional_parts = false. + local exp_err = 'Field 2 was not found in the tuple' + local key_def = key_def_lib.new({ + {type = 'string', fieldno = 1, is_nullable = true}, + {type = 'string', fieldno = 2}, + }) + local ok, err = pcall(key_def.extract_key, key_def, + box.tuple.new({'foo'})) + test:is_deeply({ok, tostring(err)}, {false, exp_err}, + 'short tuple with a non-nullable part (case 3)') + + -- A tuple has a field that does not match corresponding key + -- part type. + local exp_err = 'Supplied key type of part 2 does not match index ' .. + 'part type: expected string' + local key_def = key_def_lib.new({ + {type = 'string', fieldno = 1}, + {type = 'string', fieldno = 2}, + {type = 'string', fieldno = 3}, + }) + local ok, err = pcall(key_def.extract_key, key_def, {'one', 'two', 3}) + test:is_deeply({ok, tostring(err)}, {false, exp_err}, + 'wrong field type') +end) + +-- Case: compare(). +test:test('compare()', function(test) + test:plan(8) + + test:is(key_def_a:compare(tuple_b, tuple_a), 1, + 'case 1: great (tuple argument)') + test:is(key_def_a:compare(tuple_b, tuple_c), -1, + 'case 2: less (tuple argument)') + test:is(key_def_b:compare(tuple_b, tuple_a), -1, + 'case 3: less (tuple argument)') + test:is(key_def_b:compare(tuple_a, tuple_c), 0, + 'case 4: equal (tuple argument)') + + test:is(key_def_a:compare(tuple_b:totable(), tuple_a:totable()), 1, + 'case 1: great (table argument)') + test:is(key_def_a:compare(tuple_b:totable(), tuple_c:totable()), -1, + 'case 2: less (table argument)') + test:is(key_def_b:compare(tuple_b:totable(), tuple_a:totable()), -1, + 'case 3: less (table argument)') + test:is(key_def_b:compare(tuple_a:totable(), tuple_c:totable()), 0, + 'case 4: equal (table argument)') +end) + +-- Case: compare_with_key(). +test:test('compare_with_key()', function(test) + test:plan(2) + + local key = {1, 22} + test:is(key_def_b:compare_with_key(tuple_a:totable(), key), 0, 'table') + + local key = box.tuple.new({1, 22}) + test:is(key_def_b:compare_with_key(tuple_a, key), 0, 'tuple') +end) + +-- Case: totable(). +test:test('totable()', function(test) + test:plan(2) + + local exp = set_key_part_defaults(parts_a) + test:is_deeply(key_def_a:totable(), exp, 'case 1') + + local exp = set_key_part_defaults(parts_b) + test:is_deeply(key_def_b:totable(), exp, 'case 2') +end) + +-- Case: __serialize(). +test:test('__serialize()', function(test) + test:plan(2) + + local exp = set_key_part_defaults(parts_a) + test:is(json.encode(key_def_a), json.encode(exp), 'case 1') + + local exp = set_key_part_defaults(parts_b) + test:is(json.encode(key_def_b), json.encode(exp), 'case 2') +end) + +-- Case: tostring(). +test:test('tostring()', function(test) + test:plan(2) + + local exp = '<struct key_def &>' + test:is(tostring(key_def_a), exp, 'case 1') + test:is(tostring(key_def_b), exp, 'case 2') +end) + +-- Case: merge(). +test:test('merge()', function(test) + test:plan(6) + + local key_def_ab = key_def_a:merge(key_def_b) + local exp_parts = fun.iter(key_def_a:totable()) + :chain(fun.iter(key_def_b:totable())):totable() + test:is_deeply(key_def_ab:totable(), exp_parts, + 'case 1: verify with :totable()') + test:is_deeply(key_def_ab:extract_key(tuple_a):totable(), {1, 1, 22}, + 'case 1: verify with :extract_key()') + + local key_def_ba = key_def_b:merge(key_def_a) + local exp_parts = fun.iter(key_def_b:totable()) + :chain(fun.iter(key_def_a:totable())):totable() + test:is_deeply(key_def_ba:totable(), exp_parts, + 'case 2: verify with :totable()') + test:is_deeply(key_def_ba:extract_key(tuple_a):totable(), {1, 22, 1}, + 'case 2: verify with :extract_key()') + + -- Intersecting parts + NULL parts. + local key_def_cb = key_def_c:merge(key_def_b) + local exp_parts = key_def_c:totable() + exp_parts[#exp_parts + 1] = {type = 'number', fieldno = 3, + is_nullable = false} + test:is_deeply(key_def_cb:totable(), exp_parts, + 'case 3: verify with :totable()') + test:is_deeply(key_def_cb:extract_key(tuple_a):totable(), + {1, 1, box.NULL, 22}, 'case 3: verify with :extract_key()') +end) + +os.exit(test:check() and 0 or 1) -- 2.21.0 ^ permalink raw reply [flat|nested] 25+ messages in thread
[parent not found: <6d915212-e80f-4a6d-d884-b838bf25f8a7@tarantool.org>]
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module [not found] ` <6d915212-e80f-4a6d-d884-b838bf25f8a7@tarantool.org> @ 2019-03-28 11:22 ` Alexander Turenko 2019-04-03 11:10 ` Vladimir Davydov 0 siblings, 1 reply; 25+ messages in thread From: Alexander Turenko @ 2019-03-28 11:22 UTC (permalink / raw) To: Vladimir Davydov; +Cc: tarantool-patches, Kirill Shcherbatov Changes I made (squashed and force-pushed): * Updated a comment (see below). * Removed #4025 from commit messages as duplicate of #3398. * Fixed the typo: `sk:pairs({1}))` -> `sk:pairs({1})` Vladimir, can you, please, look into the patchset? branch: kshch/gh-4025-lua-key-kef-methods WBR, Alexander Turenko. On Thu, Mar 28, 2019 at 12:01:24PM +0300, Kirill Shcherbatov wrote: > I've decided do not introduce public helper box_tuple_validate_parts, just > inline this code. > ================================================= > > There are several reasons to add this module: > > * Factor out key parts parsing code from the tuples merger (#3276). > * Support comparing a tuple with a key / a tuple, support merging > key_defs from Lua (#3398). > * Support extracting a key from a tuple (#4025). > > The format of `parts` parameter in the `key_def.new(parts)` call is > compatible with the following structures: > > * box.space[...].index[...].parts; > * net_box_conn.space[...].index[...].parts. > > A key_def instance has the following methods: > > * :extract_key(tuple) -> key (as tuple) > * :compare(tuple_a, tuple_b) -> number > * :compare_with_key(tuple, key) -> number > * :merge(another_key_def) -> new key_def instance > * :totable() -> table > > Note functions that accept tuple(s) also allow to pass Lua > table(s) instead. > > Needed for #3276. > Fixes #3398. > Fixes #4025. > > @TarantoolBot document > Title: lua: key_def module > > See the commit message for an API reference. > > Example for extract_key(): > > ```lua > -- Remove values got from a secondary non-unique index. > local key_def_lib = require('key_def') > local s = box.schema.space.create('test') > local pk = s:create_index('pk') > local sk = s:create_index('test', {unique = false, parts = { > {2, 'number', path = 'a'}, {2, 'number', path = 'b'}}}) > s:insert{1, {a = 1, b = 1}} > s:insert{2, {a = 1, b = 2}} > local key_def = key_def_lib.new(pk.parts) > for _, tuple in sk:pairs({1})) do > local key = key_def:extract_key(tuple) > pk:delete(key) > end > ``` > --- > src/CMakeLists.txt | 1 + > src/box/CMakeLists.txt | 2 + > src/box/lua/init.c | 5 + > src/box/lua/key_def.c | 462 ++++++++++++++++++++++++++++++++++ > src/box/lua/key_def.h | 63 +++++ > src/box/lua/key_def.lua | 19 ++ > src/box/lua/space.cc | 28 +-- > test/box-tap/key_def.test.lua | 370 +++++++++++++++++++++++++++ > 8 files changed, 924 insertions(+), 26 deletions(-) > create mode 100644 src/box/lua/key_def.c > create mode 100644 src/box/lua/key_def.h > create mode 100644 src/box/lua/key_def.lua > create mode 100755 test/box-tap/key_def.test.lua > > diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt > index 7c2395517..a6a18142b 100644 > --- a/src/CMakeLists.txt > +++ b/src/CMakeLists.txt > @@ -136,6 +136,7 @@ set(api_headers > ${CMAKE_SOURCE_DIR}/src/lua/string.h > ${CMAKE_SOURCE_DIR}/src/box/txn.h > ${CMAKE_SOURCE_DIR}/src/box/key_def.h > + ${CMAKE_SOURCE_DIR}/src/box/lua/key_def.h > ${CMAKE_SOURCE_DIR}/src/box/field_def.h > ${CMAKE_SOURCE_DIR}/src/box/tuple.h > ${CMAKE_SOURCE_DIR}/src/box/tuple_format.h > diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt > index 59e91b65a..906ce22e6 100644 > --- a/src/box/CMakeLists.txt > +++ b/src/box/CMakeLists.txt > @@ -12,6 +12,7 @@ lua_source(lua_sources lua/net_box.lua) > lua_source(lua_sources lua/upgrade.lua) > lua_source(lua_sources lua/console.lua) > lua_source(lua_sources lua/xlog.lua) > +lua_source(lua_sources lua/key_def.lua) > set(bin_sources) > bin_source(bin_sources bootstrap.snap bootstrap.h) > > @@ -139,6 +140,7 @@ add_library(box STATIC > lua/net_box.c > lua/xlog.c > lua/sql.c > + lua/key_def.c > ${bin_sources}) > > target_link_libraries(box box_error tuple stat xrow xlog vclock crc32 scramble > diff --git a/src/box/lua/init.c b/src/box/lua/init.c > index 744b2c895..68ef58909 100644 > --- a/src/box/lua/init.c > +++ b/src/box/lua/init.c > @@ -59,9 +59,11 @@ > #include "box/lua/console.h" > #include "box/lua/tuple.h" > #include "box/lua/sql.h" > +#include "box/lua/key_def.h" > > extern char session_lua[], > tuple_lua[], > + key_def_lua[], > schema_lua[], > load_cfg_lua[], > xlog_lua[], > @@ -80,6 +82,7 @@ static const char *lua_sources[] = { > "box/console", console_lua, > "box/load_cfg", load_cfg_lua, > "box/xlog", xlog_lua, > + "box/key_def", key_def_lua, > NULL > }; > > @@ -312,6 +315,8 @@ box_lua_init(struct lua_State *L) > lua_pop(L, 1); > tarantool_lua_console_init(L); > lua_pop(L, 1); > + luaopen_key_def(L); > + lua_pop(L, 1); > > /* Load Lua extension */ > for (const char **s = lua_sources; *s; s += 2) { > diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c > new file mode 100644 > index 000000000..4df6c2045 > --- /dev/null > +++ b/src/box/lua/key_def.c > @@ -0,0 +1,462 @@ > +/* > + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file. > + * > + * Redistribution and use in source and binary forms, with or > + * without modification, are permitted provided that the following > + * conditions are met: > + * > + * 1. Redistributions of source code must retain the above > + * copyright notice, this list of conditions and the > + * following disclaimer. > + * > + * 2. Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following > + * disclaimer in the documentation and/or other materials > + * provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED > + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL > + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, > + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF > + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#include "box/coll_id_cache.h" > +#include "box/lua/key_def.h" > +#include "box/tuple.h" > +#include "diag.h" > +#include "fiber.h" > +#include "lua/utils.h" > +#include "tuple.h" > + > +static uint32_t key_def_type_id = 0; > + > +/** > + * Set key_part_def from a table on top of a Lua stack. > + * > + * When successful return 0, otherwise return -1 and set a diag. > + */ > +static int > +luaT_key_def_set_part(struct lua_State *L, struct key_part_def *parts, > + int part_idx, struct region *region) > +{ > + struct key_part_def *part = &parts[part_idx]; > + *part = key_part_def_default; > + > + /* Set part->fieldno. */ > + lua_pushstring(L, "fieldno"); > + lua_gettable(L, -2); > + if (lua_isnil(L, -1)) { > + diag_set(IllegalParams, "fieldno must not be nil"); > + return -1; > + } > + /* > + * Transform one-based Lua fieldno to zero-based > + * fieldno to use in key_def_new(). > + */ > + part->fieldno = lua_tointeger(L, -1) - TUPLE_INDEX_BASE; > + lua_pop(L, 1); > + > + /* Set part->type. */ > + lua_pushstring(L, "type"); > + lua_gettable(L, -2); > + if (lua_isnil(L, -1)) { > + diag_set(IllegalParams, "type must not be nil"); > + return -1; > + } > + size_t type_len; > + const char *type_name = lua_tolstring(L, -1, &type_len); > + lua_pop(L, 1); > + part->type = field_type_by_name(type_name, type_len); > + switch (part->type) { > + case FIELD_TYPE_ANY: > + case FIELD_TYPE_ARRAY: > + case FIELD_TYPE_MAP: > + /* Tuple comparators don't support these types. */ > + diag_set(IllegalParams, "Unsupported field type: %s", > + type_name); > + return -1; > + case field_type_MAX: > + diag_set(IllegalParams, "Unknown field type: %s", type_name); > + return -1; > + default: > + /* Pass though. */ > + break; > + } > + > + /* Set part->is_nullable and part->nullable_action. */ > + lua_pushstring(L, "is_nullable"); > + lua_gettable(L, -2); > + if (!lua_isnil(L, -1) && lua_toboolean(L, -1) != 0) { > + part->is_nullable = true; > + part->nullable_action = ON_CONFLICT_ACTION_NONE; > + } > + lua_pop(L, 1); > + > + /* > + * Set part->coll_id using collation_id. > + * > + * The value will be checked in key_def_new(). > + */ > + lua_pushstring(L, "collation_id"); > + lua_gettable(L, -2); > + if (!lua_isnil(L, -1)) > + part->coll_id = lua_tointeger(L, -1); > + lua_pop(L, 1); > + > + /* Set part->coll_id using collation. */ > + lua_pushstring(L, "collation"); > + lua_gettable(L, -2); > + if (!lua_isnil(L, -1)) { > + /* Check for conflicting options. */ > + if (part->coll_id != COLL_NONE) { > + diag_set(IllegalParams, "Conflicting options: " > + "collation_id and collation"); > + return -1; > + } > + > + size_t coll_name_len; > + const char *coll_name = lua_tolstring(L, -1, &coll_name_len); > + struct coll_id *coll_id = coll_by_name(coll_name, > + coll_name_len); > + if (coll_id == NULL) { > + diag_set(IllegalParams, "Unknown collation: \"%s\"", > + coll_name); > + return -1; > + } > + part->coll_id = coll_id->id; > + } > + lua_pop(L, 1); > + > + /* Set part->path (JSON path). */ > + lua_pushstring(L, "path"); > + lua_gettable(L, -2); > + if (!lua_isnil(L, -1)) { > + size_t path_len; > + const char *path = lua_tolstring(L, -1, &path_len); > + if (json_path_validate(path, path_len, TUPLE_INDEX_BASE) != 0) { > + diag_set(ClientError, ER_WRONG_INDEX_OPTIONS, > + part_idx + TUPLE_INDEX_BASE, "invalid path"); > + return -1; > + } > + char *tmp = region_alloc(region, path_len + 1); > + if (tmp == NULL) { > + diag_set(OutOfMemory, path_len + 1, "region", "path"); > + return -1; > + } > + /* > + * lua_tolstring() guarantees that a string have > + * trailing '\0'. > + */ > + memcpy(tmp, path, path_len + 1); > + part->path = tmp; > + } else { > + part->path = NULL; > + } > + lua_pop(L, 1); > + return 0; > +} > + > +void > +lbox_push_key_part(struct lua_State *L, const struct key_part *part) > +{ > + lua_newtable(L); > + > + lua_pushstring(L, field_type_strs[part->type]); > + lua_setfield(L, -2, "type"); > + > + lua_pushnumber(L, part->fieldno + TUPLE_INDEX_BASE); > + lua_setfield(L, -2, "fieldno"); > + > + if (part->path != NULL) { > + lua_pushlstring(L, part->path, part->path_len); > + lua_setfield(L, -2, "path"); > + } > + > + lua_pushboolean(L, key_part_is_nullable(part)); > + lua_setfield(L, -2, "is_nullable"); > + > + if (part->coll_id != COLL_NONE) { > + struct coll_id *coll_id = coll_by_id(part->coll_id); > + assert(coll_id != NULL); > + lua_pushstring(L, coll_id->name); > + lua_setfield(L, -2, "collation"); > + } > +} > + > +struct key_def * > +check_key_def(struct lua_State *L, int idx) > +{ > + if (lua_type(L, idx) != LUA_TCDATA) > + return NULL; > + > + uint32_t cdata_type; > + struct key_def **key_def_ptr = luaL_checkcdata(L, idx, &cdata_type); > + if (key_def_ptr == NULL || cdata_type != key_def_type_id) > + return NULL; > + return *key_def_ptr; > +} > + > +/** > + * Free a key_def from a Lua code. > + */ > +static int > +lbox_key_def_gc(struct lua_State *L) > +{ > + struct key_def *key_def = check_key_def(L, 1); > + if (key_def == NULL) > + return 0; > + box_key_def_delete(key_def); > + return 0; > +} > + > +/** > + * Take existent tuple from LUA stack or build a new tuple with > + * default format from table, check for compatibility with a > + * given key_def. Take tuple reference pointer on success. > + */ Updated a bit: diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c index 4df6c2045..dc610007b 100644 --- a/src/box/lua/key_def.c +++ b/src/box/lua/key_def.c @@ -219,9 +219,13 @@ lbox_key_def_gc(struct lua_State *L) } /** - * Take existent tuple from LUA stack or build a new tuple with - * default format from table, check for compatibility with a - * given key_def. Take tuple reference pointer on success. + * Validate a tuple from a given index on a Lua stack against + * a key def and return the tuple. + * + * If a table is passed instead of a tuple, create a new tuple + * from it. + * + * Return a refcounted tuple (either provided one or a new one). */ static struct tuple * lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) > +static struct tuple * > +lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) > +{ > + struct tuple *tuple = luaT_istuple(L, idx); > + if (tuple == NULL) > + tuple = luaT_tuple_new(L, idx, box_tuple_format_default()); > + if (tuple == NULL) > + return NULL; > + /* Check that tuple match with the key definition. */ > + uint32_t min_field_count = > + tuple_format_min_field_count(&key_def, 1, NULL, 0); > + uint32_t field_count = tuple_field_count(tuple); > + if (field_count < min_field_count) { > + diag_set(ClientError, ER_NO_SUCH_FIELD_NO, field_count + 1); > + return NULL; > + } > + for (uint32_t idx = 0; idx < key_def->part_count; idx++) { > + struct key_part *part = &key_def->parts[idx]; > + const char *field = tuple_field_by_part(tuple, part); > + if (field == NULL) { > + assert(key_def->has_optional_parts); > + continue; > + } > + if (key_part_validate(part->type, field, idx, > + key_part_is_nullable(part)) != 0) > + return NULL; > + } > + tuple_ref(tuple); > + return tuple; > +} > + > +static int > +lbox_key_def_extract_key(struct lua_State *L) > +{ > + struct key_def *key_def; > + if (lua_gettop(L) != 2 || (key_def = check_key_def(L, 1)) == NULL) > + return luaL_error(L, "Usage: key_def:extract_key(tuple)"); > + > + struct tuple *tuple; > + if ((tuple = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) > + return luaT_error(L); > + > + uint32_t key_size; > + char *key = tuple_extract_key(tuple, key_def, &key_size); > + tuple_unref(tuple); > + if (key == NULL) > + return luaT_error(L); > + > + struct tuple *ret = > + box_tuple_new(box_tuple_format_default(), key, key + key_size); > + if (ret == NULL) > + return luaT_error(L); > + luaT_pushtuple(L, ret); > + return 1; > +} > + > +static int > +lbox_key_def_compare(struct lua_State *L) > +{ > + struct key_def *key_def; > + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL) { > + return luaL_error(L, "Usage: key_def:" > + "compare(tuple_a, tuple_b)"); > + } > + > + struct tuple *tuple_a, *tuple_b; > + if ((tuple_a = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) > + return luaT_error(L); > + if ((tuple_b = lbox_key_def_check_tuple(L, key_def, 3)) == NULL) { > + tuple_unref(tuple_a); > + return luaT_error(L); > + } > + > + int rc = tuple_compare(tuple_a, tuple_b, key_def); > + tuple_unref(tuple_a); > + tuple_unref(tuple_b); > + lua_pushinteger(L, rc); > + return 1; > +} > + > +static int > +lbox_key_def_compare_with_key(struct lua_State *L) > +{ > + struct key_def *key_def; > + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL) { > + return luaL_error(L, "Usage: key_def:" > + "compare_with_key(tuple, key)"); > + } > + > + struct tuple *tuple, *key_tuple = NULL; > + struct tuple_format *format = box_tuple_format_default(); > + if ((tuple = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) > + return luaT_error(L); > + if ((key_tuple = luaT_tuple_new(L, 3, format)) == NULL) { > + tuple_unref(tuple); > + return luaT_error(L); > + } > + tuple_ref(key_tuple); > + > + const char *key = tuple_data(key_tuple); > + assert(mp_typeof(*key) == MP_ARRAY); > + uint32_t part_count = mp_decode_array(&key); > + if (key_validate_parts(key_def, key, part_count, true) != 0) { > + tuple_unref(tuple); > + tuple_unref(key_tuple); > + return luaT_error(L); > + } > + > + int rc = tuple_compare_with_key(tuple, key, part_count, key_def); > + tuple_unref(tuple); > + tuple_unref(key_tuple); > + lua_pushinteger(L, rc); > + return 1; > +} > + > +static int > +lbox_key_def_merge(struct lua_State *L) > +{ > + struct key_def *key_def_a, *key_def_b; > + if (lua_gettop(L) != 2 || (key_def_a = check_key_def(L, 1)) == NULL || > + (key_def_b = check_key_def(L, 2)) == NULL) > + return luaL_error(L, "Usage: key_def:merge(second_key_def)"); > + > + struct key_def *new_key_def = key_def_merge(key_def_a, key_def_b); > + if (new_key_def == NULL) > + return luaT_error(L); > + > + *(struct key_def **) luaL_pushcdata(L, key_def_type_id) = new_key_def; > + lua_pushcfunction(L, lbox_key_def_gc); > + luaL_setcdatagc(L, -2); > + return 1; > +} > + > +static int > +lbox_key_def_to_table(struct lua_State *L) > +{ > + struct key_def *key_def; > + if (lua_gettop(L) != 1 || (key_def = check_key_def(L, 1)) == NULL) > + return luaL_error(L, "Usage: key_def:totable()"); > + > + lua_createtable(L, key_def->part_count, 0); > + for (uint32_t i = 0; i < key_def->part_count; ++i) { > + lbox_push_key_part(L, &key_def->parts[i]); > + lua_rawseti(L, -2, i + 1); > + } > + return 1; > +} > + > +/** > + * Create a new key_def from a Lua table. > + * > + * Expected a table of key parts on the Lua stack. The format is > + * the same as box.space.<...>.index.<...>.parts or corresponding > + * net.box's one. > + * > + * Push the new key_def as cdata to a Lua stack. > + */ > +static int > +lbox_key_def_new(struct lua_State *L) > +{ > + if (lua_gettop(L) != 1 || lua_istable(L, 1) != 1) > + return luaL_error(L, "Bad params, use: key_def.new({" > + "{fieldno = fieldno, type = type" > + "[, is_nullable = <boolean>]" > + "[, path = <string>]" > + "[, collation_id = <number>]" > + "[, collation = <string>]}, ...}"); > + > + uint32_t part_count = lua_objlen(L, 1); > + const ssize_t parts_size = sizeof(struct key_part_def) * part_count; > + > + struct region *region = &fiber()->gc; > + size_t region_svp = region_used(region); > + struct key_part_def *parts = region_alloc(region, parts_size); > + if (parts == NULL) { > + diag_set(OutOfMemory, parts_size, "region", "parts"); > + return luaT_error(L); > + } > + > + for (uint32_t i = 0; i < part_count; ++i) { > + lua_pushinteger(L, i + 1); > + lua_gettable(L, 1); > + if (luaT_key_def_set_part(L, parts, i, region) != 0) { > + region_truncate(region, region_svp); > + return luaT_error(L); > + } > + } > + > + struct key_def *key_def = key_def_new(parts, part_count); > + region_truncate(region, region_svp); > + if (key_def == NULL) > + return luaT_error(L); > + > + /* > + * Calculate minimal field count of tuples with specified > + * key and update key_def optionality to use correct > + * compare/extract functions. > + */ > + uint32_t min_field_count = > + tuple_format_min_field_count(&key_def, 1, NULL, 0); > + key_def_update_optionality(key_def, min_field_count); > + > + *(struct key_def **) luaL_pushcdata(L, key_def_type_id) = key_def; > + lua_pushcfunction(L, lbox_key_def_gc); > + luaL_setcdatagc(L, -2); > + > + return 1; > +} > + > +LUA_API int > +luaopen_key_def(struct lua_State *L) > +{ > + luaL_cdef(L, "struct key_def;"); > + key_def_type_id = luaL_ctypeid(L, "struct key_def&"); > + > + /* Export C functions to Lua. */ > + static const struct luaL_Reg meta[] = { > + {"new", lbox_key_def_new}, > + {NULL, NULL} > + }; > + luaL_register_module(L, "key_def", meta); > + > + lua_newtable(L); /* key_def.internal */ > + lua_pushcfunction(L, lbox_key_def_extract_key); > + lua_setfield(L, -2, "extract_key"); > + lua_pushcfunction(L, lbox_key_def_compare); > + lua_setfield(L, -2, "compare"); > + lua_pushcfunction(L, lbox_key_def_compare_with_key); > + lua_setfield(L, -2, "compare_with_key"); > + lua_pushcfunction(L, lbox_key_def_merge); > + lua_setfield(L, -2, "merge"); > + lua_pushcfunction(L, lbox_key_def_to_table); > + lua_setfield(L, -2, "totable"); > + lua_setfield(L, -2, "internal"); > + > + return 1; > +} > diff --git a/src/box/lua/key_def.h b/src/box/lua/key_def.h > new file mode 100644 > index 000000000..7dc7158e8 > --- /dev/null > +++ b/src/box/lua/key_def.h > @@ -0,0 +1,63 @@ > +#ifndef TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED > +#define TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED > +/* > + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file. > + * > + * Redistribution and use in source and binary forms, with or > + * without modification, are permitted provided that the following > + * conditions are met: > + * > + * 1. Redistributions of source code must retain the above > + * copyright notice, this list of conditions and the > + * following disclaimer. > + * > + * 2. Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following > + * disclaimer in the documentation and/or other materials > + * provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY AUTHORS ``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED > + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL > + * AUTHORS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, > + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF > + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#if defined(__cplusplus) > +extern "C" { > +#endif /* defined(__cplusplus) */ > + > +struct lua_State; > +struct key_part; > + > +/** > + * Push a new table representing a key_part to a Lua stack. > + */ > +void > +lbox_push_key_part(struct lua_State *L, const struct key_part *part); > + > +/** > + * Extract a key_def object from a Lua stack. > + */ > +struct key_def * > +check_key_def(struct lua_State *L, int idx); > + > +/** > + * Register the module. > + */ > +int > +luaopen_key_def(struct lua_State *L); > + > +#if defined(__cplusplus) > +} /* extern "C" */ > +#endif /* defined(__cplusplus) */ > + > +#endif /* TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED */ > diff --git a/src/box/lua/key_def.lua b/src/box/lua/key_def.lua > new file mode 100644 > index 000000000..122243cc4 > --- /dev/null > +++ b/src/box/lua/key_def.lua > @@ -0,0 +1,19 @@ > +local ffi = require('ffi') > +local key_def = require('key_def') > +local key_def_t = ffi.typeof('struct key_def') > + > +local methods = { > + ['extract_key'] = key_def.internal.extract_key, > + ['compare'] = key_def.internal.compare, > + ['compare_with_key'] = key_def.internal.compare_with_key, > + ['merge'] = key_def.internal.merge, > + ['totable'] = key_def.internal.totable, > + ['__serialize'] = key_def.internal.totable, > +} > + > +ffi.metatype(key_def_t, { > + __index = function(self, key) > + return methods[key] > + end, > + __tostring = function(key_def) return "<struct key_def &>" end, > +}) > diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc > index ca793e423..6eecbc756 100644 > --- a/src/box/lua/space.cc > +++ b/src/box/lua/space.cc > @@ -30,6 +30,7 @@ > */ > #include "box/lua/space.h" > #include "box/lua/tuple.h" > +#include "box/lua/key_def.h" > #include "box/sql/sqlLimit.h" > #include "lua/utils.h" > #include "lua/trigger.h" > @@ -286,32 +287,7 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i) > > for (uint32_t j = 0; j < index_def->key_def->part_count; j++) { > lua_pushnumber(L, j + 1); > - lua_newtable(L); > - const struct key_part *part = > - &index_def->key_def->parts[j]; > - > - lua_pushstring(L, field_type_strs[part->type]); > - lua_setfield(L, -2, "type"); > - > - lua_pushnumber(L, part->fieldno + TUPLE_INDEX_BASE); > - lua_setfield(L, -2, "fieldno"); > - > - if (part->path != NULL) { > - lua_pushlstring(L, part->path, part->path_len); > - lua_setfield(L, -2, "path"); > - } > - > - lua_pushboolean(L, key_part_is_nullable(part)); > - lua_setfield(L, -2, "is_nullable"); > - > - if (part->coll_id != COLL_NONE) { > - struct coll_id *coll_id = > - coll_by_id(part->coll_id); > - assert(coll_id != NULL); > - lua_pushstring(L, coll_id->name); > - lua_setfield(L, -2, "collation"); > - } > - > + lbox_push_key_part(L, &index_def->key_def->parts[j]); > lua_settable(L, -3); /* index[k].parts[j] */ > } > > diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua > new file mode 100755 > index 000000000..756520b8c > --- /dev/null > +++ b/test/box-tap/key_def.test.lua > @@ -0,0 +1,370 @@ > +#!/usr/bin/env tarantool > + > +local tap = require('tap') > +local ffi = require('ffi') > +local json = require('json') > +local fun = require('fun') > +local key_def_lib = require('key_def') > + > +local usage_error = 'Bad params, use: key_def.new({' .. > + '{fieldno = fieldno, type = type' .. > + '[, is_nullable = <boolean>]' .. > + '[, path = <string>]' .. > + '[, collation_id = <number>]' .. > + '[, collation = <string>]}, ...}' > + > +local function coll_not_found(fieldno, collation) > + if type(collation) == 'number' then > + return ('Wrong index options (field %d): ' .. > + 'collation was not found by ID'):format(fieldno) > + end > + > + return ('Unknown collation: "%s"'):format(collation) > +end > + > +local function set_key_part_defaults(parts) > + local res = {} > + for i, part in ipairs(parts) do > + res[i] = table.copy(part) > + if res[i].is_nullable == nil then > + res[i].is_nullable = false > + end > + end > + return res > +end > + > +local key_def_new_cases = { > + -- Cases to call before box.cfg{}. > + { > + 'Pass a field on an unknown type', > + parts = {{ > + fieldno = 2, > + type = 'unknown', > + }}, > + exp_err = 'Unknown field type: unknown', > + }, > + { > + 'Try to use collation_id before box.cfg{}', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation_id = 2, > + }}, > + exp_err = coll_not_found(1, 2), > + }, > + { > + 'Try to use collation before box.cfg{}', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation = 'unicode_ci', > + }}, > + exp_err = coll_not_found(1, 'unicode_ci'), > + }, > + function() > + -- For collations. > + box.cfg{} > + end, > + -- Cases to call after box.cfg{}. > + { > + 'Try to use both collation_id and collation', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation_id = 2, > + collation = 'unicode_ci', > + }}, > + exp_err = 'Conflicting options: collation_id and collation', > + }, > + { > + 'Unknown collation_id', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation_id = 42, > + }}, > + exp_err = coll_not_found(1, 42), > + }, > + { > + 'Unknown collation name', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation = 'unknown', > + }}, > + exp_err = 'Unknown collation: "unknown"', > + }, > + { > + 'Bad parts parameter type', > + parts = 1, > + exp_err = usage_error, > + }, > + { > + 'No parameters', > + params = {}, > + exp_err = usage_error, > + }, > + { > + 'Two parameters', > + params = {{}, {}}, > + exp_err = usage_error, > + }, > + { > + 'Invalid JSON path', > + parts = {{ > + fieldno = 1, > + type = 'string', > + path = '[3[', > + }}, > + exp_err = 'Wrong index options (field 1): invalid path', > + }, > + { > + 'Success case; zero parts', > + parts = {}, > + exp_err = nil, > + }, > + { > + 'Success case; one part', > + parts = {{ > + fieldno = 1, > + type = 'string', > + }}, > + exp_err = nil, > + }, > + { > + 'Success case; one part with a JSON path', > + parts = {{ > + fieldno = 1, > + type = 'string', > + path = '[3]', > + }}, > + exp_err = nil, > + }, > +} > + > +local test = tap.test('key_def') > + > +test:plan(#key_def_new_cases - 1 + 7) > +for _, case in ipairs(key_def_new_cases) do > + if type(case) == 'function' then > + case() > + else > + local ok, res > + if case.params then > + ok, res = pcall(key_def_lib.new, unpack(case.params)) > + else > + ok, res = pcall(key_def_lib.new, case.parts) > + end > + if case.exp_err == nil then > + ok = ok and type(res) == 'cdata' and > + ffi.istype('struct key_def', res) > + test:ok(ok, case[1]) > + else > + local err = tostring(res) -- cdata -> string > + test:is_deeply({ok, err}, {false, case.exp_err}, case[1]) > + end > + end > +end > + > +-- Prepare source data for test cases. > +local parts_a = { > + {type = 'unsigned', fieldno = 1}, > +} > +local parts_b = { > + {type = 'number', fieldno = 2}, > + {type = 'number', fieldno = 3}, > +} > +local parts_c = { > + {type = 'scalar', fieldno = 2}, > + {type = 'scalar', fieldno = 1}, > + {type = 'string', fieldno = 4, is_nullable = true}, > +} > +local key_def_a = key_def_lib.new(parts_a) > +local key_def_b = key_def_lib.new(parts_b) > +local key_def_c = key_def_lib.new(parts_c) > +local tuple_a = box.tuple.new({1, 1, 22}) > +local tuple_b = box.tuple.new({2, 1, 11}) > +local tuple_c = box.tuple.new({3, 1, 22}) > + > +-- Case: extract_key(). > +test:test('extract_key()', function(test) > + test:plan(9) > + > + test:is_deeply(key_def_a:extract_key(tuple_a):totable(), {1}, 'case 1') > + test:is_deeply(key_def_b:extract_key(tuple_a):totable(), {1, 22}, 'case 2') > + > + -- JSON path. > + local res = key_def_lib.new({ > + {type = 'string', fieldno = 1, path = 'a.b'}, > + }):extract_key(box.tuple.new({{a = {b = 'foo'}}})):totable() > + test:is_deeply(res, {'foo'}, 'JSON path (tuple argument)') > + > + local res = key_def_lib.new({ > + {type = 'string', fieldno = 1, path = 'a.b'}, > + }):extract_key({{a = {b = 'foo'}}}):totable() > + test:is_deeply(res, {'foo'}, 'JSON path (table argument)') > + > + -- A key def has a **nullable** part with a field that is over > + -- a tuple size. > + -- > + -- The key def options are: > + -- > + -- * is_nullable = true; > + -- * has_optional_parts = true. > + test:is_deeply(key_def_c:extract_key(tuple_a):totable(), {1, 1, box.NULL}, > + 'short tuple with a nullable part') > + > + -- A key def has a **non-nullable** part with a field that is > + -- over a tuple size. > + -- > + -- The key def options are: > + -- > + -- * is_nullable = false; > + -- * has_optional_parts = false. > + local exp_err = 'Field 2 was not found in the tuple' > + local key_def = key_def_lib.new({ > + {type = 'string', fieldno = 1}, > + {type = 'string', fieldno = 2}, > + }) > + local ok, err = pcall(key_def.extract_key, key_def, > + box.tuple.new({'foo'})) > + test:is_deeply({ok, tostring(err)}, {false, exp_err}, > + 'short tuple with a non-nullable part (case 1)') > + > + -- Same as before, but a max fieldno is over tuple:len() + 1. > + local exp_err = 'Field 2 was not found in the tuple' > + local key_def = key_def_lib.new({ > + {type = 'string', fieldno = 1}, > + {type = 'string', fieldno = 2}, > + {type = 'string', fieldno = 3}, > + }) > + local ok, err = pcall(key_def.extract_key, key_def, > + box.tuple.new({'foo'})) > + test:is_deeply({ok, tostring(err)}, {false, exp_err}, > + 'short tuple with a non-nullable part (case 2)') > + > + -- Same as before, but with another key def options: > + -- > + -- * is_nullable = true; > + -- * has_optional_parts = false. > + local exp_err = 'Field 2 was not found in the tuple' > + local key_def = key_def_lib.new({ > + {type = 'string', fieldno = 1, is_nullable = true}, > + {type = 'string', fieldno = 2}, > + }) > + local ok, err = pcall(key_def.extract_key, key_def, > + box.tuple.new({'foo'})) > + test:is_deeply({ok, tostring(err)}, {false, exp_err}, > + 'short tuple with a non-nullable part (case 3)') > + > + -- A tuple has a field that does not match corresponding key > + -- part type. > + local exp_err = 'Supplied key type of part 2 does not match index ' .. > + 'part type: expected string' > + local key_def = key_def_lib.new({ > + {type = 'string', fieldno = 1}, > + {type = 'string', fieldno = 2}, > + {type = 'string', fieldno = 3}, > + }) > + local ok, err = pcall(key_def.extract_key, key_def, {'one', 'two', 3}) > + test:is_deeply({ok, tostring(err)}, {false, exp_err}, > + 'wrong field type') > +end) > + > +-- Case: compare(). > +test:test('compare()', function(test) > + test:plan(8) > + > + test:is(key_def_a:compare(tuple_b, tuple_a), 1, > + 'case 1: great (tuple argument)') > + test:is(key_def_a:compare(tuple_b, tuple_c), -1, > + 'case 2: less (tuple argument)') > + test:is(key_def_b:compare(tuple_b, tuple_a), -1, > + 'case 3: less (tuple argument)') > + test:is(key_def_b:compare(tuple_a, tuple_c), 0, > + 'case 4: equal (tuple argument)') > + > + test:is(key_def_a:compare(tuple_b:totable(), tuple_a:totable()), 1, > + 'case 1: great (table argument)') > + test:is(key_def_a:compare(tuple_b:totable(), tuple_c:totable()), -1, > + 'case 2: less (table argument)') > + test:is(key_def_b:compare(tuple_b:totable(), tuple_a:totable()), -1, > + 'case 3: less (table argument)') > + test:is(key_def_b:compare(tuple_a:totable(), tuple_c:totable()), 0, > + 'case 4: equal (table argument)') > +end) > + > +-- Case: compare_with_key(). > +test:test('compare_with_key()', function(test) > + test:plan(2) > + > + local key = {1, 22} > + test:is(key_def_b:compare_with_key(tuple_a:totable(), key), 0, 'table') > + > + local key = box.tuple.new({1, 22}) > + test:is(key_def_b:compare_with_key(tuple_a, key), 0, 'tuple') > +end) > + > +-- Case: totable(). > +test:test('totable()', function(test) > + test:plan(2) > + > + local exp = set_key_part_defaults(parts_a) > + test:is_deeply(key_def_a:totable(), exp, 'case 1') > + > + local exp = set_key_part_defaults(parts_b) > + test:is_deeply(key_def_b:totable(), exp, 'case 2') > +end) > + > +-- Case: __serialize(). > +test:test('__serialize()', function(test) > + test:plan(2) > + > + local exp = set_key_part_defaults(parts_a) > + test:is(json.encode(key_def_a), json.encode(exp), 'case 1') > + > + local exp = set_key_part_defaults(parts_b) > + test:is(json.encode(key_def_b), json.encode(exp), 'case 2') > +end) > + > +-- Case: tostring(). > +test:test('tostring()', function(test) > + test:plan(2) > + > + local exp = '<struct key_def &>' > + test:is(tostring(key_def_a), exp, 'case 1') > + test:is(tostring(key_def_b), exp, 'case 2') > +end) > + > +-- Case: merge(). > +test:test('merge()', function(test) > + test:plan(6) > + > + local key_def_ab = key_def_a:merge(key_def_b) > + local exp_parts = fun.iter(key_def_a:totable()) > + :chain(fun.iter(key_def_b:totable())):totable() > + test:is_deeply(key_def_ab:totable(), exp_parts, > + 'case 1: verify with :totable()') > + test:is_deeply(key_def_ab:extract_key(tuple_a):totable(), {1, 1, 22}, > + 'case 1: verify with :extract_key()') > + > + local key_def_ba = key_def_b:merge(key_def_a) > + local exp_parts = fun.iter(key_def_b:totable()) > + :chain(fun.iter(key_def_a:totable())):totable() > + test:is_deeply(key_def_ba:totable(), exp_parts, > + 'case 2: verify with :totable()') > + test:is_deeply(key_def_ba:extract_key(tuple_a):totable(), {1, 22, 1}, > + 'case 2: verify with :extract_key()') > + > + -- Intersecting parts + NULL parts. > + local key_def_cb = key_def_c:merge(key_def_b) > + local exp_parts = key_def_c:totable() > + exp_parts[#exp_parts + 1] = {type = 'number', fieldno = 3, > + is_nullable = false} > + test:is_deeply(key_def_cb:totable(), exp_parts, > + 'case 3: verify with :totable()') > + test:is_deeply(key_def_cb:extract_key(tuple_a):totable(), > + {1, 1, box.NULL, 22}, 'case 3: verify with :extract_key()') > +end) > + > +os.exit(test:check() and 0 or 1) > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-03-28 11:22 ` Alexander Turenko @ 2019-04-03 11:10 ` Vladimir Davydov 2019-04-03 11:46 ` Alexander Turenko 2019-04-04 5:07 ` Alexander Turenko 0 siblings, 2 replies; 25+ messages in thread From: Vladimir Davydov @ 2019-04-03 11:10 UTC (permalink / raw) To: Alexander Turenko, Kirill Shcherbatov; +Cc: tarantool-patches On Thu, Mar 28, 2019 at 02:22:01PM +0300, Alexander Turenko wrote: > Changes I made (squashed and force-pushed): > > * Updated a comment (see below). > * Removed #4025 from commit messages as duplicate of #3398. > * Fixed the typo: `sk:pairs({1}))` -> `sk:pairs({1})` > > Vladimir, can you, please, look into the patchset? Patch 1 looks good to me. Pasting patch 2 here for review: > From 0ba422c6f9a4daa4716275ebb256a9e76e49aea1 Mon Sep 17 00:00:00 2001 > From: Kirill Shcherbatov <kshcherbatov@tarantool.org> > Date: Mon, 7 Jan 2019 19:12:50 +0300 > Subject: [PATCH] lua: add key_def lua module > > There are several reasons to add this module: > > * Factor out key parts parsing code from the tuples merger (#3276). > * Support comparing a tuple with a key / a tuple, support merging > key_defs from Lua (#3398). > * Support extracting a key from a tuple (#4025). > > The format of `parts` parameter in the `key_def.new(parts)` call is > compatible with the following structures: > > * box.space[...].index[...].parts; > * net_box_conn.space[...].index[...].parts. > > A key_def instance has the following methods: > > * :extract_key(tuple) -> key (as tuple) > * :compare(tuple_a, tuple_b) -> number > * :compare_with_key(tuple, key) -> number What number do these functions return? > * :merge(another_key_def) -> new key_def instance What does 'merge' do? > * :totable() -> table Does this function return key_def parts? In what format? Please elaborate the comments. Also, key_def.compare() sounds like it compares key definitions, not tuples. May be, we should move these functions to box.tuple module? Also, returning 1, 0, -1 to Lua looks uncommon. May be, we'd better introduce 'equal', 'greater', 'less', etc helpers returning bool? I'm not strongly against the proposed API, but I think we should agree on it with other members of the team, potential users, and Kostja. Also, how is this module supposed to handle multikey indexes? > > Note functions that accept tuple(s) also allow to pass Lua > table(s) instead. > > Needed for #3276. > Fixes #3398. > Fixes #4025. > > @TarantoolBot document > Title: lua: key_def module > > See the commit message for an API reference. Rather than referring to the commit message, please move @TarantoolBot request above. > > Example for extract_key(): > > ```lua > -- Remove values got from a secondary non-unique index. > local key_def_lib = require('key_def') > local s = box.schema.space.create('test') > local pk = s:create_index('pk') > local sk = s:create_index('test', {unique = false, parts = { > {2, 'number', path = 'a'}, {2, 'number', path = 'b'}}}) > s:insert{1, {a = 1, b = 1}} > s:insert{2, {a = 1, b = 2}} > local key_def = key_def_lib.new(pk.parts) > for _, tuple in sk:pairs({1})) do > local key = key_def:extract_key(tuple) > pk:delete(key) > end > ``` > > diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c > new file mode 100644 > index 00000000..4df6c204 > --- /dev/null > +++ b/src/box/lua/key_def.c > +/** > + * Set key_part_def from a table on top of a Lua stack. > + * > + * When successful return 0, otherwise return -1 and set a diag. > + */ > +static int > +luaT_key_def_set_part(struct lua_State *L, struct key_part_def *parts, > + int part_idx, struct region *region) What is the region needed for? Please mention in the comment. > +{ > + struct key_part_def *part = &parts[part_idx]; > + *part = key_part_def_default; > + > + /* Set part->fieldno. */ > + lua_pushstring(L, "fieldno"); > + lua_gettable(L, -2); > + if (lua_isnil(L, -1)) { > + diag_set(IllegalParams, "fieldno must not be nil"); > + return -1; > + } > + /* > + * Transform one-based Lua fieldno to zero-based > + * fieldno to use in key_def_new(). > + */ > + part->fieldno = lua_tointeger(L, -1) - TUPLE_INDEX_BASE; > + lua_pop(L, 1); > + > + /* Set part->type. */ > + lua_pushstring(L, "type"); > + lua_gettable(L, -2); > + if (lua_isnil(L, -1)) { > + diag_set(IllegalParams, "type must not be nil"); > + return -1; > + } > + size_t type_len; > + const char *type_name = lua_tolstring(L, -1, &type_len); > + lua_pop(L, 1); > + part->type = field_type_by_name(type_name, type_len); > + switch (part->type) { > + case FIELD_TYPE_ANY: > + case FIELD_TYPE_ARRAY: > + case FIELD_TYPE_MAP: > + /* Tuple comparators don't support these types. */ > + diag_set(IllegalParams, "Unsupported field type: %s", > + type_name); > + return -1; > + case field_type_MAX: > + diag_set(IllegalParams, "Unknown field type: %s", type_name); > + return -1; > + default: > + /* Pass though. */ > + break; > + } > + > + /* Set part->is_nullable and part->nullable_action. */ > + lua_pushstring(L, "is_nullable"); > + lua_gettable(L, -2); > + if (!lua_isnil(L, -1) && lua_toboolean(L, -1) != 0) { > + part->is_nullable = true; > + part->nullable_action = ON_CONFLICT_ACTION_NONE; > + } > + lua_pop(L, 1); > + > + /* > + * Set part->coll_id using collation_id. > + * > + * The value will be checked in key_def_new(). > + */ > + lua_pushstring(L, "collation_id"); > + lua_gettable(L, -2); > + if (!lua_isnil(L, -1)) > + part->coll_id = lua_tointeger(L, -1); > + lua_pop(L, 1); > + > + /* Set part->coll_id using collation. */ > + lua_pushstring(L, "collation"); > + lua_gettable(L, -2); > + if (!lua_isnil(L, -1)) { > + /* Check for conflicting options. */ > + if (part->coll_id != COLL_NONE) { > + diag_set(IllegalParams, "Conflicting options: " > + "collation_id and collation"); > + return -1; > + } > + > + size_t coll_name_len; > + const char *coll_name = lua_tolstring(L, -1, &coll_name_len); > + struct coll_id *coll_id = coll_by_name(coll_name, > + coll_name_len); > + if (coll_id == NULL) { > + diag_set(IllegalParams, "Unknown collation: \"%s\"", > + coll_name); > + return -1; > + } > + part->coll_id = coll_id->id; > + } > + lua_pop(L, 1); > + > + /* Set part->path (JSON path). */ > + lua_pushstring(L, "path"); > + lua_gettable(L, -2); > + if (!lua_isnil(L, -1)) { > + size_t path_len; > + const char *path = lua_tolstring(L, -1, &path_len); > + if (json_path_validate(path, path_len, TUPLE_INDEX_BASE) != 0) { > + diag_set(ClientError, ER_WRONG_INDEX_OPTIONS, > + part_idx + TUPLE_INDEX_BASE, "invalid path"); Mixing ER_WRONG_INDEX_OPTIONS and IllegalParams looks ugly. Please fix. > + return -1; > + } > + char *tmp = region_alloc(region, path_len + 1); > + if (tmp == NULL) { > + diag_set(OutOfMemory, path_len + 1, "region", "path"); > + return -1; > + } > + /* > + * lua_tolstring() guarantees that a string have > + * trailing '\0'. > + */ > + memcpy(tmp, path, path_len + 1); > + part->path = tmp; > + } else { > + part->path = NULL; > + } > + lua_pop(L, 1); > + return 0; > +} > + > +void > +lbox_push_key_part(struct lua_State *L, const struct key_part *part) > +{ > + lua_newtable(L); > + > + lua_pushstring(L, field_type_strs[part->type]); > + lua_setfield(L, -2, "type"); > + > + lua_pushnumber(L, part->fieldno + TUPLE_INDEX_BASE); > + lua_setfield(L, -2, "fieldno"); > + > + if (part->path != NULL) { > + lua_pushlstring(L, part->path, part->path_len); > + lua_setfield(L, -2, "path"); > + } > + > + lua_pushboolean(L, key_part_is_nullable(part)); > + lua_setfield(L, -2, "is_nullable"); > + > + if (part->coll_id != COLL_NONE) { > + struct coll_id *coll_id = coll_by_id(part->coll_id); > + assert(coll_id != NULL); > + lua_pushstring(L, coll_id->name); > + lua_setfield(L, -2, "collation"); > + } > +} > + > +struct key_def * > +check_key_def(struct lua_State *L, int idx) Please prefix the name with lbox_ or... I dunno - the naming looks inconsistent: luaT_key_def_set_part, lbox_push_key_part, check_key_def. Is there some kind of pattern? > +{ > + if (lua_type(L, idx) != LUA_TCDATA) > + return NULL; > + > + uint32_t cdata_type; > + struct key_def **key_def_ptr = luaL_checkcdata(L, idx, &cdata_type); > + if (key_def_ptr == NULL || cdata_type != key_def_type_id) > + return NULL; > + return *key_def_ptr; > +} > + > +/** > + * Free a key_def from a Lua code. > + */ > +static int > +lbox_key_def_gc(struct lua_State *L) > +{ > + struct key_def *key_def = check_key_def(L, 1); > + if (key_def == NULL) > + return 0; > + box_key_def_delete(key_def); > + return 0; > +} > + > +/** > + * Take existent tuple from LUA stack or build a new tuple with > + * default format from table, check for compatibility with a > + * given key_def. Take tuple reference pointer on success. > + */ > +static struct tuple * > +lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) > +{ > + struct tuple *tuple = luaT_istuple(L, idx); > + if (tuple == NULL) > + tuple = luaT_tuple_new(L, idx, box_tuple_format_default()); > + if (tuple == NULL) > + return NULL; > + /* Check that tuple match with the key definition. */ > + uint32_t min_field_count = > + tuple_format_min_field_count(&key_def, 1, NULL, 0); > + uint32_t field_count = tuple_field_count(tuple); > + if (field_count < min_field_count) { > + diag_set(ClientError, ER_NO_SUCH_FIELD_NO, field_count + 1); > + return NULL; > + } > + for (uint32_t idx = 0; idx < key_def->part_count; idx++) { > + struct key_part *part = &key_def->parts[idx]; > + const char *field = tuple_field_by_part(tuple, part); > + if (field == NULL) { > + assert(key_def->has_optional_parts); > + continue; > + } > + if (key_part_validate(part->type, field, idx, > + key_part_is_nullable(part)) != 0) > + return NULL; > + } > + tuple_ref(tuple); > + return tuple; > +} The code checking a tuple against key_def should live somewhere in src/box - chances are high that we miss lua/key_def.c when we extend key_def struct again. Anyway, this check is incorrect: tarantool> s = box.schema.space.create('test') --- ... tarantool> i = box.space.test:create_index('pk', {parts = {{1, 'unsigned', path = 'a'}, {1, 'unsigned', path = 'b'} }}) --- ... tarantool> k = require('key_def').new(i.parts) --- ... tarantool> k:extract_key({1, 2, 3}) tarantool: /home/vlad/src/tarantool/src/box/lua/key_def.c:246: lbox_key_def_check_tuple: Assertion `key_def->has_optional_parts' failed. Please fix it and don't forget to add a test case. Probably, we should reuse tuple_validate() for checking a tuple against a key_def so as not to implement the same code again. > + > +static int > +lbox_key_def_extract_key(struct lua_State *L) > +{ > + struct key_def *key_def; > + if (lua_gettop(L) != 2 || (key_def = check_key_def(L, 1)) == NULL) > + return luaL_error(L, "Usage: key_def:extract_key(tuple)"); > + > + struct tuple *tuple; > + if ((tuple = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) > + return luaT_error(L); > + > + uint32_t key_size; > + char *key = tuple_extract_key(tuple, key_def, &key_size); > + tuple_unref(tuple); > + if (key == NULL) > + return luaT_error(L); > + > + struct tuple *ret = > + box_tuple_new(box_tuple_format_default(), key, key + key_size); > + if (ret == NULL) > + return luaT_error(L); > + luaT_pushtuple(L, ret); > + return 1; > +} > + > +static int > +lbox_key_def_compare(struct lua_State *L) > +{ > + struct key_def *key_def; > + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL) { > + return luaL_error(L, "Usage: key_def:" > + "compare(tuple_a, tuple_b)"); > + } > + > + struct tuple *tuple_a, *tuple_b; > + if ((tuple_a = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) > + return luaT_error(L); > + if ((tuple_b = lbox_key_def_check_tuple(L, key_def, 3)) == NULL) { > + tuple_unref(tuple_a); > + return luaT_error(L); > + } > + > + int rc = tuple_compare(tuple_a, tuple_b, key_def); > + tuple_unref(tuple_a); > + tuple_unref(tuple_b); > + lua_pushinteger(L, rc); > + return 1; > +} > + > +static int > +lbox_key_def_compare_with_key(struct lua_State *L) > +{ > + struct key_def *key_def; > + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL) { > + return luaL_error(L, "Usage: key_def:" > + "compare_with_key(tuple, key)"); > + } > + > + struct tuple *tuple, *key_tuple = NULL; > + struct tuple_format *format = box_tuple_format_default(); > + if ((tuple = lbox_key_def_check_tuple(L, key_def, 2)) == NULL) > + return luaT_error(L); > + if ((key_tuple = luaT_tuple_new(L, 3, format)) == NULL) { > + tuple_unref(tuple); > + return luaT_error(L); > + } > + tuple_ref(key_tuple); > + > + const char *key = tuple_data(key_tuple); Creating a tuple even though we just need msgpack doesn't look good to me. Can we encode msgpack without creating a tuple? How does index.get handle it? > + assert(mp_typeof(*key) == MP_ARRAY); > + uint32_t part_count = mp_decode_array(&key); > + if (key_validate_parts(key_def, key, part_count, true) != 0) { > + tuple_unref(tuple); > + tuple_unref(key_tuple); > + return luaT_error(L); > + } > + > + int rc = tuple_compare_with_key(tuple, key, part_count, key_def); > + tuple_unref(tuple); > + tuple_unref(key_tuple); > + lua_pushinteger(L, rc); > + return 1; > +} > + > +static int > +lbox_key_def_merge(struct lua_State *L) > +{ > + struct key_def *key_def_a, *key_def_b; > + if (lua_gettop(L) != 2 || (key_def_a = check_key_def(L, 1)) == NULL || > + (key_def_b = check_key_def(L, 2)) == NULL) > + return luaL_error(L, "Usage: key_def:merge(second_key_def)"); > + > + struct key_def *new_key_def = key_def_merge(key_def_a, key_def_b); > + if (new_key_def == NULL) > + return luaT_error(L); > + > + *(struct key_def **) luaL_pushcdata(L, key_def_type_id) = new_key_def; > + lua_pushcfunction(L, lbox_key_def_gc); > + luaL_setcdatagc(L, -2); > + return 1; > +} > + > +static int > +lbox_key_def_to_table(struct lua_State *L) > +{ > + struct key_def *key_def; > + if (lua_gettop(L) != 1 || (key_def = check_key_def(L, 1)) == NULL) > + return luaL_error(L, "Usage: key_def:totable()"); > + > + lua_createtable(L, key_def->part_count, 0); > + for (uint32_t i = 0; i < key_def->part_count; ++i) { > + lbox_push_key_part(L, &key_def->parts[i]); > + lua_rawseti(L, -2, i + 1); > + } > + return 1; > +} > + > +/** > + * Create a new key_def from a Lua table. > + * > + * Expected a table of key parts on the Lua stack. The format is > + * the same as box.space.<...>.index.<...>.parts or corresponding > + * net.box's one. > + * > + * Push the new key_def as cdata to a Lua stack. > + */ > +static int > +lbox_key_def_new(struct lua_State *L) > +{ > + if (lua_gettop(L) != 1 || lua_istable(L, 1) != 1) > + return luaL_error(L, "Bad params, use: key_def.new({" > + "{fieldno = fieldno, type = type" > + "[, is_nullable = <boolean>]" > + "[, path = <string>]" > + "[, collation_id = <number>]" > + "[, collation = <string>]}, ...}"); > + > + uint32_t part_count = lua_objlen(L, 1); > + const ssize_t parts_size = sizeof(struct key_part_def) * part_count; > + > + struct region *region = &fiber()->gc; > + size_t region_svp = region_used(region); > + struct key_part_def *parts = region_alloc(region, parts_size); > + if (parts == NULL) { > + diag_set(OutOfMemory, parts_size, "region", "parts"); > + return luaT_error(L); > + } > + > + for (uint32_t i = 0; i < part_count; ++i) { > + lua_pushinteger(L, i + 1); > + lua_gettable(L, 1); > + if (luaT_key_def_set_part(L, parts, i, region) != 0) { > + region_truncate(region, region_svp); > + return luaT_error(L); > + } > + } > + > + struct key_def *key_def = key_def_new(parts, part_count); > + region_truncate(region, region_svp); > + if (key_def == NULL) > + return luaT_error(L); > + > + /* > + * Calculate minimal field count of tuples with specified > + * key and update key_def optionality to use correct > + * compare/extract functions. > + */ > + uint32_t min_field_count = > + tuple_format_min_field_count(&key_def, 1, NULL, 0); > + key_def_update_optionality(key_def, min_field_count); > + > + *(struct key_def **) luaL_pushcdata(L, key_def_type_id) = key_def; > + lua_pushcfunction(L, lbox_key_def_gc); > + luaL_setcdatagc(L, -2); > + > + return 1; > +} > + > +LUA_API int > +luaopen_key_def(struct lua_State *L) > +{ > + luaL_cdef(L, "struct key_def;"); > + key_def_type_id = luaL_ctypeid(L, "struct key_def&"); > + > + /* Export C functions to Lua. */ > + static const struct luaL_Reg meta[] = { > + {"new", lbox_key_def_new}, > + {NULL, NULL} > + }; > + luaL_register_module(L, "key_def", meta); > + > + lua_newtable(L); /* key_def.internal */ > + lua_pushcfunction(L, lbox_key_def_extract_key); > + lua_setfield(L, -2, "extract_key"); > + lua_pushcfunction(L, lbox_key_def_compare); > + lua_setfield(L, -2, "compare"); > + lua_pushcfunction(L, lbox_key_def_compare_with_key); > + lua_setfield(L, -2, "compare_with_key"); > + lua_pushcfunction(L, lbox_key_def_merge); > + lua_setfield(L, -2, "merge"); > + lua_pushcfunction(L, lbox_key_def_to_table); > + lua_setfield(L, -2, "totable"); > + lua_setfield(L, -2, "internal"); Why 'internal'? We use them as is as key_def methods. E.g. box.tuple.* methods aren't internal. > + > + return 1; > +} > diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc > index ca793e42..6eecbc75 100644 > --- a/src/box/lua/space.cc > +++ b/src/box/lua/space.cc > @@ -30,6 +30,7 @@ > */ > #include "box/lua/space.h" > #include "box/lua/tuple.h" > +#include "box/lua/key_def.h" > #include "box/sql/sqlLimit.h" > #include "lua/utils.h" > #include "lua/trigger.h" > @@ -286,32 +287,7 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i) > > for (uint32_t j = 0; j < index_def->key_def->part_count; j++) { > lua_pushnumber(L, j + 1); > - lua_newtable(L); > - const struct key_part *part = > - &index_def->key_def->parts[j]; > - > - lua_pushstring(L, field_type_strs[part->type]); > - lua_setfield(L, -2, "type"); > - > - lua_pushnumber(L, part->fieldno + TUPLE_INDEX_BASE); > - lua_setfield(L, -2, "fieldno"); > - > - if (part->path != NULL) { > - lua_pushlstring(L, part->path, part->path_len); > - lua_setfield(L, -2, "path"); > - } > - > - lua_pushboolean(L, key_part_is_nullable(part)); > - lua_setfield(L, -2, "is_nullable"); > - > - if (part->coll_id != COLL_NONE) { > - struct coll_id *coll_id = > - coll_by_id(part->coll_id); > - assert(coll_id != NULL); > - lua_pushstring(L, coll_id->name); > - lua_setfield(L, -2, "collation"); > - } > - > + lbox_push_key_part(L, &index_def->key_def->parts[j]); > lua_settable(L, -3); /* index[k].parts[j] */ Why not factor out lbox_push_key_def instead? > } > > diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua > new file mode 100755 > index 00000000..1270f82e > --- /dev/null > +++ b/test/box-tap/key_def.test.lua > @@ -0,0 +1,370 @@ > +#!/usr/bin/env tarantool > + > +local tap = require('tap') > +local ffi = require('ffi') > +local json = require('json') > +local fun = require('fun') > +local key_def_lib = require('key_def') > + > +local usage_error = 'Bad params, use: key_def.new({' .. > + '{fieldno = fieldno, type = type' .. > + '[, is_nullable = <boolean>]' .. > + '[, path = <string>]' .. > + '[, collation_id = <number>]' .. > + '[, collation = <string>]}, ...}' > + > +local function coll_not_found(fieldno, collation) > + if type(collation) == 'number' then > + return ('Wrong index options (field %d): ' .. > + 'collation was not found by ID'):format(fieldno) > + end > + > + return ('Unknown collation: "%s"'):format(collation) > +end > + > +local function set_key_part_defaults(parts) > + local res = {} > + for i, part in ipairs(parts) do > + res[i] = table.copy(part) > + if res[i].is_nullable == nil then > + res[i].is_nullable = false > + end > + end > + return res > +end > + > +local key_def_new_cases = { > + -- Cases to call before box.cfg{}. > + { > + 'Pass a field on an unknown type', > + parts = {{ > + fieldno = 2, > + type = 'unknown', > + }}, > + exp_err = 'Unknown field type: unknown', > + }, > + { > + 'Try to use collation_id before box.cfg{}', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation_id = 2, > + }}, > + exp_err = coll_not_found(1, 2), > + }, > + { > + 'Try to use collation before box.cfg{}', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation = 'unicode_ci', > + }}, > + exp_err = coll_not_found(1, 'unicode_ci'), > + }, > + function() > + -- For collations. > + box.cfg{} > + end, > + -- Cases to call after box.cfg{}. > + { > + 'Try to use both collation_id and collation', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation_id = 2, > + collation = 'unicode_ci', > + }}, > + exp_err = 'Conflicting options: collation_id and collation', > + }, > + { > + 'Unknown collation_id', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation_id = 999, > + }}, > + exp_err = coll_not_found(1, 999), > + }, > + { > + 'Unknown collation name', > + parts = {{ > + fieldno = 1, > + type = 'string', > + collation = 'unknown', > + }}, > + exp_err = 'Unknown collation: "unknown"', > + }, > + { > + 'Bad parts parameter type', > + parts = 1, > + exp_err = usage_error, > + }, > + { > + 'No parameters', > + params = {}, > + exp_err = usage_error, > + }, > + { > + 'Two parameters', > + params = {{}, {}}, > + exp_err = usage_error, > + }, > + { > + 'Invalid JSON path', > + parts = {{ > + fieldno = 1, > + type = 'string', > + path = '[3[', > + }}, > + exp_err = 'Wrong index options (field 1): invalid path', > + }, > + { > + 'Success case; zero parts', > + parts = {}, > + exp_err = nil, > + }, > + { > + 'Success case; one part', > + parts = {{ > + fieldno = 1, > + type = 'string', > + }}, > + exp_err = nil, > + }, > + { > + 'Success case; one part with a JSON path', > + parts = {{ > + fieldno = 1, > + type = 'string', > + path = '[3]', > + }}, > + exp_err = nil, > + }, > +} > + > +local test = tap.test('key_def') > + > +test:plan(#key_def_new_cases - 1 + 7) > +for _, case in ipairs(key_def_new_cases) do > + if type(case) == 'function' then > + case() > + else > + local ok, res > + if case.params then > + ok, res = pcall(key_def_lib.new, unpack(case.params)) > + else > + ok, res = pcall(key_def_lib.new, case.parts) > + end > + if case.exp_err == nil then > + ok = ok and type(res) == 'cdata' and > + ffi.istype('struct key_def', res) > + test:ok(ok, case[1]) > + else > + local err = tostring(res) -- cdata -> string > + test:is_deeply({ok, err}, {false, case.exp_err}, case[1]) > + end > + end > +end > + > +-- Prepare source data for test cases. > +local parts_a = { > + {type = 'unsigned', fieldno = 1}, > +} > +local parts_b = { > + {type = 'number', fieldno = 2}, > + {type = 'number', fieldno = 3}, > +} > +local parts_c = { > + {type = 'scalar', fieldno = 2}, > + {type = 'scalar', fieldno = 1}, > + {type = 'string', fieldno = 4, is_nullable = true}, > +} > +local key_def_a = key_def_lib.new(parts_a) > +local key_def_b = key_def_lib.new(parts_b) > +local key_def_c = key_def_lib.new(parts_c) > +local tuple_a = box.tuple.new({1, 1, 22}) > +local tuple_b = box.tuple.new({2, 1, 11}) > +local tuple_c = box.tuple.new({3, 1, 22}) > + > +-- Case: extract_key(). > +test:test('extract_key()', function(test) > + test:plan(9) > + > + test:is_deeply(key_def_a:extract_key(tuple_a):totable(), {1}, 'case 1') > + test:is_deeply(key_def_b:extract_key(tuple_a):totable(), {1, 22}, 'case 2') > + > + -- JSON path. > + local res = key_def_lib.new({ > + {type = 'string', fieldno = 1, path = 'a.b'}, > + }):extract_key(box.tuple.new({{a = {b = 'foo'}}})):totable() > + test:is_deeply(res, {'foo'}, 'JSON path (tuple argument)') > + > + local res = key_def_lib.new({ > + {type = 'string', fieldno = 1, path = 'a.b'}, > + }):extract_key({{a = {b = 'foo'}}}):totable() > + test:is_deeply(res, {'foo'}, 'JSON path (table argument)') I like key_def_new_cases - they are very easy to read or extend. I don't quite like the tests below, because they refer to objects created a few screens above (tuple_a, key_def_a, etc). Could you please rewrite them in a similar to key_def_new_cases fashion, without referring to any predefined variables? Also, it looks like you don't test json tuple comparison properly. Please add more test cases. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-03 11:10 ` Vladimir Davydov @ 2019-04-03 11:46 ` Alexander Turenko 2019-04-03 12:01 ` Vladimir Davydov 2019-04-04 5:07 ` Alexander Turenko 1 sibling, 1 reply; 25+ messages in thread From: Alexander Turenko @ 2019-04-03 11:46 UTC (permalink / raw) To: Vladimir Davydov; +Cc: Kirill Shcherbatov, tarantool-patches > > +-- Case: extract_key(). > > +test:test('extract_key()', function(test) > > + test:plan(9) > > + > > + test:is_deeply(key_def_a:extract_key(tuple_a):totable(), {1}, 'case 1') > > + test:is_deeply(key_def_b:extract_key(tuple_a):totable(), {1, 22}, 'case 2') > > + > > + -- JSON path. > > + local res = key_def_lib.new({ > > + {type = 'string', fieldno = 1, path = 'a.b'}, > > + }):extract_key(box.tuple.new({{a = {b = 'foo'}}})):totable() > > + test:is_deeply(res, {'foo'}, 'JSON path (tuple argument)') > > + > > + local res = key_def_lib.new({ > > + {type = 'string', fieldno = 1, path = 'a.b'}, > > + }):extract_key({{a = {b = 'foo'}}}):totable() > > + test:is_deeply(res, {'foo'}, 'JSON path (table argument)') > > I like key_def_new_cases - they are very easy to read or extend. > I don't quite like the tests below, because they refer to objects > created a few screens above (tuple_a, key_def_a, etc). Could you > please rewrite them in a similar to key_def_new_cases fashion, > without referring to any predefined variables? It is easy to separate test cases from a testing code in case of one function like key_def.new(), but it is not so easy when we need to test several functions with different behaviour. So I vote up for inlining related data (tuple_a and so on) to test cases, but doubt these cases could be written in such declarative manner as I did for key_def.new(). Thank you for the detailed review (eps. for the case that passes over validation)! WBR, Alexander Turenko. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-03 11:46 ` Alexander Turenko @ 2019-04-03 12:01 ` Vladimir Davydov 2019-04-03 13:26 ` Alexander Turenko 0 siblings, 1 reply; 25+ messages in thread From: Vladimir Davydov @ 2019-04-03 12:01 UTC (permalink / raw) To: Alexander Turenko; +Cc: Kirill Shcherbatov, tarantool-patches On Wed, Apr 03, 2019 at 02:46:44PM +0300, Alexander Turenko wrote: > > > +-- Case: extract_key(). > > > +test:test('extract_key()', function(test) > > > + test:plan(9) > > > + > > > + test:is_deeply(key_def_a:extract_key(tuple_a):totable(), {1}, 'case 1') > > > + test:is_deeply(key_def_b:extract_key(tuple_a):totable(), {1, 22}, 'case 2') > > > + > > > + -- JSON path. > > > + local res = key_def_lib.new({ > > > + {type = 'string', fieldno = 1, path = 'a.b'}, > > > + }):extract_key(box.tuple.new({{a = {b = 'foo'}}})):totable() > > > + test:is_deeply(res, {'foo'}, 'JSON path (tuple argument)') > > > + > > > + local res = key_def_lib.new({ > > > + {type = 'string', fieldno = 1, path = 'a.b'}, > > > + }):extract_key({{a = {b = 'foo'}}}):totable() > > > + test:is_deeply(res, {'foo'}, 'JSON path (table argument)') > > > > I like key_def_new_cases - they are very easy to read or extend. > > I don't quite like the tests below, because they refer to objects > > created a few screens above (tuple_a, key_def_a, etc). Could you > > please rewrite them in a similar to key_def_new_cases fashion, > > without referring to any predefined variables? > > It is easy to separate test cases from a testing code in case of one > function like key_def.new(), but it is not so easy when we need to test > several functions with different behaviour. So I vote up for inlining > related data (tuple_a and so on) to test cases, but doubt these cases > could be written in such declarative manner as I did for key_def.new(). I didn't mean to mix all function test cases in one table. I meant using a separate table for each function. Something like this: tuple_compare_test_cases = { { 'Tuple compare with collation', parts = {{ fieldno = 1, type = 'string', collation = 'unicode_ci', }}, tuple1 = {'test1', 1, 2}, tuple2 = {'test2', 3}, exp_err = nil, exp_ret = 1, } ... } tuple_extract_key_cases = { ... } Do you think it would be an overkill? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-03 12:01 ` Vladimir Davydov @ 2019-04-03 13:26 ` Alexander Turenko 0 siblings, 0 replies; 25+ messages in thread From: Alexander Turenko @ 2019-04-03 13:26 UTC (permalink / raw) To: Vladimir Davydov; +Cc: Kirill Shcherbatov, tarantool-patches On Wed, Apr 03, 2019 at 03:01:21PM +0300, Vladimir Davydov wrote: > On Wed, Apr 03, 2019 at 02:46:44PM +0300, Alexander Turenko wrote: > > > > +-- Case: extract_key(). > > > > +test:test('extract_key()', function(test) > > > > + test:plan(9) > > > > + > > > > + test:is_deeply(key_def_a:extract_key(tuple_a):totable(), {1}, 'case 1') > > > > + test:is_deeply(key_def_b:extract_key(tuple_a):totable(), {1, 22}, 'case 2') > > > > + > > > > + -- JSON path. > > > > + local res = key_def_lib.new({ > > > > + {type = 'string', fieldno = 1, path = 'a.b'}, > > > > + }):extract_key(box.tuple.new({{a = {b = 'foo'}}})):totable() > > > > + test:is_deeply(res, {'foo'}, 'JSON path (tuple argument)') > > > > + > > > > + local res = key_def_lib.new({ > > > > + {type = 'string', fieldno = 1, path = 'a.b'}, > > > > + }):extract_key({{a = {b = 'foo'}}}):totable() > > > > + test:is_deeply(res, {'foo'}, 'JSON path (table argument)') > > > > > > I like key_def_new_cases - they are very easy to read or extend. > > > I don't quite like the tests below, because they refer to objects > > > created a few screens above (tuple_a, key_def_a, etc). Could you > > > please rewrite them in a similar to key_def_new_cases fashion, > > > without referring to any predefined variables? > > > > It is easy to separate test cases from a testing code in case of one > > function like key_def.new(), but it is not so easy when we need to test > > several functions with different behaviour. So I vote up for inlining > > related data (tuple_a and so on) to test cases, but doubt these cases > > could be written in such declarative manner as I did for key_def.new(). > > I didn't mean to mix all function test cases in one table. I meant > using a separate table for each function. Something like this: > > tuple_compare_test_cases = { > { > 'Tuple compare with collation', > parts = {{ > fieldno = 1, > type = 'string', > collation = 'unicode_ci', > }}, > tuple1 = {'test1', 1, 2}, > tuple2 = {'test2', 3}, > exp_err = nil, > exp_ret = 1, > } > ... > } > > tuple_extract_key_cases = { > ... > } > > Do you think it would be an overkill? It seems this way will look more structured, you are right. I agree, but don't insist, because the cases are simple (a function call + is/is_deeply). ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-03 11:10 ` Vladimir Davydov 2019-04-03 11:46 ` Alexander Turenko @ 2019-04-04 5:07 ` Alexander Turenko 2019-04-04 8:04 ` Kirill Shcherbatov 2019-04-04 8:38 ` Vladimir Davydov 1 sibling, 2 replies; 25+ messages in thread From: Alexander Turenko @ 2019-04-04 5:07 UTC (permalink / raw) To: Vladimir Davydov; +Cc: Kirill Shcherbatov, tarantool-patches > > A key_def instance has the following methods: > > > > * :extract_key(tuple) -> key (as tuple) > > * :compare(tuple_a, tuple_b) -> number > > * :compare_with_key(tuple, key) -> number > > What number do these functions return? > > > * :merge(another_key_def) -> new key_def instance > > What does 'merge' do? > > > * :totable() -> table > > Does this function return key_def parts? In what format? > Please elaborate the comments. Note: I think it worth to leave this list of brief descriptions in this format and describe meaning of arguments and return values for each function below. > > Also, key_def.compare() sounds like it compares key definitions, not > tuples. May be, we should move these functions to box.tuple module? I'm tentative about that. The key_def Lua module is planned to be the interface to comparators and here we're using comparators. I don't like spreading of such function across several modules. Maybe 'key_def' name is not good and we need to use something dedicated from the word 'comparator'? > > Also, returning 1, 0, -1 to Lua looks uncommon. May be, we'd better > introduce 'equal', 'greater', 'less', etc helpers returning bool? A function for table.sort(table, func) returns boolean, so it make sense. I'm a bit afraid that we'll need to make two calls: say, :less() and :equal() to determine an order of tuples strictly. But I cannot provide a case where it can be necessary. Are you know one? > > I'm not strongly against the proposed API, but I think we should agree > on it with other members of the team, potential users, and Kostja. I propose to discuss the questions you arose between us and then send RFC email for the API (something very like docbot comment we already have). > > +struct key_def * > > +check_key_def(struct lua_State *L, int idx) > > Please prefix the name with lbox_ or... I dunno - the naming looks > inconsistent: luaT_key_def_set_part, lbox_push_key_part, check_key_def. > Is there some kind of pattern? I understood the convention so: luaL/luaT is somewhat that operates on a Lua stack / state, but cannot be called from Lua directly (because either receive or return C values). So luaT_key_def_set_part() looks right, but lbox_push_key_part(), lbox_key_def_check_tuple() and check_key_def() seems to need be prefixed with luaT. I'll also update check_ibuf(), check_merger_source() and check_merger_context() in the merger patchset (they are statis however). > > +/** > > + * Take existent tuple from LUA stack or build a new tuple with > > + * default format from table, check for compatibility with a > > + * given key_def. Take tuple reference pointer on success. > > + */ > > +static struct tuple * > > +lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) > > +{ > > + struct tuple *tuple = luaT_istuple(L, idx); > > + if (tuple == NULL) > > + tuple = luaT_tuple_new(L, idx, box_tuple_format_default()); > > + if (tuple == NULL) > > + return NULL; > > + /* Check that tuple match with the key definition. */ > > + uint32_t min_field_count = > > + tuple_format_min_field_count(&key_def, 1, NULL, 0); > > + uint32_t field_count = tuple_field_count(tuple); > > + if (field_count < min_field_count) { > > + diag_set(ClientError, ER_NO_SUCH_FIELD_NO, field_count + 1); > > + return NULL; > > + } > > + for (uint32_t idx = 0; idx < key_def->part_count; idx++) { > > + struct key_part *part = &key_def->parts[idx]; > > + const char *field = tuple_field_by_part(tuple, part); > > + if (field == NULL) { > > + assert(key_def->has_optional_parts); > > + continue; > > + } > > + if (key_part_validate(part->type, field, idx, > > + key_part_is_nullable(part)) != 0) > > + return NULL; > > + } > > + tuple_ref(tuple); > > + return tuple; > > +} > > The code checking a tuple against key_def should live somewhere in > src/box - chances are high that we miss lua/key_def.c when we extend > key_def struct again. Can you suggest where it is better to place this code: src/box/key_def.c or src/box/tuple.c? > > +LUA_API int > > +luaopen_key_def(struct lua_State *L) > > +{ > > + luaL_cdef(L, "struct key_def;"); > > + key_def_type_id = luaL_ctypeid(L, "struct key_def&"); > > + > > + /* Export C functions to Lua. */ > > + static const struct luaL_Reg meta[] = { > > + {"new", lbox_key_def_new}, > > + {NULL, NULL} > > + }; > > + luaL_register_module(L, "key_def", meta); > > + > > + lua_newtable(L); /* key_def.internal */ > > + lua_pushcfunction(L, lbox_key_def_extract_key); > > + lua_setfield(L, -2, "extract_key"); > > + lua_pushcfunction(L, lbox_key_def_compare); > > + lua_setfield(L, -2, "compare"); > > + lua_pushcfunction(L, lbox_key_def_compare_with_key); > > + lua_setfield(L, -2, "compare_with_key"); > > + lua_pushcfunction(L, lbox_key_def_merge); > > + lua_setfield(L, -2, "merge"); > > + lua_pushcfunction(L, lbox_key_def_to_table); > > + lua_setfield(L, -2, "totable"); > > + lua_setfield(L, -2, "internal"); > > Why 'internal'? We use them as is as key_def methods. > E.g. box.tuple.* methods aren't internal. To distinguish between module and instance methods and don't confuse a user with, say, tab completion in a console. fio.c does the same. However using, say, <luafun iterator>:map(box.tuple.totable) is convenient, so maybe it worth to name this table, say, 'key_def.instance'? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-04 5:07 ` Alexander Turenko @ 2019-04-04 8:04 ` Kirill Shcherbatov 2019-04-04 9:05 ` Vladimir Davydov 2019-04-04 8:38 ` Vladimir Davydov 1 sibling, 1 reply; 25+ messages in thread From: Kirill Shcherbatov @ 2019-04-04 8:04 UTC (permalink / raw) To: tarantool-patches, Alexander Turenko, Vladimir Davydov >> The code checking a tuple against key_def should live somewhere in >> src/box - chances are high that we miss lua/key_def.c when we extend >> key_def struct again.> > Can you suggest where it is better to place this code: src/box/key_def.c > or src/box/tuple.c? Due to the fact that this code needs table_field_by_part and tuple_format_min_field_count, it cannot be placed in key_def.c Placing it in tuple.h is not quite correct either; that's why we've got it inlined. >> Probably, we should reuse tuple_validate() for checking a tuple against >> a key_def so as not to implement the same code again. Unfortunately tuple_validate() is designed for format validation while we don't have format here and I don't like create it for validation event in case of error. As for assertion you've found, that is my fault; required fix doesn't increase code complexity (not sure about error message): @@ -243,8 +242,11 @@ lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) struct key_part *part = &key_def->parts[idx]; const char *field = tuple_field_by_part(tuple, part); if (field == NULL) { - assert(key_def->has_optional_parts); - continue; + if (key_part_is_nullable(part)) + continue; + diag_set(IllegalParams, "tuple doesn't match key " + "definition (part %d)", idx); + return NULL; } if (key_part_validate(part->type, field, idx, key_part_is_nullable(part)) != 0) >> Mixing ER_WRONG_INDEX_OPTIONS and IllegalParams looks ugly. Please fix. @@ -143,8 +143,7 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *parts, size_t path_len; const char *path = lua_tolstring(L, -1, &path_len); if (json_path_validate(path, path_len, TUPLE_INDEX_BASE) != 0) { - diag_set(ClientError, ER_WRONG_INDEX_OPTIONS, - part_idx + TUPLE_INDEX_BASE, "invalid path"); + diag_set(IllegalParams, "invalid path: \"%s\"", path); return -1; } I quote Alexander's message because it contains answers and suggestions for other questions. On 04.04.2019 8:07, Alexander Turenko wrote: >>> A key_def instance has the following methods: >>> >>> * :extract_key(tuple) -> key (as tuple) >>> * :compare(tuple_a, tuple_b) -> number >>> * :compare_with_key(tuple, key) -> number >> >> What number do these functions return? >> >>> * :merge(another_key_def) -> new key_def instance >> >> What does 'merge' do? >> >>> * :totable() -> table >> >> Does this function return key_def parts? In what format? >> Please elaborate the comments. > > Note: I think it worth to leave this list of brief descriptions in this > format and describe meaning of arguments and return values for each > function below. > >> >> Also, key_def.compare() sounds like it compares key definitions, not >> tuples. May be, we should move these functions to box.tuple module? > > I'm tentative about that. The key_def Lua module is planned to be the > interface to comparators and here we're using comparators. I don't like > spreading of such function across several modules. Maybe 'key_def' name > is not good and we need to use something dedicated from the word > 'comparator'? > >> >> Also, returning 1, 0, -1 to Lua looks uncommon. May be, we'd better >> introduce 'equal', 'greater', 'less', etc helpers returning bool? > > A function for table.sort(table, func) returns boolean, so it make > sense. I'm a bit afraid that we'll need to make two calls: say, :less() > and :equal() to determine an order of tuples strictly. But I cannot > provide a case where it can be necessary. Are you know one? > >> >> I'm not strongly against the proposed API, but I think we should agree >> on it with other members of the team, potential users, and Kostja. > > I propose to discuss the questions you arose between us and then send > RFC email for the API (something very like docbot comment we already > have). > >>> +struct key_def * >>> +check_key_def(struct lua_State *L, int idx) >> >> Please prefix the name with lbox_ or... I dunno - the naming looks >> inconsistent: luaT_key_def_set_part, lbox_push_key_part, check_key_def. >> Is there some kind of pattern? > > I understood the convention so: luaL/luaT is somewhat that operates on a > Lua stack / state, but cannot be called from Lua directly (because > either receive or return C values). So luaT_key_def_set_part() looks > right, but lbox_push_key_part(), lbox_key_def_check_tuple() and > check_key_def() seems to need be prefixed with luaT. > > I'll also update check_ibuf(), check_merger_source() and > check_merger_context() in the merger patchset (they are statis however). > >>> +/** >>> + * Take existent tuple from LUA stack or build a new tuple with >>> + * default format from table, check for compatibility with a >>> + * given key_def. Take tuple reference pointer on success. >>> + */ >>> +static struct tuple * >>> +lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) >>> +{ >>> + struct tuple *tuple = luaT_istuple(L, idx); >>> + if (tuple == NULL) >>> + tuple = luaT_tuple_new(L, idx, box_tuple_format_default()); >>> + if (tuple == NULL) >>> + return NULL; >>> + /* Check that tuple match with the key definition. */ >>> + uint32_t min_field_count = >>> + tuple_format_min_field_count(&key_def, 1, NULL, 0); >>> + uint32_t field_count = tuple_field_count(tuple); >>> + if (field_count < min_field_count) { >>> + diag_set(ClientError, ER_NO_SUCH_FIELD_NO, field_count + 1); >>> + return NULL; >>> + } >>> + for (uint32_t idx = 0; idx < key_def->part_count; idx++) { >>> + struct key_part *part = &key_def->parts[idx]; >>> + const char *field = tuple_field_by_part(tuple, part); >>> + if (field == NULL) { >>> + assert(key_def->has_optional_parts); >>> + continue; >>> + } >>> + if (key_part_validate(part->type, field, idx, >>> + key_part_is_nullable(part)) != 0) >>> + return NULL; >>> + } >>> + tuple_ref(tuple); >>> + return tuple; >>> +} >> >> The code checking a tuple against key_def should live somewhere in >> src/box - chances are high that we miss lua/key_def.c when we extend >> key_def struct again. > > Can you suggest where it is better to place this code: src/box/key_def.c > or src/box/tuple.c? > >>> +LUA_API int >>> +luaopen_key_def(struct lua_State *L) >>> +{ >>> + luaL_cdef(L, "struct key_def;"); >>> + key_def_type_id = luaL_ctypeid(L, "struct key_def&"); >>> + >>> + /* Export C functions to Lua. */ >>> + static const struct luaL_Reg meta[] = { >>> + {"new", lbox_key_def_new}, >>> + {NULL, NULL} >>> + }; >>> + luaL_register_module(L, "key_def", meta); >>> + >>> + lua_newtable(L); /* key_def.internal */ >>> + lua_pushcfunction(L, lbox_key_def_extract_key); >>> + lua_setfield(L, -2, "extract_key"); >>> + lua_pushcfunction(L, lbox_key_def_compare); >>> + lua_setfield(L, -2, "compare"); >>> + lua_pushcfunction(L, lbox_key_def_compare_with_key); >>> + lua_setfield(L, -2, "compare_with_key"); >>> + lua_pushcfunction(L, lbox_key_def_merge); >>> + lua_setfield(L, -2, "merge"); >>> + lua_pushcfunction(L, lbox_key_def_to_table); >>> + lua_setfield(L, -2, "totable"); >>> + lua_setfield(L, -2, "internal"); >> >> Why 'internal'? We use them as is as key_def methods. >> E.g. box.tuple.* methods aren't internal. > > To distinguish between module and instance methods and don't confuse a > user with, say, tab completion in a console. fio.c does the same. > However using, say, <luafun iterator>:map(box.tuple.totable) is > convenient, so maybe it worth to name this table, say, > 'key_def.instance'? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-04 8:04 ` Kirill Shcherbatov @ 2019-04-04 9:05 ` Vladimir Davydov 2019-04-04 11:46 ` Alexander Turenko 0 siblings, 1 reply; 25+ messages in thread From: Vladimir Davydov @ 2019-04-04 9:05 UTC (permalink / raw) To: Kirill Shcherbatov; +Cc: tarantool-patches, Alexander Turenko On Thu, Apr 04, 2019 at 11:04:10AM +0300, Kirill Shcherbatov wrote: > >> The code checking a tuple against key_def should live somewhere in > >> src/box - chances are high that we miss lua/key_def.c when we extend > >> key_def struct again.> > > Can you suggest where it is better to place this code: src/box/key_def.c > > or src/box/tuple.c? > Due to the fact that this code needs table_field_by_part and > tuple_format_min_field_count, it cannot be placed in key_def.c > Placing it in tuple.h is not quite correct either; that's why we've got it inlined. Well, you could put it in tuple_format.c. > > >> Probably, we should reuse tuple_validate() for checking a tuple against > >> a key_def so as not to implement the same code again. > > Unfortunately tuple_validate() is designed for format validation while we don't > have format here and I don't like create it for validation event in case of error. Creating a format on each call to 'compare' is prohibitive from performance pov, you're right. However, we could attach a format to Lua's key_def object. This would also speed up comparisons, as tuples created for this format would have fieldmap. > > > As for assertion you've found, that is my fault; required fix doesn't increase > code complexity (not sure about error message): > > @@ -243,8 +242,11 @@ lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) > struct key_part *part = &key_def->parts[idx]; > const char *field = tuple_field_by_part(tuple, part); > if (field == NULL) { > - assert(key_def->has_optional_parts); > - continue; > + if (key_part_is_nullable(part)) > + continue; > + diag_set(IllegalParams, "tuple doesn't match key " > + "definition (part %d)", idx); > + return NULL; Again, mixing IllegalParams and ClientError (from key_part_validate) doesn't look good. Also, after this fix you don't need the min_field_count check. However, what I don't like is that in case a tuple is created from a Lua table, you'll have to access all fields twice - on validation and comparison - which is costly without a fieldmap. One more argument for attaching a format to Lua's key_def object. > } > if (key_part_validate(part->type, field, idx, > key_part_is_nullable(part)) != 0) > > > >> Mixing ER_WRONG_INDEX_OPTIONS and IllegalParams looks ugly. Please fix. > > @@ -143,8 +143,7 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *parts, > size_t path_len; > const char *path = lua_tolstring(L, -1, &path_len); > if (json_path_validate(path, path_len, TUPLE_INDEX_BASE) != 0) { > - diag_set(ClientError, ER_WRONG_INDEX_OPTIONS, > - part_idx + TUPLE_INDEX_BASE, "invalid path"); > + diag_set(IllegalParams, "invalid path: \"%s\"", path); Then you don't need to pass parts+part_idx to this function - you can pass only the part you need to set, as it was initially done by Alexander. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-04 9:05 ` Vladimir Davydov @ 2019-04-04 11:46 ` Alexander Turenko 2019-04-04 14:36 ` Vladimir Davydov 0 siblings, 1 reply; 25+ messages in thread From: Alexander Turenko @ 2019-04-04 11:46 UTC (permalink / raw) To: Vladimir Davydov; +Cc: Kirill Shcherbatov, tarantool-patches > > >> Probably, we should reuse tuple_validate() for checking a tuple against > > >> a key_def so as not to implement the same code again. > > > > Unfortunately tuple_validate() is designed for format validation while we don't > > have format here and I don't like create it for validation event in case of error. > > Creating a format on each call to 'compare' is prohibitive from > performance pov, you're right. However, we could attach a format to > Lua's key_def object. This would also speed up comparisons, as tuples So this module will not more be relatively simple layer that exposes box/key_def into Lua and this again suggests me that the name key_def maybe is not good choice. > created for this format would have fieldmap. Note: It will speed up comparisons only in case when a user give a lua table as input, not when it give a tuple. However if a user give a tuple it seems that it is in (s)he responsibility to think about the format. And, while we're here, maybe it worth to provide a function like box.tuple.new(), but with key_def instance as an argument to allow a user to create tuples with necessary formats (to speed up comparisons and other fields accesses)? > However, what I don't like is that in case a tuple is created from a Lua > table, you'll have to access all fields twice - on validation and > comparison - which is costly without a fieldmap. One more argument for > attaching a format to Lua's key_def object. I think we can add *_unchecked() function variants like in msgpack/msgpackffi if the peformance will be really a problem and someone will request for them. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-04 11:46 ` Alexander Turenko @ 2019-04-04 14:36 ` Vladimir Davydov 0 siblings, 0 replies; 25+ messages in thread From: Vladimir Davydov @ 2019-04-04 14:36 UTC (permalink / raw) To: Alexander Turenko; +Cc: Kirill Shcherbatov, tarantool-patches On Thu, Apr 04, 2019 at 02:46:22PM +0300, Alexander Turenko wrote: > > > >> Probably, we should reuse tuple_validate() for checking a tuple against > > > >> a key_def so as not to implement the same code again. > > > > > > Unfortunately tuple_validate() is designed for format validation while we don't > > > have format here and I don't like create it for validation event in case of error. > > > > Creating a format on each call to 'compare' is prohibitive from > > performance pov, you're right. However, we could attach a format to > > Lua's key_def object. This would also speed up comparisons, as tuples > > So this module will not more be relatively simple layer that exposes > box/key_def into Lua and this again suggests me that the name key_def > maybe is not good choice. > > > created for this format would have fieldmap. > > Note: It will speed up comparisons only in case when a user give a lua Yeah, right. So speeding up comparisons isn't an iron-clad argument for attaching a format to Lua key_def object. May be not worth the complexity it will introduce. > table as input, not when it give a tuple. However if a user give a tuple > it seems that it is in (s)he responsibility to think about the format. > > And, while we're here, maybe it worth to provide a function like > box.tuple.new(), but with key_def instance as an argument to allow a > user to create tuples with necessary formats (to speed up comparisons > and other fields accesses)? No, I don't think so: the API wouldn't be generic enough, because one might want to compare the same tuple using different key definitions. > > > However, what I don't like is that in case a tuple is created from a Lua > > table, you'll have to access all fields twice - on validation and > > comparison - which is costly without a fieldmap. One more argument for > > attaching a format to Lua's key_def object. > > I think we can add *_unchecked() function variants like in > msgpack/msgpackffi if the peformance will be really a problem and > someone will request for them. I don't think that _unchecked (i.e. crash on malformed input) is a good idea, especially in this case. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-04 5:07 ` Alexander Turenko 2019-04-04 8:04 ` Kirill Shcherbatov @ 2019-04-04 8:38 ` Vladimir Davydov 2019-04-04 11:17 ` Alexander Turenko 1 sibling, 1 reply; 25+ messages in thread From: Vladimir Davydov @ 2019-04-04 8:38 UTC (permalink / raw) To: Alexander Turenko; +Cc: Kirill Shcherbatov, tarantool-patches On Thu, Apr 04, 2019 at 08:07:34AM +0300, Alexander Turenko wrote: > > Also, key_def.compare() sounds like it compares key definitions, not > > tuples. May be, we should move these functions to box.tuple module? > > I'm tentative about that. The key_def Lua module is planned to be the > interface to comparators and here we're using comparators. I don't like > spreading of such function across several modules. Maybe 'key_def' name > is not good and we need to use something dedicated from the word > 'comparator'? It's not just a comparator. It's also used for extracting keys. Regarding moving this code to box.tuple - I don't insist. Let's just solicit approval from Kostja on this one. > > > > > Also, returning 1, 0, -1 to Lua looks uncommon. May be, we'd better > > introduce 'equal', 'greater', 'less', etc helpers returning bool? > > A function for table.sort(table, func) returns boolean, so it make > sense. I'm a bit afraid that we'll need to make two calls: say, :less() > and :equal() to determine an order of tuples strictly. But I cannot > provide a case where it can be necessary. Are you know one? No, I don't write much code in Lua. However, if we decided to switch to bool return parameter, I'd implement all possible combinations, i.e. eq, ge, le, gt, lt. > > > > > I'm not strongly against the proposed API, but I think we should agree > > on it with other members of the team, potential users, and Kostja. > > I propose to discuss the questions you arose between us and then send > RFC email for the API (something very like docbot comment we already > have). Agree, an RFC letter with this API and possible alternatives would be good. > > > > +struct key_def * > > > +check_key_def(struct lua_State *L, int idx) > > > > Please prefix the name with lbox_ or... I dunno - the naming looks > > inconsistent: luaT_key_def_set_part, lbox_push_key_part, check_key_def. > > Is there some kind of pattern? > > I understood the convention so: luaL/luaT is somewhat that operates on a > Lua stack / state, but cannot be called from Lua directly (because > either receive or return C values). So luaT_key_def_set_part() looks > right, but lbox_push_key_part(), lbox_key_def_check_tuple() and > check_key_def() seems to need be prefixed with luaT. > > I'll also update check_ibuf(), check_merger_source() and > check_merger_context() in the merger patchset (they are statis however). Okay. I just want to see some pattern, at least in the scope of the same source file. > > > > +/** > > > + * Take existent tuple from LUA stack or build a new tuple with > > > + * default format from table, check for compatibility with a > > > + * given key_def. Take tuple reference pointer on success. > > > + */ > > > +static struct tuple * > > > +lbox_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx) > > > +{ > > > + struct tuple *tuple = luaT_istuple(L, idx); > > > + if (tuple == NULL) > > > + tuple = luaT_tuple_new(L, idx, box_tuple_format_default()); > > > + if (tuple == NULL) > > > + return NULL; > > > + /* Check that tuple match with the key definition. */ > > > + uint32_t min_field_count = > > > + tuple_format_min_field_count(&key_def, 1, NULL, 0); > > > + uint32_t field_count = tuple_field_count(tuple); > > > + if (field_count < min_field_count) { > > > + diag_set(ClientError, ER_NO_SUCH_FIELD_NO, field_count + 1); > > > + return NULL; > > > + } > > > + for (uint32_t idx = 0; idx < key_def->part_count; idx++) { > > > + struct key_part *part = &key_def->parts[idx]; > > > + const char *field = tuple_field_by_part(tuple, part); > > > + if (field == NULL) { > > > + assert(key_def->has_optional_parts); > > > + continue; > > > + } > > > + if (key_part_validate(part->type, field, idx, > > > + key_part_is_nullable(part)) != 0) > > > + return NULL; > > > + } > > > + tuple_ref(tuple); > > > + return tuple; > > > +} > > > > The code checking a tuple against key_def should live somewhere in > > src/box - chances are high that we miss lua/key_def.c when we extend > > key_def struct again. > > Can you suggest where it is better to place this code: src/box/key_def.c > or src/box/tuple.c? Actually, I was thinking about reusing tuple_validate() for this to avoid code duplication. However, if we decide to introduce a separate function, I guess it should live either in key_def.c or tuple_format.c. > > > > +LUA_API int > > > +luaopen_key_def(struct lua_State *L) > > > +{ > > > + luaL_cdef(L, "struct key_def;"); > > > + key_def_type_id = luaL_ctypeid(L, "struct key_def&"); > > > + > > > + /* Export C functions to Lua. */ > > > + static const struct luaL_Reg meta[] = { > > > + {"new", lbox_key_def_new}, > > > + {NULL, NULL} > > > + }; > > > + luaL_register_module(L, "key_def", meta); > > > + > > > + lua_newtable(L); /* key_def.internal */ > > > + lua_pushcfunction(L, lbox_key_def_extract_key); > > > + lua_setfield(L, -2, "extract_key"); > > > + lua_pushcfunction(L, lbox_key_def_compare); > > > + lua_setfield(L, -2, "compare"); > > > + lua_pushcfunction(L, lbox_key_def_compare_with_key); > > > + lua_setfield(L, -2, "compare_with_key"); > > > + lua_pushcfunction(L, lbox_key_def_merge); > > > + lua_setfield(L, -2, "merge"); > > > + lua_pushcfunction(L, lbox_key_def_to_table); > > > + lua_setfield(L, -2, "totable"); > > > + lua_setfield(L, -2, "internal"); > > > > Why 'internal'? We use them as is as key_def methods. > > E.g. box.tuple.* methods aren't internal. > > To distinguish between module and instance methods and don't confuse a But 'compare', 'merge', etc can be used both as module methods and as instance method, i.e. neither of the ways of using say compare is wrong: k = key_def.new(...) k:compare(t1, t2) key_def.compare(k, t1, t2) > user with, say, tab completion in a console. fio.c does the same. I like to see all public methods suggested to me when I type key_def.<TAB><TAB>. Don't know about fio, but box.tuple does the same: t:update(...) box.tuple.update(t, ...) are equally correct AFAIU. > However using, say, <luafun iterator>:map(box.tuple.totable) is > convenient, so maybe it worth to name this table, say, > 'key_def.instance'? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-04 8:38 ` Vladimir Davydov @ 2019-04-04 11:17 ` Alexander Turenko 2019-04-04 12:00 ` Alexander Turenko 0 siblings, 1 reply; 25+ messages in thread From: Alexander Turenko @ 2019-04-04 11:17 UTC (permalink / raw) To: Vladimir Davydov; +Cc: Kirill Shcherbatov, tarantool-patches On Thu, Apr 04, 2019 at 11:38:24AM +0300, Vladimir Davydov wrote: > On Thu, Apr 04, 2019 at 08:07:34AM +0300, Alexander Turenko wrote: > > > Also, key_def.compare() sounds like it compares key definitions, not > > > tuples. May be, we should move these functions to box.tuple module? > > > > I'm tentative about that. The key_def Lua module is planned to be the > > interface to comparators and here we're using comparators. I don't like > > spreading of such function across several modules. Maybe 'key_def' name > > is not good and we need to use something dedicated from the word > > 'comparator'? > > It's not just a comparator. It's also used for extracting keys. > Regarding moving this code to box.tuple - I don't insist. Let's > just solicit approval from Kostja on this one. It is a comparator except extracting keys. Or keys-related functions except comparing a tuple against a tuple. Now I don't sure. > > > > > > > > Also, returning 1, 0, -1 to Lua looks uncommon. May be, we'd better > > > introduce 'equal', 'greater', 'less', etc helpers returning bool? > > > > A function for table.sort(table, func) returns boolean, so it make > > sense. I'm a bit afraid that we'll need to make two calls: say, :less() > > and :equal() to determine an order of tuples strictly. But I cannot > > provide a case where it can be necessary. Are you know one? > > No, I don't write much code in Lua. > > However, if we decided to switch to bool return parameter, I'd implement > all possible combinations, i.e. eq, ge, le, gt, lt. I mean are there cases when we need to check, say t1 < t2 and if it is false then check whether t1 == t2 or t1 > t2? In other words, cases when we need to distinguish all three possible situations. Some algorithms? I looked throught [1] and it seems it does not contain any reason why <=> operator was added to C++20 except ones related to comparison operations autogeneration. We can just add :cmp() in addition to all other variants, what do you think? We can also do the trick: return {eq = <boolean>} from :le() / :ge() instead of true, but this way looks weird. [1]: http://open-std.org/JTC1/SC22/WG21/docs/papers/2017/p0515r0.pdf > > > > +LUA_API int > > > > +luaopen_key_def(struct lua_State *L) > > > > +{ > > > > + luaL_cdef(L, "struct key_def;"); > > > > + key_def_type_id = luaL_ctypeid(L, "struct key_def&"); > > > > + > > > > + /* Export C functions to Lua. */ > > > > + static const struct luaL_Reg meta[] = { > > > > + {"new", lbox_key_def_new}, > > > > + {NULL, NULL} > > > > + }; > > > > + luaL_register_module(L, "key_def", meta); > > > > + > > > > + lua_newtable(L); /* key_def.internal */ > > > > + lua_pushcfunction(L, lbox_key_def_extract_key); > > > > + lua_setfield(L, -2, "extract_key"); > > > > + lua_pushcfunction(L, lbox_key_def_compare); > > > > + lua_setfield(L, -2, "compare"); > > > > + lua_pushcfunction(L, lbox_key_def_compare_with_key); > > > > + lua_setfield(L, -2, "compare_with_key"); > > > > + lua_pushcfunction(L, lbox_key_def_merge); > > > > + lua_setfield(L, -2, "merge"); > > > > + lua_pushcfunction(L, lbox_key_def_to_table); > > > > + lua_setfield(L, -2, "totable"); > > > > + lua_setfield(L, -2, "internal"); > > > > > > Why 'internal'? We use them as is as key_def methods. > > > E.g. box.tuple.* methods aren't internal. > > > > To distinguish between module and instance methods and don't confuse a > > But 'compare', 'merge', etc can be used both as module methods and as > instance method, i.e. neither of the ways of using say compare is wrong: > > k = key_def.new(...) > k:compare(t1, t2) > key_def.compare(k, t1, t2) > > > user with, say, tab completion in a console. fio.c does the same. > > I like to see all public methods suggested to me when I type > key_def.<TAB><TAB>. Don't know about fio, but box.tuple does the same: > > t:update(...) > box.tuple.update(t, ...) > > are equally correct AFAIU. Hm. I didn't thought about that in this way. Okay. > > > However using, say, <luafun iterator>:map(box.tuple.totable) is > > convenient, so maybe it worth to name this table, say, > > 'key_def.instance'? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-04 11:17 ` Alexander Turenko @ 2019-04-04 12:00 ` Alexander Turenko 2019-04-04 14:42 ` Vladimir Davydov 0 siblings, 1 reply; 25+ messages in thread From: Alexander Turenko @ 2019-04-04 12:00 UTC (permalink / raw) To: Vladimir Davydov; +Cc: Kirill Shcherbatov, tarantool-patches > > > > Also, returning 1, 0, -1 to Lua looks uncommon. May be, we'd better > > > > introduce 'equal', 'greater', 'less', etc helpers returning bool? > > > > > > A function for table.sort(table, func) returns boolean, so it make > > > sense. I'm a bit afraid that we'll need to make two calls: say, :less() > > > and :equal() to determine an order of tuples strictly. But I cannot > > > provide a case where it can be necessary. Are you know one? > > > > No, I don't write much code in Lua. > > > > However, if we decided to switch to bool return parameter, I'd implement > > all possible combinations, i.e. eq, ge, le, gt, lt. > > I mean are there cases when we need to check, say t1 < t2 and if it is > false then check whether t1 == t2 or t1 > t2? In other words, cases when > we need to distinguish all three possible situations. Some algorithms? > > I looked throught [1] and it seems it does not contain any reason why > <=> operator was added to C++20 except ones related to comparison > operations autogeneration. > > We can just add :cmp() in addition to all other variants, what do you > think? > > We can also do the trick: return {eq = <boolean>} from :le() / :ge() > instead of true, but this way looks weird. > > [1]: http://open-std.org/JTC1/SC22/WG21/docs/papers/2017/p0515r0.pdf One example that looks valid: search in a binary search tree, where non-leaf nodes hold elements. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module 2019-04-04 12:00 ` Alexander Turenko @ 2019-04-04 14:42 ` Vladimir Davydov 0 siblings, 0 replies; 25+ messages in thread From: Vladimir Davydov @ 2019-04-04 14:42 UTC (permalink / raw) To: Alexander Turenko; +Cc: Kirill Shcherbatov, tarantool-patches On Thu, Apr 04, 2019 at 03:00:40PM +0300, Alexander Turenko wrote: > > > > > Also, returning 1, 0, -1 to Lua looks uncommon. May be, we'd better > > > > > introduce 'equal', 'greater', 'less', etc helpers returning bool? > > > > > > > > A function for table.sort(table, func) returns boolean, so it make > > > > sense. I'm a bit afraid that we'll need to make two calls: say, :less() > > > > and :equal() to determine an order of tuples strictly. But I cannot > > > > provide a case where it can be necessary. Are you know one? > > > > > > No, I don't write much code in Lua. > > > > > > However, if we decided to switch to bool return parameter, I'd implement > > > all possible combinations, i.e. eq, ge, le, gt, lt. > > > > I mean are there cases when we need to check, say t1 < t2 and if it is > > false then check whether t1 == t2 or t1 > t2? In other words, cases when > > we need to distinguish all three possible situations. Some algorithms? > > > > I looked throught [1] and it seems it does not contain any reason why > > <=> operator was added to C++20 except ones related to comparison > > operations autogeneration. > > > > We can just add :cmp() in addition to all other variants, what do you > > think? > > > > We can also do the trick: return {eq = <boolean>} from :le() / :ge() > > instead of true, but this way looks weird. > > > > [1]: http://open-std.org/JTC1/SC22/WG21/docs/papers/2017/p0515r0.pdf > > One example that looks valid: search in a binary search tree, where > non-leaf nodes hold elements. Okay, I see, it means that someone might actually need cmp(), not eq/le/etc, in which case adding all those methods returning bool doesn't look like a good idea. I tend to think that the API proposed originally was okay, after all. Let's send an RFC to dev@ and see what others think. ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2019-04-04 14:42 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-27 14:29 [tarantool-patches] [PATCH v2 0/2] lua: add key_def lua module Kirill Shcherbatov 2019-03-27 14:29 ` [tarantool-patches] [PATCH v2 1/2] lua: add luaT_tuple_new() Kirill Shcherbatov 2019-03-28 9:01 ` [tarantool-patches] " Konstantin Osipov 2019-03-28 9:18 ` Alexander Turenko 2019-04-03 18:01 ` [tarantool-patches] " Vladimir Davydov 2019-04-04 2:51 ` Alexander Turenko 2019-04-04 8:14 ` Vladimir Davydov 2019-03-27 14:29 ` [tarantool-patches] [PATCH v2 2/2] lua: add key_def lua module Kirill Shcherbatov 2019-03-28 2:01 ` Alexander Turenko 2019-03-28 7:38 ` [tarantool-patches] " Kirill Shcherbatov 2019-03-28 8:41 ` Kirill Shcherbatov [not found] ` <6d915212-e80f-4a6d-d884-b838bf25f8a7@tarantool.org> 2019-03-28 11:22 ` Alexander Turenko 2019-04-03 11:10 ` Vladimir Davydov 2019-04-03 11:46 ` Alexander Turenko 2019-04-03 12:01 ` Vladimir Davydov 2019-04-03 13:26 ` Alexander Turenko 2019-04-04 5:07 ` Alexander Turenko 2019-04-04 8:04 ` Kirill Shcherbatov 2019-04-04 9:05 ` Vladimir Davydov 2019-04-04 11:46 ` Alexander Turenko 2019-04-04 14:36 ` Vladimir Davydov 2019-04-04 8:38 ` Vladimir Davydov 2019-04-04 11:17 ` Alexander Turenko 2019-04-04 12:00 ` Alexander Turenko 2019-04-04 14:42 ` Vladimir Davydov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox