From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 8B25F299D4 for ; Tue, 26 Mar 2019 09:37:57 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id rGiV9VkoMZ-y for ; Tue, 26 Mar 2019 09:37:57 -0400 (EDT) Received: from smtp14.mail.ru (smtp14.mail.ru [94.100.181.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 177E82474D for ; Tue, 26 Mar 2019 09:37:57 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] lua: export key_def methods for LUA key_def object References: <20190326014624.jzrpzwghnrtfx3mr@tkn_work_nb> From: Kirill Shcherbatov Message-ID: <2a06406f-3ea0-b892-5c8d-a0ee54e07d60@tarantool.org> Date: Tue, 26 Mar 2019 16:37:54 +0300 MIME-Version: 1.0 In-Reply-To: <20190326014624.jzrpzwghnrtfx3mr@tkn_work_nb> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, Alexander Turenko Hi! Thank you for review. https://github.com/tarantool/tarantool/tree/kshch/gh-4025-lua-key-kef-methods https://github.com/tarantool/tarantool/tree/kshch/gh-4025-lua-key-kef-methods-110 Pathes for 2.1.1 Fixup for your commit: ============================================================= --- src/box/lua/key_def.c | 48 +++++++++++++++++++++++++++-------- test/box-tap/key_def.test.lua | 11 ++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c index 36c469526..38a9e80a7 100644 --- a/src/box/lua/key_def.c +++ b/src/box/lua/key_def.c @@ -33,6 +33,7 @@ #include #include +#include "fiber.h" #include "diag.h" #include "box/key_def.h" #include "box/box.h" @@ -48,8 +49,10 @@ static uint32_t key_def_type_id = 0; * 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 *part) +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]; /* Set part->fieldno. */ lua_pushstring(L, "fieldno"); lua_gettable(L, -2); @@ -94,11 +97,11 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *part) /* Set part->is_nullable and part->nullable_action. */ lua_pushstring(L, "is_nullable"); lua_gettable(L, -2); - if (lua_isnil(L, -1)) { + if (lua_isnil(L, -1) || lua_toboolean(L, -1) == 0) { part->is_nullable = false; part->nullable_action = ON_CONFLICT_ACTION_DEFAULT; } else { - part->is_nullable = lua_toboolean(L, -1); + part->is_nullable = true; part->nullable_action = ON_CONFLICT_ACTION_NONE; } lua_pop(L, 1); @@ -143,9 +146,29 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *part) /* Set part->sort_order. */ part->sort_order = SORT_ORDER_ASC; - /* Set part->path. */ - part->path = NULL; - + /* Set part->path for JSON index. */ + 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; + } + memcpy(tmp, path, path_len); + tmp[path_len] = '\0'; + part->path = tmp; + } else { + part->path = NULL; + } + lua_pop(L, 1); return 0; } @@ -191,28 +214,31 @@ lbox_key_def_new(struct lua_State *L) return luaL_error(L, "Bad params, use: key_def.new({" "{fieldno = fieldno, type = type" "[, is_nullable = ]" + "[, path = ]" "[, collation_id = ]" "[, collation = ]}, ...}"); uint32_t part_count = lua_objlen(L, 1); const ssize_t parts_size = sizeof(struct key_part_def) * part_count; - struct key_part_def *parts = malloc(parts_size); + 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, "malloc", "parts"); + 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]) != 0) { - free(parts); + 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); - free(parts); + region_truncate(region, region_svp); if (key_def == NULL) return luaT_error(L); diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua index 7e6e0e330..fd5d60b18 100755 --- a/test/box-tap/key_def.test.lua +++ b/test/box-tap/key_def.test.lua @@ -7,6 +7,7 @@ local key_def = require('key_def') local usage_error = 'Bad params, use: key_def.new({' .. '{fieldno = fieldno, type = type' .. '[, is_nullable = ]' .. + '[, path = ]' .. '[, collation_id = ]' .. '[, collation = ]}, ...}' @@ -95,6 +96,15 @@ local cases = { 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 = {}, @@ -105,6 +115,7 @@ local cases = { parts = { fieldno = 1, type = 'string', + path = "[3]", }, exp_err = nil, }, -- 2.21.0 ============================================================= Main patch: ============================================================= Functions extract_key, compare, compare_with_key, merge defined for key_def introduced for LUA key_def object. Closes #4025 Closes #3398 @TarantoolBot document Title: Built-in methods for key_def object Now it is possible to work with index base object - key definition in LUA. Available methods: key = key_def:extract_key(tuple) key_def:compare(tuple_a, tuple_b) key_def:compare_with_key(tuple, key), where key is tuple or table new_key_def = key_def:merge(second_key_def) Example: -- Removal values for secondary non-unique index key_def = require('key_def') s = box.schema.space.create('test') pk = s:create_index('pk') idx = 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}} pk_def = key_def.new(pk.parts) for _, tuple in pairs(idx:select({1, nil})) do local key = pk_def:extract_key(tuple) pk:delete(key) end --- src/box/CMakeLists.txt | 1 + src/box/lua/init.c | 2 + src/box/lua/key_def.c | 133 ++++++++++++++++++++++++++++++++++ src/box/lua/key_def.h | 7 ++ src/box/lua/key_def.lua | 19 +++++ src/box/lua/space.cc | 28 +------ test/box-tap/key_def.test.lua | 43 ++++++++++- 7 files changed, 206 insertions(+), 27 deletions(-) create mode 100644 src/box/lua/key_def.lua diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt index f25c21045..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) diff --git a/src/box/lua/init.c b/src/box/lua/init.c index 69f346414..68ef58909 100644 --- a/src/box/lua/init.c +++ b/src/box/lua/init.c @@ -63,6 +63,7 @@ extern char session_lua[], tuple_lua[], + key_def_lua[], schema_lua[], load_cfg_lua[], xlog_lua[], @@ -81,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 }; diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c index 38a9e80a7..c2eb3c071 100644 --- a/src/box/lua/key_def.c +++ b/src/box/lua/key_def.c @@ -35,8 +35,10 @@ #include #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" /* TUPLE_INDEX_BASE */ @@ -172,6 +174,28 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *parts, 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) { @@ -198,6 +222,102 @@ lbox_key_def_gc(struct lua_State *L) return 0; } +static int +lbox_key_def_extract_key(lua_State *L) +{ + struct key_def *key_def; + struct tuple *tuple; + if (lua_gettop(L) != 2 || (key_def = check_key_def(L, 1)) == NULL || + (tuple = luaT_istuple(L, 2)) == NULL) + return luaL_error(L, "Usage: key_def:extract_key(tuple)"); + + uint32_t key_size; + char *key = tuple_extract_key(tuple, key_def, &key_size); + 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(lua_State *L) +{ + struct key_def *key_def; + struct tuple *tuple_a, *tuple_b; + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL || + (tuple_a = luaT_istuple(L, 2)) == NULL || + (tuple_b = luaT_istuple(L, 3)) == NULL) + return luaL_error(L, "Usage: key_def:compare(tuple_a, tuple_b)"); + + int rc = tuple_compare(tuple_a, tuple_b, key_def); + lua_pushinteger(L, rc); + return 1; +} + +static int +lbox_key_def_compare_with_key(lua_State *L) +{ + struct key_def *key_def; + struct tuple *tuple, *key_tuple; + if (lua_gettop(L) != 3 || (key_def = check_key_def(L, 1)) == NULL || + (tuple = luaT_istuple(L, 2)) == NULL) + goto usage_error; + + lua_remove(L, 1); + lua_remove(L, 1); + if (luaT_tuple_new(L, box_tuple_format_default()) != 1 || + (key_tuple = luaT_istuple(L, -1)) == NULL) + goto usage_error; + + const char *key = tuple_data(key_tuple); + assert(mp_typeof(*key) == MP_ARRAY); + uint32_t part_count = mp_decode_array(&key); + + int rc = tuple_compare_with_key(tuple, key, part_count, key_def); + lua_pushinteger(L, rc); + return 1; +usage_error: + return luaL_error(L, "Usage: key_def:compare_with_key(tuple, key)"); +} + +static int +lbox_key_def_merge(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:to_table()"); + + 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. * @@ -262,5 +382,18 @@ luaopen_key_def(struct lua_State *L) }; 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, "to_table"); + lua_setfield(L, -2, "internal"); + return 1; } diff --git a/src/box/lua/key_def.h b/src/box/lua/key_def.h index 61894d452..7dc7158e8 100644 --- a/src/box/lua/key_def.h +++ b/src/box/lua/key_def.h @@ -36,6 +36,13 @@ 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. diff --git a/src/box/lua/key_def.lua b/src/box/lua/key_def.lua new file mode 100644 index 000000000..1bf76e528 --- /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, + ['to_table'] = key_def.internal.to_table, + ['__serialize'] = key_def.internal.to_table, +} + +ffi.metatype(key_def_t, { + __index = function(self, key) + return methods[key] + end, + __tostring = function(key_def) return "" end, +}) diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc index 9dfc97b6a..dbcdc9300 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 index fd5d60b18..c22ef62a8 100755 --- a/test/box-tap/key_def.test.lua +++ b/test/box-tap/key_def.test.lua @@ -123,7 +123,7 @@ local cases = { local test = tap.test('key_def') -test:plan(#cases - 1) +test:plan(#cases - 1 + 16) for _, case in ipairs(cases) do if type(case) == 'function' then case() @@ -145,4 +145,45 @@ for _, case in ipairs(cases) do end end +-- +-- gh-4025: Introduce built-in function to get index key +-- from tuple. +-- gh-3398: Access to index compare function from lua. +-- +tuple_a = box.tuple.new({1, 1, 22}) +tuple_b = box.tuple.new({2, 1, 11}) +tuple_c = box.tuple.new({3, 1, 22}) +pk_parts = {{type = 'unsigned', fieldno = 1}} +sk_parts = {{type = 'number', fieldno=2}, {type='number', fieldno=3}} +pk_def = key_def.new(pk_parts) +test:is_deeply(pk_def:extract_key(tuple_a):totable(), {1}, "pk extract") +sk_def = key_def.new(sk_parts) +test:is_deeply(sk_def:extract_key(tuple_a):totable(), {1, 22}, "sk extract") + +test:is(pk_def:compare(tuple_b, tuple_a), 1, "pk compare 1") +test:is(pk_def:compare(tuple_b, tuple_c), -1, "pk compare 2") +test:is(sk_def:compare(tuple_b, tuple_a), -1, "sk compare 1") +test:is(sk_def:compare(tuple_a, tuple_c), 0, "sk compare 1") + +pk_sk_def = pk_def:merge(sk_def) +test:ok(pk_sk_def, "pksk merge") +test:is_deeply(pk_sk_def:extract_key(tuple_a):totable(), {1, 1, 22}, + "pksk compare 1") +test:is(pk_sk_def:compare(tuple_a, tuple_b), -1, "pksk compare 2") +sk_pk_def = sk_def:merge(pk_def) +test:ok(pk_sk_def, "skpk merge") +test:is_deeply(sk_pk_def:extract_key(tuple_a):totable(), {1, 22, 1}, + "skpk compare 1") +test:is(sk_pk_def:compare(tuple_a, tuple_b), 1, "skpk compare 1") + +key = sk_def:extract_key(tuple_a) +test:is(sk_def:compare_with_key(tuple_a, key), 0, + "sk compare_with_key - extracted tuple key") +test:is(sk_def:compare_with_key(tuple_a, {1, 22}), 0, + "sk compare_with_key - table key") +test:is_deeply(sk_def:to_table(), + {{type='number', fieldno=2, is_nullable=false}, + {type='number', fieldno=3, is_nullable=false}}, "sk to_table") +test:is(tostring(sk_def) == "", true, "tostring(sk_def)") + os.exit(test:check() and 0 or 1) -- 2.21.0