* [PATCH v3 1/1] lua: add key_def lua module
@ 2019-04-22 12:28 Kirill Shcherbatov
2019-04-24 18:13 ` [tarantool-patches] " Konstantin Osipov
2019-04-30 10:30 ` Vladimir Davydov
0 siblings, 2 replies; 7+ messages in thread
From: Kirill Shcherbatov @ 2019-04-22 12:28 UTC (permalink / raw)
To: tarantool-patches, vdavydov.dev; +Cc: alexander.turenko, Kirill Shcherbatov
Needed for #3276.
Fixes #3398.
Fixes #4025.
@TarantoolBot document
Title: lua: key_def module
It is convenient to have tuple compare function into lua-land for the
following case:
- exporting key from tuple to iterate over secondary non-unique index
and delete tuples from space
- support comparing a tuple with a key / a tuple, support merging
key_defs from Lua
- factor out key parts parsing code from the tuples merger
A key_def instance has the following methods:
- :extract_key(tuple) -> key (as tuple)
Receives tuple or Lua table. Returns tuple representing extracted
key.
- :compare(tuple_a, tuple_b) -> number
Receives tuples or Lua tables.
Returns:
- a value > 0 when tuple_a > tuple_b,
- a value == 0 when tuple_a == tuple_b,
- a value < 0 otherwise.
- :compare_with_key(tuple, key) -> number
- a value > 0 when key(tuple) > key,
- a value == 0 when key(tuple) == key,
- a value < 0 otherwise.
- :merge(another_key_def) -> new key_def instance
Constructs a new key definition with a set union of key parts
from first and second key defs
- :totable() -> table
Dump key_def object as a Lua table (needed to support __serialize
method)
The root key_def library exports all instance methods directly.
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.
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
```
---
http://github.com/tarantool/tarantool/tree/kshch/gh-4025-lua-key-kef-methods
https://github.com/tarantool/tarantool/issues/4025
src/CMakeLists.txt | 1 +
src/box/CMakeLists.txt | 2 +
src/box/errcode.h | 1 +
src/box/lua/init.c | 5 +
src/box/lua/key_def.c | 425 ++++++++++++++++++++++++++++++++
src/box/lua/key_def.h | 63 +++++
src/box/lua/key_def.lua | 19 ++
src/box/lua/space.cc | 34 +--
src/box/tuple.h | 28 +++
test/box-tap/key_def.test.lua | 448 ++++++++++++++++++++++++++++++++++
test/box/misc.result | 1 +
11 files changed, 995 insertions(+), 32 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 31600745a..7fbbc7803 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)
@@ -140,6 +141,7 @@ add_library(box STATIC
lua/net_box.c
lua/xlog.c
lua/execute.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/errcode.h b/src/box/errcode.h
index 3f8cb8e0e..72485d3c8 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -246,6 +246,7 @@ struct errcode_record {
/*191 */_(ER_SQL_PARSER_LIMIT, "%s %d exceeds the limit (%d)") \
/*192 */_(ER_INDEX_DEF_UNSUPPORTED, "%s are prohibited in an index definition") \
/*193 */_(ER_CK_DEF_UNSUPPORTED, "%s are prohibited in a CHECK constraint definition") \
+ /*194 */_(ER_TUPLE_KEY_PART_MISSED, "Supplied tuple field for part %u does not exists") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 3ff4f3227..6b8be5096 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/execute.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..d5fe237c0
--- /dev/null
+++ b/src/box/lua/key_def.c
@@ -0,0 +1,425 @@
+/*
+ * 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 "misc.h"
+#include "tuple.h"
+
+static uint32_t key_def_type_id = 0;
+
+void
+luaT_push_key_def(struct lua_State *L, const struct key_def *key_def)
+{
+ lua_createtable(L, key_def->part_count, 0);
+ for (uint32_t i = 0; i < key_def->part_count; ++i) {
+ const struct key_part *part = &key_def->parts[i];
+ 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");
+ }
+ lua_rawseti(L, -2, i + 1);
+ }
+}
+
+/**
+ * Set key_part_def from a table on top of a Lua stack.
+ * The region argument is used to allocate a JSON path when
+ * required.
+ * 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,
+ struct region *region)
+{
+ *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(IllegalParams, "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;
+}
+
+static struct tuple *
+luaT_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 || tuple_validate_parts(key_def, tuple) != 0)
+ return NULL;
+ tuple_ref(tuple);
+ return tuple;
+}
+
+struct key_def *
+luaT_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 = luaT_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 = luaT_check_key_def(L, 1)) == NULL)
+ return luaL_error(L, "Usage: key_def:extract_key(tuple)");
+
+ struct tuple *tuple;
+ if ((tuple = luaT_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 = luaT_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 = luaT_key_def_check_tuple(L, key_def, 2)) == NULL)
+ return luaT_error(L);
+ if ((tuple_b = luaT_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 = luaT_check_key_def(L, 1)) == NULL) {
+ return luaL_error(L, "Usage: key_def:"
+ "compare_with_key(tuple, key)");
+ }
+
+ struct tuple *tuple = luaT_key_def_check_tuple(L, key_def, 2);
+ if (tuple == NULL)
+ return luaT_error(L);
+
+ size_t key_len;
+ const char *key = lbox_encode_tuple_on_gc(L, 3, &key_len);
+ uint32_t part_count = mp_decode_array(&key);
+ if (key_validate_parts(key_def, key, part_count, true) != 0) {
+ tuple_unref(tuple);
+ return luaT_error(L);
+ }
+
+ int rc = tuple_compare_with_key(tuple, key, part_count, key_def);
+ tuple_unref(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 = luaT_check_key_def(L, 1)) == NULL ||
+ (key_def_b = luaT_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 = luaT_check_key_def(L, 1)) == NULL)
+ return luaL_error(L, "Usage: key_def:totable()");
+
+ luaT_push_key_def(L, key_def);
+ 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);
+
+ /*
+ * Compare and extract key_def methods must work even with
+ * tuples with omitted (optional) fields. As there is no
+ * space format which would guarantee certain minimal
+ * field_count, pass min_field_count = 0 to ensure that
+ * functions will work correctly in such case.
+ */
+ key_def_update_optionality(key_def, 0);
+
+ *(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},
+ {"extract_key", lbox_key_def_extract_key},
+ {"compare", lbox_key_def_compare},
+ {"compare_with_key", lbox_key_def_compare_with_key},
+ {"merge", lbox_key_def_merge},
+ {"totable", lbox_key_def_to_table},
+ {NULL, NULL}
+ };
+ luaL_register_module(L, "key_def", meta);
+ return 1;
+}
diff --git a/src/box/lua/key_def.h b/src/box/lua/key_def.h
new file mode 100644
index 000000000..2cecfaae5
--- /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_def;
+
+/**
+ * Push a new table representing a key_def to a Lua stack.
+ */
+void
+luaT_push_key_def(struct lua_State *L, const struct key_def *key_def);
+
+/**
+ * Extract a key_def object from a Lua stack.
+ */
+struct key_def *
+luaT_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..586005709
--- /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.extract_key,
+ ['compare'] = key_def.compare,
+ ['compare_with_key'] = key_def.compare_with_key,
+ ['merge'] = key_def.merge,
+ ['totable'] = key_def.totable,
+ ['__serialize'] = key_def.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 93269b72b..100da0a79 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,38 +287,7 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i)
lua_setfield(L, -2, "name");
lua_pushstring(L, "parts");
- lua_newtable(L);
-
- 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");
- }
-
- lua_settable(L, -3); /* index[k].parts[j] */
- }
+ luaT_push_key_def(L, index_def->key_def);
lua_settable(L, -3); /* space.index[k].parts */
diff --git a/src/box/tuple.h b/src/box/tuple.h
index eed8e1a34..a3ac45574 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -670,6 +670,34 @@ tuple_field_by_part(struct tuple *tuple, struct key_part *part)
tuple_field_map(tuple), part);
}
+/**
+ * Check that tuple fields match with given key definition
+ * key_def.
+ * @param key_def Key definition.
+ * @param tuple Tuple to validate.
+ *
+ * @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)
+{
+ 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) {
+ if (key_part_is_nullable(part))
+ continue;
+ diag_set(ClientError, ER_TUPLE_KEY_PART_MISSED, idx);
+ return -1;
+ }
+ 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..5fc9cc67b
--- /dev/null
+++ b/test/box-tap/key_def.test.lua
@@ -0,0 +1,448 @@
+#!/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 = '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.
+
+-- Case: extract_key().
+test:test('extract_key()', function(test)
+ test:plan(13)
+
+ local key_def_a = key_def_lib.new({
+ {type = 'unsigned', fieldno = 1},
+ })
+ local key_def_b = key_def_lib.new({
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ })
+ local key_def_c = key_def_lib.new({
+ {type = 'scalar', fieldno = 2},
+ {type = 'scalar', fieldno = 1},
+ {type = 'string', fieldno = 4, is_nullable = true},
+ })
+ local tuple_a = box.tuple.new({1, 1, 22})
+
+ 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 = 'Supplied tuple field for part 1 does not exists'
+ 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 = 'Supplied tuple field for part 1 does not exists'
+ 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 = 'Supplied tuple field for part 1 does not exists'
+ 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')
+
+ local key_def = key_def_lib.new({
+ {type = 'number', fieldno = 1, path='a'},
+ {type = 'number', fieldno = 1, path='b'},
+ {type = 'number', fieldno = 1, path='c', is_nullable=true},
+ {type = 'number', fieldno = 3, is_nullable=true},
+ })
+ local ok, err = pcall(key_def.extract_key, key_def,
+ box.tuple.new({1, 1, 22}))
+ test:is_deeply({ok, tostring(err)},
+ {false, 'Supplied tuple field for part 0 does not exists'},
+ 'invalid JSON structure')
+ test:is_deeply(key_def:extract_key({{a=1, b=2}, 1}):totable(),
+ {1, 2, box.NULL, box.NULL},
+ 'tuple with optional parts - case 1')
+ test:is_deeply(key_def:extract_key({{a=1, b=2, c=3}, 1}):totable(),
+ {1, 2, 3, box.NULL},
+ 'tuple with optional parts - case 2')
+ test:is_deeply(key_def:extract_key({{a=1, b=2}, 1, 3}):totable(),
+ {1, 2, box.NULL, 3},
+ 'tuple with optional parts - case 3')
+end)
+
+-- Case: compare().
+test:test('compare()', function(test)
+ test:plan(8)
+
+ local key_def_a = key_def_lib.new({
+ {type = 'unsigned', fieldno = 1},
+ })
+ local key_def_b = key_def_lib.new({
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ })
+ 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})
+
+ 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_def_b = key_def_lib.new({
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ })
+ local tuple_a = box.tuple.new({1, 1, 22})
+
+ 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 parts_a = {
+ {type = 'unsigned', fieldno = 1}
+ }
+ local parts_b = {
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ }
+ local key_def_a = key_def_lib.new(parts_a)
+ local key_def_b = key_def_lib.new(parts_b)
+
+ 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 parts_a = {
+ {type = 'unsigned', fieldno = 1}
+ }
+ local parts_b = {
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ }
+ local key_def_a = key_def_lib.new(parts_a)
+ local key_def_b = key_def_lib.new(parts_b)
+
+ 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 parts_a = {
+ {type = 'unsigned', fieldno = 1}
+ }
+ local parts_b = {
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ }
+ local key_def_a = key_def_lib.new(parts_a)
+ local key_def_b = key_def_lib.new(parts_b)
+
+ 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_a = key_def_lib.new({
+ {type = 'unsigned', fieldno = 1},
+ })
+ local key_def_b = key_def_lib.new({
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ })
+ local key_def_c = key_def_lib.new({
+ {type = 'scalar', fieldno = 2},
+ {type = 'scalar', fieldno = 1},
+ {type = 'string', fieldno = 4, is_nullable = true},
+ })
+ local tuple_a = box.tuple.new({1, 1, 22})
+
+ 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)
diff --git a/test/box/misc.result b/test/box/misc.result
index a1f7a0990..7d6d54241 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -522,6 +522,7 @@ t;
191: box.error.SQL_PARSER_LIMIT
192: box.error.INDEX_DEF_UNSUPPORTED
193: box.error.CK_DEF_UNSUPPORTED
+ 194: box.error.TUPLE_KEY_PART_MISSED
...
test_run:cmd("setopt delimiter ''");
---
--
2.21.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [tarantool-patches] [PATCH v3 1/1] lua: add key_def lua module
2019-04-22 12:28 [PATCH v3 1/1] lua: add key_def lua module Kirill Shcherbatov
@ 2019-04-24 18:13 ` Konstantin Osipov
2019-04-25 8:42 ` [tarantool-patches] " Alexander Turenko
2019-04-30 10:30 ` Vladimir Davydov
1 sibling, 1 reply; 7+ messages in thread
From: Konstantin Osipov @ 2019-04-24 18:13 UTC (permalink / raw)
To: tarantool-patches; +Cc: vdavydov.dev, alexander.turenko, Kirill Shcherbatov
* Kirill Shcherbatov <kshcherbatov@tarantool.org> [19/04/22 17:40]:
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -246,6 +246,7 @@ struct errcode_record {
> /*191 */_(ER_SQL_PARSER_LIMIT, "%s %d exceeds the limit (%d)") \
> /*192 */_(ER_INDEX_DEF_UNSUPPORTED, "%s are prohibited in an index definition") \
> /*193 */_(ER_CK_DEF_UNSUPPORTED, "%s are prohibited in a CHECK constraint definition") \
> + /*194 */_(ER_TUPLE_KEY_PART_MISSED, "Supplied tuple field for part %u does not exists") \
I don't understand this error message.
Besides key part number, the message should contain field name or
number.
Besides, we already have ER_NO_SUCH_FIELD_NO and
why not use it?
ER_NO_FIELD_FOR_KEY_PART "The supplied tuple has no field %s for key part %u"
> +
> +static uint32_t key_def_type_id = 0;
Please rename to key_def_ctype_id
> +static int
> +lbox_key_def_compare(struct lua_State *L)
> +{
> + struct key_def *key_def;
> + if (lua_gettop(L) != 3 ||
> + (key_def = luaT_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 = luaT_key_def_check_tuple(L, key_def, 2)) == NULL)
> + return luaT_error(L);
> + if ((tuple_b = luaT_key_def_check_tuple(L, key_def, 3)) == NULL) {
> + tuple_unref(tuple_a);
> + return luaT_error(L);
> + }
Invoking tuple_validate and possibly tuple_new on each compare is
awfully slow.
> +static int
> +lbox_key_def_compare_with_key(struct lua_State *L)
> +{
> + struct key_def *key_def;
> + if (lua_gettop(L) != 3 ||
> + (key_def = luaT_check_key_def(L, 1)) == NULL) {
> + return luaL_error(L, "Usage: key_def:"
> + "compare_with_key(tuple, key)");
> + }
> +
> + struct tuple *tuple = luaT_key_def_check_tuple(L, key_def, 2);
> + if (tuple == NULL)
> + return luaT_error(L);
> +
> + size_t key_len;
> + const char *key = lbox_encode_tuple_on_gc(L, 3, &key_len);
> + uint32_t part_count = mp_decode_array(&key);
> + if (key_validate_parts(key_def, key, part_count, true) != 0) {
> + tuple_unref(tuple);
> + return luaT_error(L);
> + }
> +
> + int rc = tuple_compare_with_key(tuple, key, part_count, key_def);
> + tuple_unref(tuple);
> + lua_pushinteger(L, rc);
> + return 1;
> +}
This also looks as a terribly inefficient implementation for
compare.
Overall, the API looks good to me, while the implementation seems
to be too inefficient. I would consider changing extract_key() to
return char *, not struct tuple, the buffer could be allocated on
transaction region. I also think that compare should not allocate
memory or create tuples, and it should not call tuple_validate()
either.
If this is urgent, I would push since the code quality is very
good and the api would stable, but I don't see how soo inefficient
compare functions could be useful.
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v3 1/1] lua: add key_def lua module
2019-04-24 18:13 ` [tarantool-patches] " Konstantin Osipov
@ 2019-04-25 8:42 ` Alexander Turenko
0 siblings, 0 replies; 7+ messages in thread
From: Alexander Turenko @ 2019-04-25 8:42 UTC (permalink / raw)
To: Konstantin Osipov; +Cc: tarantool-patches, vdavydov.dev, Kirill Shcherbatov
> > +static int
> > +lbox_key_def_compare_with_key(struct lua_State *L)
> > +{
> > + struct key_def *key_def;
> > + if (lua_gettop(L) != 3 ||
> > + (key_def = luaT_check_key_def(L, 1)) == NULL) {
> > + return luaL_error(L, "Usage: key_def:"
> > + "compare_with_key(tuple, key)");
> > + }
> > +
> > + struct tuple *tuple = luaT_key_def_check_tuple(L, key_def, 2);
> > + if (tuple == NULL)
> > + return luaT_error(L);
> > +
> > + size_t key_len;
> > + const char *key = lbox_encode_tuple_on_gc(L, 3, &key_len);
> > + uint32_t part_count = mp_decode_array(&key);
> > + if (key_validate_parts(key_def, key, part_count, true) != 0) {
> > + tuple_unref(tuple);
> > + return luaT_error(L);
> > + }
> > +
> > + int rc = tuple_compare_with_key(tuple, key, part_count, key_def);
> > + tuple_unref(tuple);
> > + lua_pushinteger(L, rc);
> > + return 1;
> > +}
>
> This also looks as a terribly inefficient implementation for
> compare.
>
> Overall, the API looks good to me, while the implementation seems
> to be too inefficient. I would consider changing extract_key() to
> return char *, not struct tuple, the buffer could be allocated on
> transaction region. I also think that compare should not allocate
> memory or create tuples, and it should not call tuple_validate()
> either.
>
> If this is urgent, I would push since the code quality is very
> good and the api would stable, but I don't see how soo inefficient
> compare functions could be useful.
We can add *_unchecked() functions, which will not perform validation.
I'm against returning char *, because a user can do nothing with it in
Lua.
We can consider a cursor implementation as motivating example. For a
cursor that navigates over an unique index we need :extract_key(), see
[1]. Comparing of tuples is needed if we'll try to implement cursors on
a non-unique index (but maybe better to implement #3898 ('Allow
specifying primary key in iteration by secondary key').
I don't know what Michael F. want when he file #3398 ('lua: introduce
key_def module'). Maybe he just needs merger. Maybe we should introduce
built-in tuple sorter instead of exposing comparators into Lua, which
can be either fast or safe, but not both.
[1]: https://github.com/Totktonada/tarantool-merger-examples/blob/695fc9511685033f4b4b22c0df537a12ddf2eaf6/chunked_example_fast/storage.lua#L97
WBR, Alexander Turenko.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/1] lua: add key_def lua module
2019-04-22 12:28 [PATCH v3 1/1] lua: add key_def lua module Kirill Shcherbatov
2019-04-24 18:13 ` [tarantool-patches] " Konstantin Osipov
@ 2019-04-30 10:30 ` Vladimir Davydov
2019-05-06 15:27 ` [tarantool-patches] " Kirill Shcherbatov
1 sibling, 1 reply; 7+ messages in thread
From: Vladimir Davydov @ 2019-04-30 10:30 UTC (permalink / raw)
To: Kirill Shcherbatov; +Cc: tarantool-patches, alexander.turenko
On Mon, Apr 22, 2019 at 03:28:55PM +0300, Kirill Shcherbatov wrote:
> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index 3f8cb8e0e..72485d3c8 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -246,6 +246,7 @@ struct errcode_record {
> /*191 */_(ER_SQL_PARSER_LIMIT, "%s %d exceeds the limit (%d)") \
> /*192 */_(ER_INDEX_DEF_UNSUPPORTED, "%s are prohibited in an index definition") \
> /*193 */_(ER_CK_DEF_UNSUPPORTED, "%s are prohibited in a CHECK constraint definition") \
> + /*194 */_(ER_TUPLE_KEY_PART_MISSED, "Supplied tuple field for part %u does not exists") \
We have ER_FIELD_MISSING, ER_NO_SUCH_FIELD_NO, ER_NO_SUCN_FIELD_NAME.
Please reuse rather than introducing yet another error code. Also the
error message should include a path to the missing field.
>
> /*
> * !IMPORTANT! Please follow instructions at start of the file
> diff --git a/src/box/lua/init.c b/src/box/lua/init.c
> index 3ff4f3227..6b8be5096 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/execute.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..d5fe237c0
> --- /dev/null
> +++ b/src/box/lua/key_def.c
> @@ -0,0 +1,425 @@
> +/*
> + * 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 "misc.h"
> +#include "tuple.h"
> +
> +static uint32_t key_def_type_id = 0;
Here are the names of other variables storing Lua type ids:
CTID_STRUCT_IBUF
CTID_STRUCT_PORT_PTR
CTID_STRUCT_ITERATOR_REF
CTID_STRUCT_ERROR_REF
Please rename this one to conform.
> +
> +void
> +luaT_push_key_def(struct lua_State *L, const struct key_def *key_def)
> +{
> + lua_createtable(L, key_def->part_count, 0);
> + for (uint32_t i = 0; i < key_def->part_count; ++i) {
> + const struct key_part *part = &key_def->parts[i];
> + 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");
> + }
> + lua_rawseti(L, -2, i + 1);
> + }
> +}
> +
> +/**
> + * Set key_part_def from a table on top of a Lua stack.
> + * The region argument is used to allocate a JSON path when
> + * required.
> + * 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,
> + struct region *region)
> +{
I typed this by mistake:
tarantool> key_def.new({fieldno = 1, type = 'unsigned'})
and got a valid key def:
---
- []
...
Can we possibly raise an error in this case?
> + *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(IllegalParams, "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;
> +}
> +
> +static struct tuple *
> +luaT_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx)
Please write a comment explaining briefly what this function does.
Relevant to all the static functions below.
> +{
> + struct tuple *tuple = luaT_istuple(L, idx);
> + if (tuple == NULL)
> + tuple = luaT_tuple_new(L, idx, box_tuple_format_default());
> + if (tuple == NULL || tuple_validate_parts(key_def, tuple) != 0)
> + return NULL;
> + tuple_ref(tuple);
> + return tuple;
> +}
> +
> +struct key_def *
> +luaT_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 = luaT_check_key_def(L, 1);
> + if (key_def == NULL)
> + return 0;
Hmm, how's that even possible?
> + 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 = luaT_check_key_def(L, 1)) == NULL)
> + return luaL_error(L, "Usage: key_def:extract_key(tuple)");
> +
> + struct tuple *tuple;
> + if ((tuple = luaT_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_extract_key allocates a key on the region.
You ought to clean it up once you're done with it.
> + tuple_unref(tuple);
> + if (key == NULL)
> + return luaT_error(L);
> +
> + struct tuple *ret =
> + box_tuple_new(box_tuple_format_default(), key, key + key_size);
Please don't use box_ functions here - those are intended to be used
only by external modules. Use tuple_new, tuple_format_runtime directly.
It's especially confusing when you create a key def with key_def_new,
but free it with box_key_def_delete.
Anyway, do we really need to create a tuple here? May be returning a Lua
table would be fine?
> + 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 = luaT_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 = luaT_key_def_check_tuple(L, key_def, 2)) == NULL)
> + return luaT_error(L);
> + if ((tuple_b = luaT_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 = luaT_check_key_def(L, 1)) == NULL) {
> + return luaL_error(L, "Usage: key_def:"
> + "compare_with_key(tuple, key)");
> + }
> +
> + struct tuple *tuple = luaT_key_def_check_tuple(L, key_def, 2);
> + if (tuple == NULL)
> + return luaT_error(L);
> +
> + size_t key_len;
> + const char *key = lbox_encode_tuple_on_gc(L, 3, &key_len);
Again, this function allocates a key on the region. Please clean it up.
> + uint32_t part_count = mp_decode_array(&key);
> + if (key_validate_parts(key_def, key, part_count, true) != 0) {
> + tuple_unref(tuple);
> + return luaT_error(L);
> + }
> +
> + int rc = tuple_compare_with_key(tuple, key, part_count, key_def);
> + tuple_unref(tuple);
> + lua_pushinteger(L, rc);
> + return 1;
> +}
> diff --git a/src/box/lua/key_def.h b/src/box/lua/key_def.h
> new file mode 100644
> index 000000000..2cecfaae5
> --- /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_def;
> +
> +/**
> + * Push a new table representing a key_def to a Lua stack.
> + */
> +void
> +luaT_push_key_def(struct lua_State *L, const struct key_def *key_def);
> +
> +/**
> + * Extract a key_def object from a Lua stack.
The comment is insufficient: what happens if the object isn't a key def?
> + */
> +struct key_def *
> +luaT_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/tuple.h b/src/box/tuple.h
> index eed8e1a34..a3ac45574 100644
> --- a/src/box/tuple.h
> +++ b/src/box/tuple.h
> @@ -670,6 +670,34 @@ tuple_field_by_part(struct tuple *tuple, struct key_part *part)
> tuple_field_map(tuple), part);
> }
>
> +/**
> + * Check that tuple fields match with given key definition
> + * key_def.
> + * @param key_def Key definition.
> + * @param tuple Tuple to validate.
> + *
> + * @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)
> +{
IMO tuple.h isn't a right place for this function:
- First, it's relatively big and cold so I'd move it to a C file.
- Second, it's about checking if a tuple conforms to a key_def
while tuple.[hc] stores only basic struct tuple helpers and
shouldn't depend on key_def (although sadly it does for now).
The name is somewhat confusing too: we now have tuple_validate and
tuple_validate_parts both of which validate tuple parts...
What about
- renaming it to tuple_validate_key_parts so as to emphasize that it's
about key parts, not tuple parts;
- declaring it in key_def.h while defining it in tuple_extract_key.cc,
similarly to tuple_key_contains_null, as this function can be used
for checking if we can extract a specific key from a given tuple
?
> + 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) {
> + if (key_part_is_nullable(part))
> + continue;
> + diag_set(ClientError, ER_TUPLE_KEY_PART_MISSED, idx);
> + return -1;
> + }
> + if (key_part_validate(part->type, field, idx,
> + key_part_is_nullable(part)) != 0)
> + return -1;
> + }
> + return 0;
> +}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v3 1/1] lua: add key_def lua module
2019-04-30 10:30 ` Vladimir Davydov
@ 2019-05-06 15:27 ` Kirill Shcherbatov
2019-05-06 15:35 ` Alexander Turenko
2019-05-06 16:25 ` Vladimir Davydov
0 siblings, 2 replies; 7+ messages in thread
From: Kirill Shcherbatov @ 2019-05-06 15:27 UTC (permalink / raw)
To: tarantool-patches, Vladimir Davydov; +Cc: alexander.turenko
Hi! Thank you for review.
> We have ER_FIELD_MISSING, ER_NO_SUCH_FIELD_NO, ER_NO_SUCN_FIELD_NAME.
> Please reuse rather than introducing yet another error code. Also the
> error message should include a path to the missing field.
Replaced with
diag_set(ClientError, ER_FIELD_MISSING,
tt_sprintf("[%d]%.*s", part->fieldno + TUPLE_INDEX_BASE,
part->path_len, part->path));
>> +static uint32_t key_def_type_id = 0;
>
> Here are the names of other variables storing Lua type ids:
>
> CTID_STRUCT_IBUF
> CTID_STRUCT_PORT_PTR
> CTID_STRUCT_ITERATOR_REF
> CTID_STRUCT_ERROR_REF
>
> Please rename this one to conform.
Done.
> I typed this by mistake:
>
> tarantool> key_def.new({fieldno = 1, type = 'unsigned'})
>
> and got a valid key def:
>
> ---
> - []
> ...
>
> Can we possibly raise an error in this case?
I've disallowed creating a key_def with part_count = 0 so this is not problem
anymore.
>> +static struct tuple *
>> +luaT_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx)
>
> Please write a comment explaining briefly what this function does.
> Relevant to all the static functions below.
/**
* Set key_part_def from a table on top of a Lua stack.
* The region argument is used to allocate a JSON path when
* required.
* 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,
struct region *region)
/**
* Check an existent tuple pointer in LUA stack by specified
* index or attemt to construct it by LUA table.
* Increase tuple's reference counter.
* Returns not NULL tuple pointer on success, NULL otherwise.
*/
static struct tuple *
luaT_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx)
/**
* Extract key from tuple by given key definition and return
* tuple representing this key.
* Push the new key tuple as cdata to a LUA stack on success.
* Raise error otherwise.
*/
static int
lbox_key_def_extract_key(struct lua_State *L)
/**
* Compare tuples using the key definition.
* Push 0 if key_fields(tuple_a) == key_fields(tuple_b)
* <0 if key_fields(tuple_a) < key_fields(tuple_b)
* >0 if key_fields(tuple_a) > key_fields(tuple_b)
* integer to a LUA stack on success.
* Raise error otherwise.
*/
static int
lbox_key_def_compare(struct lua_State *L)
/**
* Compare tuple with key using the key definition.
* Push 0 if key_fields(tuple) == parts(key)
* <0 if key_fields(tuple) < parts(key)
* >0 if key_fields(tuple) > parts(key)
* integer to a LUA stack on success.
* Raise error otherwise.
*/
static int
lbox_key_def_compare_with_key(struct lua_State *L)
/**
* Construct and export to LUA a new key definition with a set
* union of key parts from first and second key defs. Parts of
* the new key_def consist of the first key_def's parts and those
* parts of the second key_def that were not among the first
* parts.
* Push the new key_def as cdata to a LUA stack on success.
* Raise error otherwise.
*/
static int
lbox_key_def_merge(struct lua_State *L)
/**
* Push a new table representing a key_def to a Lua stack.
*/
static int
lbox_key_def_to_table(struct lua_State *L)
/**
* 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)
>> +static int
>> +lbox_key_def_gc(struct lua_State *L)
>> +{
>> + struct key_def *key_def = luaT_check_key_def(L, 1);
>> + if (key_def == NULL)
>> + return 0;
>
> Hmm, how's that even possible?
Nope, replaced with assert.
>> + struct tuple *ret =
>> + box_tuple_new(box_tuple_format_default(), key, key + key_size);
>
> Please don't use box_ functions here - those are intended to be used
> only by external modules. Use tuple_new, tuple_format_runtime directly.
> It's especially confusing when you create a key def with key_def_new,
> but free it with box_key_def_delete.
Done.
> Anyway, do we really need to create a tuple here? May be returning a Lua
> table would be fine?
Discussed to do not implement it because of it's complexity and lack of
expediency.
>> + uint32_t key_size;
>> + char *key = tuple_extract_key(tuple, key_def, &key_size);
>
> tuple_extract_key allocates a key on the region.
> You ought to clean it up once you're done with it.
>> + size_t key_len;
>> + const char *key = lbox_encode_tuple_on_gc(L, 3, &key_len);
>
> Again, this function allocates a key on the region. Please clean it up.
Done.
>> +/**
>> + * Extract a key_def object from a Lua stack.
>
> The comment is insufficient: what happens if the object isn't a key def?
>
/**
* Push a new table representing a key_def to a Lua stack.
* Table is consists of key_def::parts tables that describe
* each part correspondingly.
* The collation and path fields are optional so resulting
* object doesn't declare them where not necessary.
*/
void
luaT_push_key_def(struct lua_State *L, const struct key_def *key_def);
/**
* Check key_def pointer in LUA stack by specified index.
* The value by idx is expected to be key_def's cdata.
* Returns not NULL tuple pointer on success, NULL otherwise.
*/
struct key_def *
luaT_check_key_def(struct lua_State *L, int idx);
> IMO tuple.h isn't a right place for this function:
>
> - First, it's relatively big and cold so I'd move it to a C file.
> - Second, it's about checking if a tuple conforms to a key_def
> while tuple.[hc] stores only basic struct tuple helpers and
> shouldn't depend on key_def (although sadly it does for now).
>
> The name is somewhat confusing too: we now have tuple_validate and
> tuple_validate_parts both of which validate tuple parts...
>
> What about
>
> - renaming it to tuple_validate_key_parts so as to emphasize that it's
> about key parts, not tuple parts;
> - declaring it in key_def.h while defining it in tuple_extract_key.cc,
> similarly to tuple_key_contains_null, as this function can be used
> for checking if we can extract a specific key from a given tuple
>
> ?
Song good. Done.
==========================================================
Needed for #3276.
Fixes #3398.
Fixes #4025.
@TarantoolBot document
Title: lua: key_def module
It is convenient to have tuple compare function into lua-land for the
following case:
- exporting key from tuple to iterate over secondary non-unique index
and delete tuples from space
- support comparing a tuple with a key / a tuple, support merging
key_defs from Lua
- factor out key parts parsing code from the tuples merger
A key_def instance has the following methods:
- :extract_key(tuple) -> key (as tuple)
Receives tuple or Lua table. Returns tuple representing extracted
key.
- :compare(tuple_a, tuple_b) -> number
Receives tuples or Lua tables.
Returns:
- a value > 0 when tuple_a > tuple_b,
- a value == 0 when tuple_a == tuple_b,
- a value < 0 otherwise.
- :compare_with_key(tuple, key) -> number
- a value > 0 when key(tuple) > key,
- a value == 0 when key(tuple) == key,
- a value < 0 otherwise.
- :merge(another_key_def) -> new key_def instance
Constructs a new key definition with a set union of key parts
from first and second key defs
- :totable() -> table
Dump key_def object as a Lua table (needed to support __serialize
method)
The root key_def library exports all instance methods directly.
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.
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/key_def.h | 12 +
src/box/lua/init.c | 5 +
src/box/lua/key_def.c | 480 ++++++++++++++++++++++++++++++++++
src/box/lua/key_def.h | 69 +++++
src/box/lua/key_def.lua | 19 ++
src/box/lua/space.cc | 34 +--
| 22 ++
test/box-tap/key_def.test.lua | 453 ++++++++++++++++++++++++++++++++
10 files changed, 1065 insertions(+), 32 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 31600745a..7fbbc7803 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)
@@ -140,6 +141,7 @@ add_library(box STATIC
lua/net_box.c
lua/xlog.c
lua/execute.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/key_def.h b/src/box/key_def.h
index 6205c278a..ff0dcbecf 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -515,6 +515,18 @@ key_part_cmp(const struct key_part *parts1, uint32_t part_count1,
bool
tuple_key_contains_null(struct tuple *tuple, struct key_def *def);
+/**
+ * Check that tuple fields match with given key definition
+ * key_def.
+ * @param key_def Key definition.
+ * @param tuple Tuple to validate.
+ *
+ * @retval 0 The tuple is valid.
+ * @retval -1 The tuple is invalid.
+ */
+int
+tuple_validate_key_parts(struct key_def *key_def, struct tuple *tuple);
+
/**
* Extract key from tuple by given key definition and return
* buffer allocated on box_txn_alloc with this key. This function
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 3ff4f3227..6b8be5096 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/execute.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..0e1236093
--- /dev/null
+++ b/src/box/lua/key_def.c
@@ -0,0 +1,480 @@
+/*
+ * 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 "misc.h"
+#include "tuple.h"
+
+static uint32_t CTID_STRUCT_KEY_DEF_REF = 0;
+
+void
+luaT_push_key_def(struct lua_State *L, const struct key_def *key_def)
+{
+ lua_createtable(L, key_def->part_count, 0);
+ for (uint32_t i = 0; i < key_def->part_count; ++i) {
+ const struct key_part *part = &key_def->parts[i];
+ 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");
+ }
+ lua_rawseti(L, -2, i + 1);
+ }
+}
+
+/**
+ * Set key_part_def from a table on top of a Lua stack.
+ * The region argument is used to allocate a JSON path when
+ * required.
+ * 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,
+ struct region *region)
+{
+ *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(IllegalParams, "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;
+}
+
+/**
+ * Check an existent tuple pointer in LUA stack by specified
+ * index or attemt to construct it by LUA table.
+ * Increase tuple's reference counter.
+ * Returns not NULL tuple pointer on success, NULL otherwise.
+ */
+static struct tuple *
+luaT_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 || tuple_validate_key_parts(key_def, tuple) != 0)
+ return NULL;
+ tuple_ref(tuple);
+ return tuple;
+}
+
+struct key_def *
+luaT_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 != CTID_STRUCT_KEY_DEF_REF)
+ 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 = luaT_check_key_def(L, 1);
+ assert(key_def != NULL);
+ key_def_delete(key_def);
+ return 0;
+}
+
+/**
+ * Extract key from tuple by given key definition and return
+ * tuple representing this key.
+ * Push the new key tuple as cdata to a LUA stack on success.
+ * Raise error otherwise.
+ */
+static int
+lbox_key_def_extract_key(struct lua_State *L)
+{
+ struct key_def *key_def;
+ if (lua_gettop(L) != 2 || (key_def = luaT_check_key_def(L, 1)) == NULL)
+ return luaL_error(L, "Usage: key_def:extract_key(tuple)");
+
+ struct tuple *tuple;
+ if ((tuple = luaT_key_def_check_tuple(L, key_def, 2)) == NULL)
+ return luaT_error(L);
+
+ struct region *region = &fiber()->gc;
+ size_t region_svp = region_used(region);
+ 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 =
+ tuple_new(tuple_format_runtime, key, key + key_size);
+ region_truncate(region, region_svp);
+ if (ret == NULL)
+ return luaT_error(L);
+ luaT_pushtuple(L, ret);
+ return 1;
+}
+
+/**
+ * Compare tuples using the key definition.
+ * Push 0 if key_fields(tuple_a) == key_fields(tuple_b)
+ * <0 if key_fields(tuple_a) < key_fields(tuple_b)
+ * >0 if key_fields(tuple_a) > key_fields(tuple_b)
+ * integer to a LUA stack on success.
+ * Raise error otherwise.
+ */
+static int
+lbox_key_def_compare(struct lua_State *L)
+{
+ struct key_def *key_def;
+ if (lua_gettop(L) != 3 ||
+ (key_def = luaT_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 = luaT_key_def_check_tuple(L, key_def, 2)) == NULL)
+ return luaT_error(L);
+ if ((tuple_b = luaT_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;
+}
+
+/**
+ * Compare tuple with key using the key definition.
+ * Push 0 if key_fields(tuple) == parts(key)
+ * <0 if key_fields(tuple) < parts(key)
+ * >0 if key_fields(tuple) > parts(key)
+ * integer to a LUA stack on success.
+ * Raise error otherwise.
+ */
+static int
+lbox_key_def_compare_with_key(struct lua_State *L)
+{
+ struct key_def *key_def;
+ if (lua_gettop(L) != 3 ||
+ (key_def = luaT_check_key_def(L, 1)) == NULL) {
+ return luaL_error(L, "Usage: key_def:"
+ "compare_with_key(tuple, key)");
+ }
+
+ struct tuple *tuple = luaT_key_def_check_tuple(L, key_def, 2);
+ if (tuple == NULL)
+ return luaT_error(L);
+
+ struct region *region = &fiber()->gc;
+ size_t region_svp = region_used(region);
+ size_t key_len;
+ const char *key = lbox_encode_tuple_on_gc(L, 3, &key_len);
+ uint32_t part_count = mp_decode_array(&key);
+ if (key_validate_parts(key_def, key, part_count, true) != 0) {
+ region_truncate(region, region_svp);
+ tuple_unref(tuple);
+ return luaT_error(L);
+ }
+
+ int rc = tuple_compare_with_key(tuple, key, part_count, key_def);
+ region_truncate(region, region_svp);
+ tuple_unref(tuple);
+ lua_pushinteger(L, rc);
+ return 1;
+}
+
+/**
+ * Construct and export to LUA a new key definition with a set
+ * union of key parts from first and second key defs. Parts of
+ * the new key_def consist of the first key_def's parts and those
+ * parts of the second key_def that were not among the first
+ * parts.
+ * Push the new key_def as cdata to a LUA stack on success.
+ * Raise error otherwise.
+ */
+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 = luaT_check_key_def(L, 1)) == NULL ||
+ (key_def_b = luaT_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,
+ CTID_STRUCT_KEY_DEF_REF) = new_key_def;
+ lua_pushcfunction(L, lbox_key_def_gc);
+ luaL_setcdatagc(L, -2);
+ return 1;
+}
+
+
+/**
+ * Push a new table representing a key_def to a Lua stack.
+ */
+static int
+lbox_key_def_to_table(struct lua_State *L)
+{
+ struct key_def *key_def;
+ if (lua_gettop(L) != 1 || (key_def = luaT_check_key_def(L, 1)) == NULL)
+ return luaL_error(L, "Usage: key_def:totable()");
+
+ luaT_push_key_def(L, key_def);
+ 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);
+ }
+ if (part_count == 0) {
+ diag_set(IllegalParams, "Key definition can only be constructed"
+ " by using at least 1 key_part");
+ 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);
+ }
+ lua_pop(L, 1);
+ }
+
+ struct key_def *key_def = key_def_new(parts, part_count);
+ region_truncate(region, region_svp);
+ if (key_def == NULL)
+ return luaT_error(L);
+
+ /*
+ * Compare and extract key_def methods must work even with
+ * tuples with omitted (optional) fields. As there is no
+ * space format which would guarantee certain minimal
+ * field_count, pass min_field_count = 0 to ensure that
+ * functions will work correctly in such case.
+ */
+ key_def_update_optionality(key_def, 0);
+
+ *(struct key_def **) luaL_pushcdata(L,
+ CTID_STRUCT_KEY_DEF_REF) = 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;");
+ CTID_STRUCT_KEY_DEF_REF = luaL_ctypeid(L, "struct key_def&");
+
+ /* Export C functions to Lua. */
+ static const struct luaL_Reg meta[] = {
+ {"new", lbox_key_def_new},
+ {"extract_key", lbox_key_def_extract_key},
+ {"compare", lbox_key_def_compare},
+ {"compare_with_key", lbox_key_def_compare_with_key},
+ {"merge", lbox_key_def_merge},
+ {"totable", lbox_key_def_to_table},
+ {NULL, NULL}
+ };
+ luaL_register_module(L, "key_def", meta);
+ return 1;
+}
diff --git a/src/box/lua/key_def.h b/src/box/lua/key_def.h
new file mode 100644
index 000000000..88b8a0019
--- /dev/null
+++ b/src/box/lua/key_def.h
@@ -0,0 +1,69 @@
+#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_def;
+
+/**
+ * Push a new table representing a key_def to a Lua stack.
+ * Table is consists of key_def::parts tables that describe
+ * each part correspondingly.
+ * The collation and path fields are optional so resulting
+ * object doesn't declare them where not necessary.
+ */
+void
+luaT_push_key_def(struct lua_State *L, const struct key_def *key_def);
+
+/**
+ * Check key_def pointer in LUA stack by specified index.
+ * The value by idx is expected to be key_def's cdata.
+ * Returns not NULL tuple pointer on success, NULL otherwise.
+ */
+struct key_def *
+luaT_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..586005709
--- /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.extract_key,
+ ['compare'] = key_def.compare,
+ ['compare_with_key'] = key_def.compare_with_key,
+ ['merge'] = key_def.merge,
+ ['totable'] = key_def.totable,
+ ['__serialize'] = key_def.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 93269b72b..100da0a79 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,38 +287,7 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i)
lua_setfield(L, -2, "name");
lua_pushstring(L, "parts");
- lua_newtable(L);
-
- 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");
- }
-
- lua_settable(L, -3); /* index[k].parts[j] */
- }
+ luaT_push_key_def(L, index_def->key_def);
lua_settable(L, -3); /* space.index[k].parts */
--git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc
index 3c49094b8..d7507bc4e 100644
--- a/src/box/tuple_extract_key.cc
+++ b/src/box/tuple_extract_key.cc
@@ -422,3 +422,25 @@ tuple_key_contains_null(struct tuple *tuple, struct key_def *def)
}
return false;
}
+
+int
+tuple_validate_key_parts(struct key_def *key_def, struct tuple *tuple)
+{
+ 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) {
+ if (key_part_is_nullable(part))
+ continue;
+ diag_set(ClientError, ER_FIELD_MISSING,
+ tt_sprintf("[%d]%.*s",
+ part->fieldno + TUPLE_INDEX_BASE,
+ part->path_len, part->path));
+ return -1;
+ }
+ if (key_part_validate(part->type, field, idx,
+ key_part_is_nullable(part)) != 0)
+ return -1;
+ }
+ return 0;
+}
diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua
new file mode 100755
index 000000000..c52ff48fe
--- /dev/null
+++ b/test/box-tap/key_def.test.lua
@@ -0,0 +1,453 @@
+#!/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 no key parts',
+ parts = {},
+ exp_err = 'Key definition can only be constructed by using at least 1 key_part',
+ },
+ {
+ "Pass a garbage instead of key parts",
+ parts = {fieldno = 1, type = 'unsigned'},
+ exp_err = 'Key definition can only be constructed by using at least 1 key_part',
+ },
+ {
+ '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 = 'invalid path',
+ },
+ {
+ '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.
+
+-- Case: extract_key().
+test:test('extract_key()', function(test)
+ test:plan(13)
+
+ local key_def_a = key_def_lib.new({
+ {type = 'unsigned', fieldno = 1},
+ })
+ local key_def_b = key_def_lib.new({
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ })
+ local key_def_c = key_def_lib.new({
+ {type = 'scalar', fieldno = 2},
+ {type = 'scalar', fieldno = 1},
+ {type = 'string', fieldno = 4, is_nullable = true},
+ })
+ local tuple_a = box.tuple.new({1, 1, 22})
+
+ 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 = 'Tuple field [2] required by space format is missing'
+ 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 = 'Tuple field [2] required by space format is missing'
+ 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 = 'Tuple field [2] required by space format is missing'
+ 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')
+
+ local key_def = key_def_lib.new({
+ {type = 'number', fieldno = 1, path='a'},
+ {type = 'number', fieldno = 1, path='b'},
+ {type = 'number', fieldno = 1, path='c', is_nullable=true},
+ {type = 'number', fieldno = 3, is_nullable=true},
+ })
+ local ok, err = pcall(key_def.extract_key, key_def,
+ box.tuple.new({1, 1, 22}))
+ test:is_deeply({ok, tostring(err)},
+ {false, 'Tuple field [1]a required by space format is missing'},
+ 'invalid JSON structure')
+ test:is_deeply(key_def:extract_key({{a=1, b=2}, 1}):totable(),
+ {1, 2, box.NULL, box.NULL},
+ 'tuple with optional parts - case 1')
+ test:is_deeply(key_def:extract_key({{a=1, b=2, c=3}, 1}):totable(),
+ {1, 2, 3, box.NULL},
+ 'tuple with optional parts - case 2')
+ test:is_deeply(key_def:extract_key({{a=1, b=2}, 1, 3}):totable(),
+ {1, 2, box.NULL, 3},
+ 'tuple with optional parts - case 3')
+end)
+
+-- Case: compare().
+test:test('compare()', function(test)
+ test:plan(8)
+
+ local key_def_a = key_def_lib.new({
+ {type = 'unsigned', fieldno = 1},
+ })
+ local key_def_b = key_def_lib.new({
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ })
+ 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})
+
+ 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_def_b = key_def_lib.new({
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ })
+ local tuple_a = box.tuple.new({1, 1, 22})
+
+ 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 parts_a = {
+ {type = 'unsigned', fieldno = 1}
+ }
+ local parts_b = {
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ }
+ local key_def_a = key_def_lib.new(parts_a)
+ local key_def_b = key_def_lib.new(parts_b)
+
+ 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 parts_a = {
+ {type = 'unsigned', fieldno = 1}
+ }
+ local parts_b = {
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ }
+ local key_def_a = key_def_lib.new(parts_a)
+ local key_def_b = key_def_lib.new(parts_b)
+
+ 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 parts_a = {
+ {type = 'unsigned', fieldno = 1}
+ }
+ local parts_b = {
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ }
+ local key_def_a = key_def_lib.new(parts_a)
+ local key_def_b = key_def_lib.new(parts_b)
+
+ 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_a = key_def_lib.new({
+ {type = 'unsigned', fieldno = 1},
+ })
+ local key_def_b = key_def_lib.new({
+ {type = 'number', fieldno = 2},
+ {type = 'number', fieldno = 3},
+ })
+ local key_def_c = key_def_lib.new({
+ {type = 'scalar', fieldno = 2},
+ {type = 'scalar', fieldno = 1},
+ {type = 'string', fieldno = 4, is_nullable = true},
+ })
+ local tuple_a = box.tuple.new({1, 1, 22})
+
+ 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] 7+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v3 1/1] lua: add key_def lua module
2019-05-06 15:27 ` [tarantool-patches] " Kirill Shcherbatov
@ 2019-05-06 15:35 ` Alexander Turenko
2019-05-06 16:25 ` Vladimir Davydov
1 sibling, 0 replies; 7+ messages in thread
From: Alexander Turenko @ 2019-05-06 15:35 UTC (permalink / raw)
To: Kirill Shcherbatov; +Cc: tarantool-patches, Vladimir Davydov
> /**
> * Check an existent tuple pointer in LUA stack by specified
Nit: Lua is not an acronym.
https://stackoverflow.com/a/13615976
> * index or attemt to construct it by LUA table.
> * Increase tuple's reference counter.
> * Returns not NULL tuple pointer on success, NULL otherwise.
> */
> static struct tuple *
> luaT_key_def_check_tuple(struct lua_State *L, struct key_def *key_def, int idx)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [tarantool-patches] Re: [PATCH v3 1/1] lua: add key_def lua module
2019-05-06 15:27 ` [tarantool-patches] " Kirill Shcherbatov
2019-05-06 15:35 ` Alexander Turenko
@ 2019-05-06 16:25 ` Vladimir Davydov
1 sibling, 0 replies; 7+ messages in thread
From: Vladimir Davydov @ 2019-05-06 16:25 UTC (permalink / raw)
To: Kirill Shcherbatov; +Cc: tarantool-patches, alexander.turenko
Pushed to master, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-06 16:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-22 12:28 [PATCH v3 1/1] lua: add key_def lua module Kirill Shcherbatov
2019-04-24 18:13 ` [tarantool-patches] " Konstantin Osipov
2019-04-25 8:42 ` [tarantool-patches] " Alexander Turenko
2019-04-30 10:30 ` Vladimir Davydov
2019-05-06 15:27 ` [tarantool-patches] " Kirill Shcherbatov
2019-05-06 15:35 ` Alexander Turenko
2019-05-06 16:25 ` Vladimir Davydov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox