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

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

1. Deleted redundant lua_pop in  lbox_space_frommap
2. Included forgotten new frommap test case

diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index a8cfb14..42eaf79 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -434,7 +434,6 @@ lbox_space_ptr_cached(struct lua_State *L)
  static int
  lbox_space_frommap(struct lua_State *L)
  {
-	int res = 0;
  	struct tuple_dictionary *dict = nullptr;
  	struct space *space = nullptr;
  	int argc = lua_gettop(L);
@@ -450,9 +449,8 @@ lbox_space_frommap(struct lua_State *L)
  		tuple = lua_toboolean(L, -1);
  	}

-	res = lbox_space_ptr_cached(L);
+	lbox_space_ptr_cached(L);
  	space = (struct space *)lua_topointer(L, -1);
-	lua_pop(L, res);

  	dict = space->format->dict;
  	lua_createtable(L, space->def->field_count, 0);
diff --git a/test/box/space_frommap.test.lua 
b/test/box/space_frommap.test.lua
new file mode 100644
index 0000000..4760b3a
--- /dev/null
+++ b/test/box/space_frommap.test.lua
@@ -0,0 +1,20 @@
+-- box.tuple test
+env = require('test_run')
+test_run = env.new()
+test_run:cmd("push filter ".."'\\.lua.*:[0-9]+: ' to 
'.lua...\"]:<line>: '")
+
+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'}
+subscriber = box.schema.space.create('subscriber', {format = format})
+subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
+subscriber:frommap({ddd = 1, aaa = 2, bbb = 3})
+subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, eee = 4})
+subscriber:frommap()
+subscriber:frommap({})
+subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {tuple = true})
+subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {tuple = false})
+subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {dummy = true})
+subscriber:drop()


