[tarantool-patches] Re: [PATCH v2 1/1] schema:frommap() to create table or tuple
- From: Vladislav Shpilevoy <v.shpilevoy@xxxxxxxxxxxxx>
- To: tarantool-patches@xxxxxxxxxxxxx, Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
- Date: Tue, 3 Apr 2018 12:47:39 +0300
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.
+ 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".
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.
Other related posts:
