Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH v2 4/6] lua: add luaT_new_key_def()
Date: Tue, 29 Jan 2019 21:52:38 +0300	[thread overview]
Message-ID: <20190129185238.vx6bdzdb2ublyorz@tkn_work_nb> (raw)
In-Reply-To: <20190110130756.iwtn6knjg5r4tizo@esperanza>

I considered https://github.com/tarantool/tarantool/issues/3398 and
decided to implement full-featured key_def lua module. Now I only
created the module stub with key_def.new() function. It is enough for
merger and I hope we can leave #3398 unimplemented for now (to fix it a
bit later).

Removed exports, moved the test cases from module-api test to a separate
file.

I replaced merger.context.new(key_parts) with
merger.context.new(key_def.new(key_parts)).

I added docbot comment, because the module becomes user visible and is
employed in the merger's docbot examples. That is why I think it is
better to have it documented despite the fact it is just stub for now.

Other comments are below. The new patch at the end of the email.

NB: branch: Totktonada/gh-3276-on-board-merger

WBR, Alexander Turenko.

> > +#include "box/lua/key_def.h"
> > +
> > +#include <lua.h>
> > +#include <lauxlib.h>
> > +#include "diag.h"
> > +#include "box/key_def.h"
> > +#include "box/box.h"
> > +#include "box/coll_id_cache.h"
> > +#include "lua/utils.h"
> > +
> > +struct key_def *
> > +luaT_new_key_def(struct lua_State *L, int idx)
> 
> If you agree with luaT_tuple_new, then rename this function to
> luaT_key_def_new pls.

The code was moved to lbox_key_def_new() and luaT_key_def_set_part().

