Re: [tarantool-patches] [PATCH 0/3] lua: add key_def lua module

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Hi! Can't find the patch that we were discussed, so, I've copied It from branch by my own.

The code you implemented is very similar in many ways to the one already implemented in key_def.c.
I have one principal proposal and one advice(may be good enough):
1) at first, all map key names must follow their semantic-twin is used with create_index/alter;
    I mean field, not fieldno e.g.
    The errors also must not differ, I believe.
    https://tarantool.io/en/doc/1.10/book/box/box_space/#box-space-create-index

2) (proposal) I think you can do without the function load_key_def_set_part, that repeats]
    key_def_decode_parts code in many moments. You may encode tuple on region with
    lbox_encode_tuple_on_gc() and pass it to key_def_decode_parts(). While key_def:new is
    not performance-critical, this is better managed way to solve this problem, I think. Consider my
    RFC diff below:

diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
index 48d111b03..653e43817 100644
--- a/src/box/lua/key_def.c
+++ b/src/box/lua/key_def.c
@@ -38,117 +38,12 @@
 #include "box/box.h"
 #include "box/coll_id_cache.h"
 #include "lua/utils.h"
+#include "fiber.h"
+#include "box/lua/misc.h" /* lbox_encode_tuple_on_gc() */
 #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);
-	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)) {
-		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;
-
-	/* Set part->path. */
-	part->path = NULL;
-
-	return 0;
-}
-
 struct key_def *
 check_key_def(struct lua_State *L, int idx)
 {
@@ -194,33 +89,34 @@ lbox_key_def_new(struct lua_State *L)
 				  "[, collation_id = <number>]"
 				  "[, collation = <string>]}, ...}");
 
+	size_t tuple_len;
 	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);
+	const ssize_t part_def_size = sizeof(struct key_part_def) * part_count;
+	struct key_part_def *part_def = malloc(part_def_size);
+	if (part_def == NULL) {
+		diag_set(OutOfMemory, part_def_size, "malloc", "parts");
+		goto error;
 	}
-
-	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);
+	const char *parts = lbox_encode_tuple_on_gc(L, 1, &tuple_len);
+	if (parts == NULL)
+		goto error;
+	(void)mp_decode_array(&parts);
+	if (key_def_decode_parts(part_def, part_count, &parts,
+				 NULL, 0, &fiber()->gc) != 0)
+		goto error;
+
+	struct key_def *key_def = key_def_new(part_def, part_count);
 	if (key_def == NULL)
-		return luaT_error(L);
+		goto error;
 
 	*(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;
+error:
+	free(part_def);
+	return luaT_error(L);
 }
 
 LUA_API int
diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua
index 7e6e0e330..3e7366252 100755
--- a/test/box-tap/key_def.test.lua
+++ b/test/box-tap/key_def.test.lua
@@ -5,7 +5,7 @@ local ffi = require('ffi')
 local key_def = require('key_def')
 
 local usage_error = 'Bad params, use: key_def.new({' ..
-                    '{fieldno = fieldno, type = type' ..
+                    '{field = fieldno, type = type' ..
                     '[, is_nullable = <boolean>]' ..
                     '[, collation_id = <number>]' ..
                     '[, collation = <string>]}, ...}'
@@ -24,7 +24,7 @@ local cases = {
     {
         'Pass a field on an unknown type',
         parts = {{
-            fieldno = 2,
+            field = 2,
             type = 'unknown',
         }},
         exp_err = 'Unknown field type: unknown',
@@ -32,7 +32,7 @@ local cases = {
     {
         'Try to use collation_id before box.cfg{}',
         parts = {{
-            fieldno = 1,
+            field = 1,
             type = 'string',
             collation_id = 2,
         }},
@@ -41,7 +41,7 @@ local cases = {
     {
         'Try to use collation before box.cfg{}',
         parts = {{
-            fieldno = 1,
+            field = 1,
             type = 'string',
             collation = 'unicode_ci',
         }},
@@ -55,7 +55,7 @@ local cases = {
     {
         'Try to use both collation_id and collation',
         parts = {{
-            fieldno = 1,
+            field = 1,
             type = 'string',
             collation_id = 2,
             collation = 'unicode_ci',
@@ -65,7 +65,7 @@ local cases = {
     {
         'Unknown collation_id',
         parts = {{
-            fieldno = 1,
+            field = 1,
             type = 'string',
             collation_id = 42,
         }},
@@ -74,7 +74,7 @@ local cases = {
     {
         'Unknown collation name',
         parts = {{
-            fieldno = 1,
+            field = 1,
             type = 'string',
             collation = 'unknown',
         }},
@@ -103,8 +103,7 @@ local cases = {
     {
         'Success case; one part',
         parts = {
-            fieldno = 1,
-            type = 'string',
+            {field = 1, type = 'string'},
         },
         exp_err = nil,
     },

================================================================

Your original patch. Consider my 3 comments:

================================================================

> 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.
> ---
>  src/CMakeLists.txt            |   1 +
>  src/box/CMakeLists.txt        |   1 +
>  src/box/lua/init.c            |   3 +
>  src/box/lua/key_def.c         | 240 ++++++++++++++++++++++++++++++++++
>  src/box/lua/key_def.h         |  56 ++++++++
>  test/box-tap/key_def.test.lua | 137 +++++++++++++++++++
>  6 files changed, 438 insertions(+)
>  create mode 100644 src/box/lua/key_def.c
>  create mode 100644 src/box/lua/key_def.h
>  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 59e91b65a..f25c21045 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 744b2c895..69f346414 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..48d111b03
> --- /dev/null
> +++ b/src/box/lua/key_def.c
> @@ -0,0 +1,240 @@
> +/*
> + * 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);
> +	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)) {
> +		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;
> +
> +	/* Set part->path. */
> +	part->path = NULL;
> +
> +	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' ..

1. s/fieldno/field

> +                    '[, 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,

2. s/fieldno/field and so on

> +            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',

3. If you accept my proposal to reuse the existing code, you will have to do something
about the fact that the collation can now be set using the name of the pre-resolver
'collation_name' --> coll_id, which occurs in update_index_parts in schema.lua

Despite the non-zero complexity of the solution to this problem, I think this place is not
a problem in the context of this decision.

> +        }},
> +        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)
> -- 
> 2.21.0