[tarantool-patches] Re: [PATCH v2 1/1] schema:frommap() to create table or tuple
Kirill Shcherbatov
kshcherbatov at tarantool.org
Wed Apr 4 17:14:53 MSK 2018
On 03.04.2018 12:47, Vladislav Shpilevoy wrote:
> Hello. Please, consider 4 comments below.
>> diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
>> index 29a9aca..2ff9a11 100644
>> --- a/src/box/lua/space.cc
>> +++ b/src/box/lua/space.cc
>> @@ -32,11 +32,13 @@
>> #include "box/lua/tuple.h"
>> #include "lua/utils.h"
>> #include "lua/trigger.h"
>> +#include "box/schema.h"
>> extern "C" {
>> #include <lua.h>
>> #include <lauxlib.h>
>> #include <lualib.h>
>> + #include <trivia/util.h>
>> } /* extern "C" */
> 1. Schema.h is included below. Trivia/util.h is not needed here.
diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index 2ff9a11..9004c0e 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -38,7 +38,6 @@ extern "C" {
#include <lua.h>
#include <lauxlib.h>
#include <lualib.h>
- #include <trivia/util.h>
} /* extern "C" */
#include "box/space.h"
>
>> + space = space_by_id(id);
>> + if (!space)
>> + luaL_error(L, "Specified space is not exists");
>> +
> 2. Please, for pointers use explicit != NULL.
>
> 3. As I noted earlier, a function must not raise an error. It must
> return nil + error message/object.
> And please, check your comments grammar: you can not say "is not
> exists". Perhaps,
> you meant "does not exist".
>
@@ -423,8 +422,14 @@ lbox_space_ptr_cached(struct lua_State *L)
lua_getfield(L, 1, "id");
uint32_t id = luaL_touint64(L, -1);
space = space_by_id(id);
- if (!space)
- luaL_error(L, "Specified space is not exists");
+ if (space == NULL) {
+ const char *err_msg;
+ err_msg = tt_sprintf("Space with id '%d' doesn't exists",
+ id);
+ lua_pushnil(L);
+ lua_pushstring(L, err_msg);
+ return 2;
+ }
/** update the cache */
luaL_pushuint64(L, global_shema_version);
@@ -462,8 +467,14 @@ lbox_space_frommap(struct lua_State *L)
table = lua_toboolean(L, -1);
}
- lbox_space_ptr_cached(L);
- space = (struct space *)lua_topointer(L, -1);
+ if (lbox_space_ptr_cached(L) == 1) {
+ space = (struct space *) lua_topointer(L, -1);
+ } else {
+ const char *err_msg = lua_tostring(L, -1);
+ lua_pushnil(L);
+ lua_pushstring(L, err_msg);
+ return 2;
+ }
dict = space->format->dict;
lua_createtable(L, space->def->field_count, 0);
> 4. I do not see my crash test in your space_frommap.test.lua. When I
> provide you a test, or you
> found an own, you must add it to the patch.
>
There is the same test case here:
diff --git a/test/box/space_frommap.test.lua
b/test/box/space_frommap.test.lua
index ff7ef98..647d761 100644
--- a/test/box/space_frommap.test.lua
+++ b/test/box/space_frommap.test.lua
@@ -21,4 +21,7 @@ subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb =
4}, {dummy = true})
_ = subscriber:create_index('primary', {parts={'aaa'}})
tuple = subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
subscriber:replace(tuple)
+_ = box.internal.space._cptr(subscriber)
subscriber:drop()
+_ = subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
+_ = box.internal.space._cptr(subscriber)
More information about the Tarantool-patches
mailing list