> 
> > +{
> > +	if (lua_istable(L, idx) != 1) {
> > +		luaL_error(L, "Bad params, use: luaT_new_key_def({"
> > +				  "{fieldno = fieldno, type = type"
> > +				  "[, is_nullable = is_nullable"
> > +				  "[, collation_id = collation_id"
> 
> Hm, what's collation_id for?

net.box exposes index parts in that way:
https://github.com/tarantool/tarantool/issues/3941

I'll leave collation_id here for now if you don't mind and will remove
it in the scope of #3941.

> 
> > +				  "[, collation = collation]]]}, ...}");
> 
> This looks like you can't specify collation without is_nullable.
> Should be
> 
> 	luaT_new_key_def({{fieldno = FIELDNO, type = TYPE[, is_nullable = true | false][, collation = COLLATION]}})

Changed to:

luaL_error(L, "Bad params, use: key_def.new({"                           
              "{fieldno = fieldno, type = type"                          
              "[, is_nullable = <boolean>]"                              
              "[, collation_id = <number>]"                              
              "[, collation = <string>]}, ...}");  

> 
> > +		unreachable();
> > +		return NULL;
> > +	}
> > +	uint32_t key_parts_count = 0;
> > +	uint32_t capacity = 8;
> > +
> > +	const ssize_t parts_size = sizeof(struct key_part_def) * capacity;
> 
> Can't we figure out the table length right away instead of reallocaing
> key_part_def array?

Sure. Fixed.

> 
> > +	struct key_part_def *parts = NULL;
> > +	parts = (struct key_part_def *) malloc(parts_size);
> > +	if (parts == NULL) {
> > +		diag_set(OutOfMemory, parts_size, "malloc", "parts");
> > +		luaT_error(L);
> > +		unreachable();
> > +		return NULL;
> > +	}
> > +
> > +	while (true) {
> 
> Would be nice to factor out part creation to a separate function.

Done.

> 
> > +		lua_pushinteger(L, key_parts_count + 1);
> 
> We would call this variable key_part_count (without 's') or even just
> part_count, as you called the array of key parts simply 'parts'.

Ok. Fixed.

> 
> > +		lua_gettable(L, idx);
> > +		if (lua_isnil(L, -1))
> > +			break;
> > +
> > +		/* Extend parts if necessary. */
> > +		if (key_parts_count == capacity) {
> > +			capacity *= 2;
> > +			struct key_part_def *old_parts = parts;
> > +			const ssize_t parts_size =
> > +				sizeof(struct key_part_def) * capacity;
> > +			parts = (struct key_part_def *) realloc(parts,
> > +								parts_size);
> > +			if (parts == NULL) {
> > +				free(old_parts);
> > +				diag_set(OutOfMemory, parts_size / 2, "malloc",
> > +					 "parts");
> > +				luaT_error(L);
> > +				unreachable();
> > +				return NULL;
> > +			}
> > +		}
> > +
> > +		/* Set parts[key_parts_count].fieldno. */
> > +		lua_pushstring(L, "fieldno");
> > +		lua_gettable(L, -2);
> > +		if (lua_isnil(L, -1)) {
> > +			free(parts);
> > +			luaL_error(L, "fieldno must not be nil");
> > +			unreachable();
> > +			return NULL;
> > +		}
> > +		/*
> > +		 * Transform one-based Lua fieldno to zero-based
> > +		 * fieldno to use in key_def_new().
> > +		 */
> > +		parts[key_parts_count].fieldno = lua_tointeger(L, -1) - 1;
> 
> Use TUPLE_INDEX_BASE instead of 1 pls.

Fixed.

> 
> > +		lua_pop(L, 1);
> > +
> > +		/* Set parts[key_parts_count].type. */
> > +		lua_pushstring(L, "type");
> > +		lua_gettable(L, -2);
> > +		if (lua_isnil(L, -1)) {
> > +			free(parts);
> > +			luaL_error(L, "type must not be nil");
> > +			unreachable();
> > +			return NULL;
> > +		}
> > +		size_t type_len;
> > +		const char *type_name = lua_tolstring(L, -1, &type_len);
> > +		lua_pop(L, 1);
> > +		parts[key_parts_count].type = field_type_by_name(type_name,
> > +								 type_len);
> > +		if (parts[key_parts_count].type == field_type_MAX) {
> > +			free(parts);
> > +			luaL_error(L, "Unknown field type: %s", type_name);
> > +			unreachable();
> > +			return NULL;
> > +		}
> > +
> > +		/*
> > +		 * Set parts[key_parts_count].is_nullable and
> > +		 * parts[key_parts_count].nullable_action.
> > +		 */
> > +		lua_pushstring(L, "is_nullable");
> > +		lua_gettable(L, -2);
> > +		if (lua_isnil(L, -1)) {
> > +			parts[key_parts_count].is_nullable = false;
> > +			parts[key_parts_count].nullable_action =
> > +				ON_CONFLICT_ACTION_DEFAULT;
> > +		} else {
> > +			parts[key_parts_count].is_nullable =
> > +				lua_toboolean(L, -1);
> > +			parts[key_parts_count].nullable_action =
> > +				ON_CONFLICT_ACTION_NONE;
> > +		}
> > +		lua_pop(L, 1);
> > +
> > +		/* Set parts[key_parts_count].coll_id using collation_id. */
> > +		lua_pushstring(L, "collation_id");
> > +		lua_gettable(L, -2);
> > +		if (lua_isnil(L, -1))
> > +			parts[key_parts_count].coll_id = COLL_NONE;
> > +		else
> > +			parts[key_parts_count].coll_id = lua_tointeger(L, -1);
> > +		lua_pop(L, 1);
> > +
> > +		/* Set parts[key_parts_count].coll_id using collation. */
> > +		lua_pushstring(L, "collation");
> > +		lua_gettable(L, -2);
> > +		/* Check whether box.cfg{} was called. */
> 
> Collations should be usable even without box.cfg IIRC. Well, not all of
> them I think, but still you don't need to check box.cfg() here AFAIU.

Removed box.cfg{} check. No collations are available before box.cfg{}
and we'll get an error for any collation ('Unknown collation "foo"' when
it is pointed by name).

Removed coll_id correctness check: it is performed if key_def_new()
anyway.

> 
> > +		if ((parts[key_parts_count].coll_id != COLL_NONE ||
> > +		    !lua_isnil(L, -1)) && !box_is_configured()) {
> > +			free(parts);
> > +			luaL_error(L, "Cannot use collations: "
> > +				      "please call box.cfg{}");
> > +			unreachable();
> > +			return NULL;
> > +		}
> > +		if (!lua_isnil(L, -1)) {
> > +			if (parts[key_parts_count].coll_id != COLL_NONE) {
> > +				free(parts);
> > +				luaL_error(L, "Conflicting options: "
> > +					      "collation_id and collation");
> > +				unreachable();
> > +				return NULL;
> > +			}
> > +			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);
> 
> Ouch, this doesn't seem to belong here. Ideally, it should be done by
> key_def_new(). Can we rework key_part_def so that it stores collation
> string instead of collation id?

Can it have negative performance impact? It seems we don't compare
key_defs, but I don't sure.

The format of vy_log_record_encode() will change and we'll need to
create upgrade script for old format.

We use numeric collation IDs in many places. It seems the change is
possible, but will heavily increase scope of work. I would skip it for
now if you don't mind. I didn't filed an issue, because I don't sure how
refactored collation support should look at whole. Maybe later.

> 
> > +			if (coll_id == NULL) {
> > +				free(parts);
> > +				luaL_error(L, "Unknown collation: \"%s\"",
> > +					   coll_name);
> > +				unreachable();
> > +				return NULL;
> > +			}
> > +			parts[key_parts_count].coll_id = coll_id->id;
> > +		}
> > +		lua_pop(L, 1);
> > +
> > +		/* Check coll_id. */
> > +		struct coll_id *coll_id =
> > +			coll_by_id(parts[key_parts_count].coll_id);
> > +		if (parts[key_parts_count].coll_id != COLL_NONE &&
> > +		    coll_id == NULL) {
> > +			uint32_t collation_id = parts[key_parts_count].coll_id;
> > +			free(parts);
> > +			luaL_error(L, "Unknown collation_id: %d", collation_id);
> > +			unreachable();
> > +			return NULL;
> > +		}
> > +
> > +		/* Set parts[key_parts_count].sort_order. */
> > +		parts[key_parts_count].sort_order = SORT_ORDER_ASC;
> > +
> > +		++key_parts_count;
> > +	}
> > +
> > +	struct key_def *key_def = key_def_new(parts, key_parts_count);
> > +	free(parts);
> > +	if (key_def == NULL) {
> > +		luaL_error(L, "Cannot create key_def");
> > +		unreachable();
> > +		return NULL;
> > +	}
> > +	return key_def;
> > +}