On 28.03.2018 21:28, Kirill Shcherbatov wrote:
> Closes #3282
> ---
>   src/box/lua/schema.lua        |   2 +
>   src/box/lua/space.cc          | 100 ++++++++++++++++++++++++++++++++++++++++++
>   src/box/lua/tuple.c           |   2 +-
>   src/box/lua/tuple.h           |   3 ++
>   test/box/space_frommap.result |  76 ++++++++++++++++++++++++++++++++
>   5 files changed, 182 insertions(+), 1 deletion(-)
>   create mode 100644 test/box/space_frommap.result
> 
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index 30c6bc6..740b885 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -1419,6 +1419,8 @@ function box.schema.space.bless(space)
>           end
>           builtin.space_run_triggers(s, yesno)
>       end
> +    space_mt.frommap = box.internal.space.frommap
> +    space_mt._ptr = box.internal.space._ptr
>       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..a8cfb14 100644
> --- a/src/box/lua/space.cc
> +++ b/src/box/lua/space.cc
> @@ -32,6 +32,7 @@
>   #include "box/lua/tuple.h"
>   #include "lua/utils.h"
>   #include "lua/trigger.h"
> +#include "box/schema.h"
>   
>   extern "C" {
>   	#include <lua.h>
> @@ -174,6 +175,16 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i)
>   	lua_pushstring(L, space->def->engine_name);
>   	lua_settable(L, i);
>   
> +	/* space.schema_version */
> +	lua_pushstring(L, "schema_version");
> +	luaL_pushuint64(L, box_schema_version());
> +	lua_settable(L, i);
> +
> +	/* space._space */
> +	lua_pushstring(L, "_space");
> +	lua_pushlightuserdata(L, space);
> +	lua_settable(L, i);
> +
>   	lua_pushstring(L, "enabled");
>   	lua_pushboolean(L, space_index(space, 0) != 0);
>   	lua_settable(L, i);
> @@ -386,6 +397,87 @@ static struct trigger on_alter_space_in_lua = {
>   	RLIST_LINK_INITIALIZER, box_lua_space_new_or_delete, NULL, NULL
>   };
>   
> +static int
> +lbox_space_ptr_cached(struct lua_State *L)
> +{
> +	if (lua_gettop(L) < 1 || !lua_istable(L, 1))
> +		luaL_error(L, "Usage: space:_ptr_cached()");
> +
> +	// take care about stack to call this function in frommap directly
> +	lua_getfield(L, 1, "schema_version");
> +	uint64_t schema_version = luaL_touint64(L, -1);
> +	lua_pop(L, 1);
> +	uint64_t global_shema_version = box_schema_version();
> +
> +	void *space = nullptr;
> +	if (schema_version == global_shema_version) {
> +		lua_getfield(L, 1, "_space");
> +		space = (void *)lua_topointer(L, -1);
> +		lua_pop(L, 1);
> +	} else {
> +		lua_getfield(L, 1, "id");
> +		uint32_t id = luaL_touint64(L, -1);
> +		lua_pop(L, 1);
> +		space = space_by_id(id);
> +
> +		luaL_pushuint64(L, global_shema_version);
> +		lua_setfield(L, 1, "schema_version");
> +
> +		lua_pushlightuserdata(L, space);
> +		lua_setfield(L, 1, "_space");
> +	}
> +
> +	lua_pushlightuserdata(L, space);
> +	return 1;
> +}
> +
> +static int
> +lbox_space_frommap(struct lua_State *L)
> +{
> +	int res = 0;
> +	struct tuple_dictionary *dict = nullptr;
> +	struct space *space = nullptr;
> +	int argc = lua_gettop(L);
> +	bool tuple = false;
> +	if (argc < 2 || argc > 3 || !lua_istable(L, 2))
> +		goto error;
> +	if (argc == 3) {
> +		if (!lua_istable(L, 3))
> +				goto error;
> +		lua_getfield(L, 3, "tuple");
> +		if (!lua_isboolean(L, -1) && !lua_isnil(L, -1))
> +				goto error;
> +		tuple = lua_toboolean(L, -1);
> +	}
> +
> +	res = lbox_space_ptr_cached(L);
> +	space = (struct space *)lua_topointer(L, -1);
> +	lua_pop(L, res);
> +
> +	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))
> +			luaL_error(L, "Unknown field '%s'", key);
> +		lua_rawseti(L, -3, fieldno+1);
> +	}
> +	if (!tuple)
> +		return 1;
> +
> +	lua_replace(L, 1);
> +	lua_settop(L, 1);
> +	return lbox_tuple_new(L);
> +error:
> +	luaL_error(L, "Usage: space:frommap(map, opts)");
> +	return 1;
> +}
> +
>   void
>   box_lua_space_init(struct lua_State *L)
>   {
> @@ -464,4 +556,12 @@ 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},
> +		{"_ptr", lbox_space_ptr_cached},
> +		{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/space_frommap.result b/test/box/space_frommap.result
> new file mode 100644
> index 0000000..884d78c
> --- /dev/null
> +++ b/test/box/space_frommap.result
> @@ -0,0 +1,76 @@
> +-- box.tuple test
> +env = require('test_run')
> +---
> +...
> +test_run = env.new()
> +---
> +...
> +test_run:cmd("push filter ".."'\\.lua.*:[0-9]+: ' to '.lua...\"]:<line>: '")
> +---
> +- true
> +...
> +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'}
> +---
> +...
> +subscriber = box.schema.space.create('subscriber', {format = format})
> +---
> +...
> +subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
> +---
> +- - 2
> +  - 4
> +  - 3
> +  - 1
> +...
> +subscriber:frommap({ddd = 1, aaa = 2, bbb = 3})
> +---
> +- - 2
> +  - 3
> +  - null
> +  - 1
> +...
> +subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, eee = 4})
> +---
> +- error: Unknown field 'eee'
> +...
> +subscriber:frommap()
> +---
> +- error: 'Usage: space:frommap(map, opts)'
> +...
> +subscriber:frommap({})
> +---
> +- []
> +...
> +subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {tuple = true})
> +---
> +- [2, 4, 3, 1]
> +...
> +subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {tuple = false})
> +---
> +- - 2
> +  - 4
> +  - 3
> +  - 1
> +...
> +subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {dummy = true})
> +---
> +- - 2
> +  - 4
> +  - 3
> +  - 1
> +...
> +subscriber:drop()
> +---
> +...
>