Re: [PATCH v2 4/6] lua: add luaT_new_key_def()

[Alexander Turenko <alexander.turenko@tarantool.org>]

Updated after recent rebase:

diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
index a77497384..48d111b03 100644
--- a/src/box/lua/key_def.c
+++ b/src/box/lua/key_def.c
@@ -143,6 +143,9 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *part)
 	/* Set part->sort_order. */
 	part->sort_order = SORT_ORDER_ASC;
 
+	/* Set part->path. */
+	part->path = NULL;
+
 	return 0;
 }

WBR, Alexander Turenko.

On Wed, Jan 30, 2019 at 01:58:33PM +0300, Alexander Turenko wrote:
> Updated a bit:
> 
> diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
> index f372048a6..a77497384 100644
> --- a/src/box/lua/key_def.c
> +++ b/src/box/lua/key_def.c
> @@ -75,9 +75,20 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *part)
>         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) {
> +       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. */
> 
> WBR, Alexander Turenko.
> 
> On Tue, Jan 29, 2019 at 09:52:38PM +0300, Alexander Turenko wrote:
> > 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)