> > +#if defined(__cplusplus)
> > +extern "C" {
> > +#endif /* defined(__cplusplus) */
> > +
> > +struct key_def;
> > +struct lua_State;
> > +
> > +/** \cond public */
> > +
> > +/**
> > + * Create the new key_def from a Lua table.
> 
> a key_def

Fixed.

> > diff --git a/test/app-tap/module_api.c b/test/app-tap/module_api.c
> > index b81a98056..34ab54bc0 100644
> > --- a/test/app-tap/module_api.c
> > +++ b/test/app-tap/module_api.c
> > @@ -449,6 +449,18 @@ test_iscallable(lua_State *L)
> >  	return 1;
> >  }
> >  
> > +static int
> > +test_luaT_new_key_def(lua_State *L)
> > +{
> > +	/*
> > +	 * Ignore the return value. Here we test whether the
> > +	 * function raises an error.
> > +	 */
> > +	luaT_new_key_def(L, 1);
> 
> It would be nice to test that it actually creates a valid key_def.
> Testing error conditions is less important.

Now I can check a type of returned lua object in a successful case and
that is all. When we'll add compare functions (in the scope #3398) we
can test they are works as expected.

I have added type checks of results of some successful key_def.new()
invocations.

----

commit 7eae92c26421b99159892638c366a90f0d6af877
Author: Alexander Turenko <alexander.turenko@tarantool.org>
Date:   Mon Jan 7 19:12:50 2019 +0300

    lua: add key_def lua module
    
    There are two reasons to add this module:
    
    * Incapsulate key_def creation from a Lua table (factor it out from
      merger's code).
    * Support comparing tuple with tuple and/or tuple with key from Lua in
      the future.
    
    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.
    
    Needed for #3276.
    Needed for #3398.
    
    @TarantoolBot document
    Title: Document built-in key_def lua module
    
    Now there is only stub with the `key_def.new(parts)` function that
    returns cdata<struct key_def &>. The only way to use it for now is pass
    it to the merger.
    
    This module will be improved in the scope of
    https://github.com/tarantool/tarantool/issues/3398
    
    See the commit message for more info.

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 04de5ad04..494c8d391 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -202,6 +202,7 @@ set(api_headers
     ${CMAKE_SOURCE_DIR}/src/lua/error.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 5521e489e..0db093768 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -139,6 +139,7 @@ add_library(box STATIC
     lua/net_box.c
     lua/xlog.c
     lua/sql.c
+    lua/key_def.c
     ${bin_sources})
 
 target_link_libraries(box box_error tuple stat xrow xlog vclock crc32 scramble
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 0e90f6be5..885354ace 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -59,6 +59,7 @@
 #include "box/lua/console.h"
 #include "box/lua/tuple.h"
 #include "box/lua/sql.h"
+#include "box/lua/key_def.h"
 
 extern char session_lua[],
 	tuple_lua[],
@@ -312,6 +313,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..f372048a6
--- /dev/null
+++ b/src/box/lua/key_def.c
@@ -0,0 +1,226 @@
+/*
+ * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "box/lua/key_def.h"
+
+#include <lua.h>
+#include <lauxlib.h>
+#include "diag.h"
+#include "box/key_def.h"
+#include "box/box.h"
+#include "box/coll_id_cache.h"
+#include "lua/utils.h"
+#include "box/tuple_format.h" /* TUPLE_INDEX_BASE */
+
+static uint32_t key_def_type_id = 0;
+
+/**
+ * Set key_part_def from a table on top of a Lua stack.
+ *
+ * When successful return 0, otherwise return -1 and set a diag.
+ */
+static int
+luaT_key_def_set_part(struct lua_State *L, struct key_part_def *part)
+{
+	/* 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);
+	if (part->type == field_type_MAX) {
+		diag_set(IllegalParams, "Unknown field type: %s", type_name);
+		return -1;
+	}
+
+	/* Set part->is_nullable and part->nullable_action. */
+	lua_pushstring(L, "is_nullable");
+	lua_gettable(L, -2);
+	if (lua_isnil(L, -1)) {
+		part->is_nullable = false;
+		part->nullable_action = ON_CONFLICT_ACTION_DEFAULT;
+	} else {
+		part->is_nullable = lua_toboolean(L, -1);
+		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 = COLL_NONE;
+	else
+		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->sort_order. */
+	part->sort_order = SORT_ORDER_ASC;
+
+	return 0;
+}
+
+struct key_def *
+check_key_def(struct lua_State *L, int idx)
+{
+	if (lua_type(L, idx) != LUA_TCDATA)
+		return NULL;
+
+	uint32_t cdata_type;
+	struct key_def **key_def_ptr = luaL_checkcdata(L, idx, &cdata_type);
+	if (key_def_ptr == NULL || cdata_type != key_def_type_id)
+		return NULL;
+	return *key_def_ptr;
+}
+
+/**
+ * Free a key_def from a Lua code.
+ */
+static int
+lbox_key_def_gc(struct lua_State *L)
+{
+	struct key_def *key_def = check_key_def(L, 1);
+	if (key_def == NULL)
+		return 0;
+	box_key_def_delete(key_def);
+	return 0;
+}
+
+/**
+ * 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.
+ *
+ * Return the new key_def as cdata.
+ */
+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>]"
+				  "[, collation_id = <number>]"
+				  "[, collation = <string>]}, ...}");
+
+	uint32_t part_count = lua_objlen(L, 1);
+	const ssize_t parts_size = sizeof(struct key_part_def) * part_count;
+	struct key_part_def *parts = malloc(parts_size);
+	if (parts == NULL) {
+		diag_set(OutOfMemory, parts_size, "malloc", "parts");
+		return luaT_error(L);
+	}
+
+	for (uint32_t i = 0; i < part_count; ++i) {
+		lua_pushinteger(L, i + 1);
+		lua_gettable(L, 1);
+		if (luaT_key_def_set_part(L, &parts[i]) != 0) {
+			free(parts);
+			return luaT_error(L);
+		}
+	}
+
+	struct key_def *key_def = key_def_new(parts, part_count);
+	free(parts);
+	if (key_def == NULL)
+		return luaT_error(L);
+
+	*(struct key_def **) luaL_pushcdata(L, key_def_type_id) = key_def;
+	lua_pushcfunction(L, lbox_key_def_gc);
+	luaL_setcdatagc(L, -2);
+
+	return 1;
+}
+
+LUA_API int
+luaopen_key_def(struct lua_State *L)
+{
+	luaL_cdef(L, "struct key_def;");
+	key_def_type_id = luaL_ctypeid(L, "struct key_def&");
+
+	/* Export C functions to Lua. */
+	static const struct luaL_Reg meta[] = {
+		{"new", lbox_key_def_new},
+		{NULL, NULL}
+	};
+	luaL_register_module(L, "key_def", meta);
+
+	return 1;
+}
diff --git a/src/box/lua/key_def.h b/src/box/lua/key_def.h
new file mode 100644
index 000000000..11cc0bfd4
--- /dev/null
+++ b/src/box/lua/key_def.h
@@ -0,0 +1,56 @@
+#ifndef TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED
+#define TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED
+/*
+ * Copyright 2010-2018, 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;
+
+/**
+ * Extract a key_def object from a Lua stack.
+ */
+struct key_def *
+check_key_def(struct lua_State *L, int idx);
+
+/**
+ * Register the module.
+ */
+int
+luaopen_key_def(struct lua_State *L);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif /* defined(__cplusplus) */
+
+#endif /* TARANTOOL_BOX_LUA_KEY_DEF_H_INCLUDED */
diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua
new file mode 100755
index 000000000..7e6e0e330
--- /dev/null
+++ b/test/box-tap/key_def.test.lua
@@ -0,0 +1,137 @@
+#!/usr/bin/env tarantool
+
+local tap = require('tap')
+local ffi = require('ffi')
+local key_def = require('key_def')
+
+local usage_error = 'Bad params, use: key_def.new({' ..
+                    '{fieldno = fieldno, type = type' ..
+                    '[, is_nullable = <boolean>]' ..
+                    '[, 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 cases = {
+    -- Cases to call before box.cfg{}.
+    {
+        'Pass a field on an unknown type',
+        parts = {{
+            fieldno = 2,
+            type = 'unknown',
+        }},
+        exp_err = 'Unknown field type: unknown',
+    },
+    {
+        'Try to use collation_id before box.cfg{}',
+        parts = {{
+            fieldno = 1,
+            type = 'string',
+            collation_id = 2,
+        }},
+        exp_err = coll_not_found(1, 2),
+    },
+    {
+        'Try to use collation before box.cfg{}',
+        parts = {{
+            fieldno = 1,
+            type = 'string',
+            collation = 'unicode_ci',
+        }},
+        exp_err = coll_not_found(1, 'unicode_ci'),
+    },
+    function()
+        -- For collations.
+        box.cfg{}
+    end,
+    -- Cases to call after box.cfg{}.
+    {
+        'Try to use both collation_id and collation',
+        parts = {{
+            fieldno = 1,
+            type = 'string',
+            collation_id = 2,
+            collation = 'unicode_ci',
+        }},
+        exp_err = 'Conflicting options: collation_id and collation',
+    },
+    {
+        'Unknown collation_id',
+        parts = {{
+            fieldno = 1,
+            type = 'string',
+            collation_id = 42,
+        }},
+        exp_err = coll_not_found(1, 42),
+    },
+    {
+        'Unknown collation name',
+        parts = {{
+            fieldno = 1,
+            type = 'string',
+            collation = 'unknown',
+        }},
+        exp_err = 'Unknown collation: "unknown"',
+    },
+    {
+        'Bad parts parameter type',
+        parts = 1,
+        exp_err = usage_error,
+    },
+    {
+        'No parameters',
+        params = {},
+        exp_err = usage_error,
+    },
+    {
+        'Two parameters',
+        params = {{}, {}},
+        exp_err = usage_error,
+    },
+    {
+        'Success case; zero parts',
+        parts = {},
+        exp_err = nil,
+    },
+    {
+        'Success case; one part',
+        parts = {
+            fieldno = 1,
+            type = 'string',
+        },
+        exp_err = nil,
+    },
+}
+
+local test = tap.test('key_def')
+
+test:plan(#cases - 1)
+for _, case in ipairs(cases) do
+    if type(case) == 'function' then
+        case()
+    else
+        local ok, res
+        if case.params then
+            ok, res = pcall(key_def.new, unpack(case.params))
+        else
+            ok, res = pcall(key_def.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
+
+os.exit(test:check() and 0 or 1)

  reply	other threads:[~2019-01-29 18:52 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09 20:20 [PATCH v2 0/6] Merger Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 1/6] Add luaL_iscallable with support of cdata metatype Alexander Turenko
2019-01-10 12:21   ` Vladimir Davydov
2019-01-09 20:20 ` [PATCH v2 2/6] Add functions to ease using Lua iterators from C Alexander Turenko
2019-01-10 12:29   ` Vladimir Davydov
2019-01-15 23:26     ` Alexander Turenko
2019-01-16  8:18       ` Vladimir Davydov
2019-01-16 11:40         ` Alexander Turenko
2019-01-16 12:20           ` Vladimir Davydov
2019-01-17  1:20             ` Alexander Turenko
2019-01-28 18:17         ` Alexander Turenko
2019-03-01  4:04           ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 3/6] lua: add luaT_newtuple() Alexander Turenko
2019-01-10 12:44   ` Vladimir Davydov
2019-01-18 21:58     ` Alexander Turenko
2019-01-23 16:12       ` Vladimir Davydov
2019-01-28 16:51         ` Alexander Turenko
2019-03-01  4:08           ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 4/6] lua: add luaT_new_key_def() Alexander Turenko
2019-01-10 13:07   ` Vladimir Davydov
2019-01-29 18:52     ` Alexander Turenko [this message]
2019-01-30 10:58       ` Alexander Turenko
2019-03-01  4:10         ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 5/6] net.box: add helpers to decode msgpack headers Alexander Turenko
2019-01-10 17:29   ` Vladimir Davydov
2019-02-01 15:11     ` Alexander Turenko
2019-03-05 12:00       ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 6/6] Add merger for tuple streams Alexander Turenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190129185238.vx6bdzdb2ublyorz@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH v2 4/6] lua: add luaT_new_key_def()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox