From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id D8AE044643A for ; Wed, 23 Sep 2020 04:40:32 +0300 (MSK) From: Alexander Turenko Date: Wed, 23 Sep 2020 04:40:15 +0300 Message-Id: <078239e37414bb009729bddf1d0822c9c978966b.1600824556.git.alexander.turenko@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 1.10 02/16] refactoring: adjust contract of luaT_tuple_new() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org, Alexander Turenko Accept an index on the Lua stack. Report an error using the diagnostics area (not raise a Lua error). The function is used by built-in key_def and merger modules on tarantool-2.*. Now we plan to expose necessary APIs from tarantool of all supported versions (including 1.10) and create external key_def and merger modules. Part of #5273 (cherry picked from commit 64a6464d31c92ea11c4f5c3ccdaada2ae9666455) The original commit message: | lua: add luaT_tuple_new() | | 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 The original message is a bit misleading, because it was not rewritten after introduction of the function with the same name in 9e2a905c069e58cdd95d91868025391ecd9404e6 ('box: fix format of tuple produced with frommap()'). --- src/box/lua/space.cc | 2 +- src/box/lua/tuple.c | 47 ++++++--- src/box/lua/tuple.h | 13 ++- test/unit/CMakeLists.txt | 4 + test/unit/luaT_tuple_new.c | 175 ++++++++++++++++++++++++++++++++ test/unit/luaT_tuple_new.result | 22 ++++ 6 files changed, 245 insertions(+), 18 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 94e895148..9ea0d6f7e 100644 --- a/src/box/lua/space.cc +++ b/src/box/lua/space.cc @@ -507,7 +507,7 @@ lbox_space_frommap(struct lua_State *L) lua_replace(L, 1); lua_settop(L, 1); - tuple = luaT_tuple_new(L, space->format); + tuple = luaT_tuple_new(L, -1, space->format); if (tuple == NULL) { struct error *e = diag_last_error(diag_get()); lua_pushnil(L); diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c index cff0beca5..8a8fd4d34 100644 --- a/src/box/lua/tuple.c +++ b/src/box/lua/tuple.c @@ -98,37 +98,38 @@ luaT_istuple(struct lua_State *L, int narg) } struct tuple * -luaT_tuple_new(struct lua_State *L, struct tuple_format *format) +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); luamp_encode_array(luaL_msgpack_default, &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 NULL; - /* box_tuple_new() doesn't leak on exception, see public API doc */ ibuf_reinit(tarantool_lua_ibuf); return tuple; } @@ -136,9 +137,23 @@ luaT_tuple_new(struct lua_State *L, struct tuple_format *format) static int lbox_tuple_new(lua_State *L) { - struct tuple *tuple = 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; } diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h index 48b630969..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 */ +/** + * 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, struct tuple_format *format); +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 d0138e8fd..255d27083 100644 --- a/test/unit/CMakeLists.txt +++ b/test/unit/CMakeLists.txt @@ -148,6 +148,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..522216ce1 --- /dev/null +++ b/test/unit/luaT_tuple_new.c @@ -0,0 +1,175 @@ +#include /* strncmp() */ +#include /* lua_*() */ +#include /* luaL_*() */ +#include /* 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; + 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(); + luaopen_msgpack(L); + box_lua_tuple_init(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.25.0