[tarantool-patches] Re: [PATCH v2 1/1] schema:frommap() to create table or tuple

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

LGTM. Vova, please, take a look.

On 09/04/2018 20:23, Kirill Shcherbatov wrote:
>  From 0cb37cac97056398c39f723aef81053cc4550485 Mon Sep 17 00:00:00 2001
> From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
> Date: Wed, 4 Apr 2018 19:32:40 +0300
> Subject: [PATCH] schema:frommap() to create table or tuple
> 
> This feature is designed to build tuple or table object by map.
> Tuple output is default. If you wish to make a table, specify table = true option.
> subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = true})
> 
> Closes #3282
> ---
>   src/box/lua/schema.lua  |  1 +
>   src/box/lua/space.cc    | 76 +++++++++++++++++++++++++++++++++++++++++++++-
>   src/box/lua/tuple.c     |  2 +-
>   src/box/lua/tuple.h     |  3 ++
>   test/box/tuple.result   | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
>   test/box/tuple.test.lua | 25 +++++++++++++++
>   6 files changed, 186 insertions(+), 2 deletions(-)
> 
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index 30c6bc6..323c2d6 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -1419,6 +1419,7 @@ function box.schema.space.bless(space)
>           end
>           builtin.space_run_triggers(s, yesno)
>       end
> +    space_mt.frommap = box.internal.space.frommap
>       space_mt.__index = space_mt
>   
>       setmetatable(space, space_mt)
> diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
> index 29a9aca..e358934 100644
> --- a/src/box/lua/space.cc
> +++ b/src/box/lua/space.cc
> @@ -178,7 +178,6 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i)
>   	lua_pushboolean(L, space_index(space, 0) != 0);
>   	lua_settable(L, i);
>   
> -
>           /* space:on_replace */
>           lua_pushstring(L, "on_replace");
>           lua_pushcfunction(L, lbox_space_on_replace);
> @@ -386,6 +385,74 @@ static struct trigger on_alter_space_in_lua = {
>   	RLIST_LINK_INITIALIZER, box_lua_space_new_or_delete, NULL, NULL
>   };
>   
> +/**
> + * Make a tuple or a table Lua object by map.
> + * @param Lua space object.
> + * @param Lua map table object.
> + * @param Lua opts table object (optional).
> + * @retval not nil A tuple or a table conforming to a space
> + *         format.
> + * @retval nil, err Can not built a tuple. A reason is returned in
> + *         the second value.
> + */
> +static int
> +lbox_space_frommap(struct lua_State *L)
> +{
> +	struct tuple_dictionary *dict = NULL;
> +	uint32_t id = 0;
> +	struct space *space = NULL;
> +	int argc = lua_gettop(L);
> +	bool table = false;
> +	if (argc < 2 || argc > 3 || !lua_istable(L, 2))
> +		goto usage_error;
> +	if (argc == 3) {
> +		if (!lua_istable(L, 3))
> +			goto usage_error;
> +		lua_getfield(L, 3, "table");
> +		if (!lua_isboolean(L, -1) && !lua_isnil(L, -1))
> +			goto usage_error;
> +		table = lua_toboolean(L, -1);
> +	}
> +
> +	lua_getfield(L, 1, "id");
> +	id = (int)lua_tointeger(L, -1);
> +	space = space_by_id(id);
> +	if (!space) {
> +		lua_pushnil(L);
> +		lua_pushstring(L, tt_sprintf("Space with id '%d' "\
> +					     "doesn't exist", id));
> +		return 2;
> +	}
> +	assert(space->format != NULL);
> +
> +	dict = space->format->dict;
> +	lua_createtable(L, space->def->field_count, 0);
> +
> +	lua_pushnil(L);
> +	while (lua_next(L, 2) != 0) {
> +		uint32_t fieldno;
> +		size_t key_len;
> +		const char *key = lua_tolstring(L, -2, &key_len);
> +		uint32_t key_hash = lua_hashstring(L, -2);
> +		if (tuple_fieldno_by_name(dict, key, key_len, key_hash,
> +					  &fieldno)) {
> +			lua_pushnil(L);
> +			lua_pushstring(L, tt_sprintf("Unknown field '%s'",
> +						     key));
> +			return 2;
> +		}
> +		lua_rawseti(L, -3, fieldno+1);
> +	}
> +	if (table)
> +		return 1;
> +
> +	lua_replace(L, 1);
> +	lua_settop(L, 1);
> +	return lbox_tuple_new(L);
> +usage_error:
> +	return luaL_error(L, "Usage: space:frommap(map, opts)");
> +}
> +
>   void
>   box_lua_space_init(struct lua_State *L)
>   {
> @@ -464,4 +531,11 @@ box_lua_space_init(struct lua_State *L)
>   	lua_pushnumber(L, VCLOCK_MAX);
>   	lua_setfield(L, -2, "REPLICA_MAX");
>   	lua_pop(L, 2); /* box, schema */
> +
> +	static const struct luaL_Reg space_internal_lib[] = {
> +		{"frommap", lbox_space_frommap},
> +		{NULL, NULL}
> +	};
> +	luaL_register(L, "box.internal.space", space_internal_lib);
> +	lua_pop(L, 1);
>   }
> diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
> index 7ca4299..92a7185 100644
> --- a/src/box/lua/tuple.c
> +++ b/src/box/lua/tuple.c
> @@ -90,7 +90,7 @@ luaT_istuple(struct lua_State *L, int narg)
>   	return *(struct tuple **) data;
>   }
>   
> -static int
> +int
>   lbox_tuple_new(lua_State *L)
>   {
>   	int argc = lua_gettop(L);
> diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h
> index 0965644..5d7062e 100644
> --- a/src/box/lua/tuple.h
> +++ b/src/box/lua/tuple.h
> @@ -66,6 +66,9 @@ luaT_istuple(struct lua_State *L, int idx);
>   
>   /** \endcond public */
>   
> +int
> +lbox_tuple_new(struct lua_State *L);
> +
>   static inline int
>   luaT_pushtupleornil(struct lua_State *L, struct tuple *tuple)
>   {
> diff --git a/test/box/tuple.result b/test/box/tuple.result
> index ec5ed90..e035cb9 100644
> --- a/test/box/tuple.result
> +++ b/test/box/tuple.result
> @@ -1060,6 +1060,87 @@ box.tuple.bsize == t.bsize
>   ---
>   - true
>   ...
> +--
> +-- gh-3282: space:frommap().
> +--
> +format = {}
> +---
> +...
> +format[1] = {name = 'aaa', type = 'unsigned'}
> +---
> +...
> +format[2] = {name = 'bbb', type = 'unsigned'}
> +---
> +...
> +format[3] = {name = 'ccc', type = 'unsigned'}
> +---
> +...
> +format[4] = {name = 'ddd', type = 'unsigned'}
> +---
> +...
> +s = box.schema.create_space('test', {format = format})
> +---
> +...
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
> +---
> +- [2, 4, 3, 1]
> +...
> +s:frommap({ddd = 1, aaa = 2, bbb = 3})
> +---
> +- [2, 3, null, 1]
> +...
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, eee = 4})
> +---
> +- null
> +- Unknown field 'eee'
> +...
> +s:frommap()
> +---
> +- error: 'Usage: space:frommap(map, opts)'
> +...
> +s:frommap({})
> +---
> +- []
> +...
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = true})
> +---
> +- - 2
> +  - 4
> +  - 3
> +  - 1
> +...
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = false})
> +---
> +- [2, 4, 3, 1]
> +...
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = box.NULL})
> +---
> +- [2, null, 3, 1]
> +...
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {dummy = true})
> +---
> +- [2, 4, 3, 1]
> +...
> +_ = s:create_index('primary', {parts = {'aaa'}})
> +---
> +...
> +tuple = s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
> +---
> +...
> +s:replace(tuple)
> +---
> +- [2, 4, 3, 1]
> +...
> +s:drop()
> +---
> +...
> +_, err = s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
> +---
> +...
> +assert(err ~= nil)
> +---
> +- true
> +...
>   test_run:cmd("clear filter")
>   ---
>   - true
> diff --git a/test/box/tuple.test.lua b/test/box/tuple.test.lua
> index 0b3392d..9df978d 100644
> --- a/test/box/tuple.test.lua
> +++ b/test/box/tuple.test.lua
> @@ -349,4 +349,29 @@ box.tuple.update == t.update
>   box.tuple.upsert == t.upsert
>   box.tuple.bsize == t.bsize
>   
> +--
> +-- gh-3282: space:frommap().
> +--
> +format = {}
> +format[1] = {name = 'aaa', type = 'unsigned'}
> +format[2] = {name = 'bbb', type = 'unsigned'}
> +format[3] = {name = 'ccc', type = 'unsigned'}
> +format[4] = {name = 'ddd', type = 'unsigned'}
> +s = box.schema.create_space('test', {format = format})
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
> +s:frommap({ddd = 1, aaa = 2, bbb = 3})
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, eee = 4})
> +s:frommap()
> +s:frommap({})
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = true})
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = false})
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = box.NULL})
> +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {dummy = true})
> +_ = s:create_index('primary', {parts = {'aaa'}})
> +tuple = s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
> +s:replace(tuple)
> +s:drop()
> +_, err = s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
> +assert(err ~= nil)
> +
>   test_run:cmd("clear filter")
>