[tarantool-patches] Re: [PATCH v1 1/1] lua: export key_def methods for LUA key_def object
Kirill Shcherbatov
kshcherbatov at tarantool.org
Tue Mar 26 16:37:54 MSK 2019
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 <lua.h>
#include <lauxlib.h>
+#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 = <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 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 = <boolean>]' ..
+ '[, path = <string>]' ..
'[, collation_id = <number>]' ..
'[, collation = <string>]}, ...}'
@@ -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 <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" /* 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 "<struct key_def &>" 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) == "<struct key_def &>", true, "tostring(sk_def)")
+
os.exit(test:check() and 0 or 1)
--
2.21.0
More information about the Tarantool-patches
mailing list