<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">See below 10 comments.</div><br class=""><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">diff --git a/src/box/lua/</span><a href="http://space.cc" style="font-family: PTMono-Regular;" class="">space.cc</a><span style="font-family: PTMono-Regular;" class=""> b/src/box/lua/</span><a href="http://space.cc" style="font-family: PTMono-Regular;" class="">space.cc</a><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">index 29a9aca..a8cfb14 100644</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">--- a/src/box/lua/</span><a href="http://space.cc" style="font-family: PTMono-Regular;" class="">space.cc</a><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+++ b/src/box/lua/</span><a href="http://space.cc" style="font-family: PTMono-Regular;" class="">space.cc</a><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">@@ -32,6 +32,7 @@</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">#include "box/lua/tuple.h"</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">#include "lua/utils.h"</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">#include "lua/trigger.h"</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+#include "box/schema.h"</span><br style="font-family: PTMono-Regular;" class=""><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">extern "C" {</span><br style="font-family: PTMono-Regular;" class=""><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">#include <lua.h></span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">@@ -174,6 +175,16 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i)</span><br style="font-family: PTMono-Regular;" class=""><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_pushstring(L, space->def->engine_name);</span><br style="font-family: PTMono-Regular;" class=""><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_settable(L, i);</span><br style="font-family: PTMono-Regular;" class=""><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">/* space.schema_version */</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_pushstring(L, "schema_version");</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">luaL_pushuint64(L, box_schema_version());</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_settable(L, i);</span><br style="font-family: PTMono-Regular;" class=""></blockquote><div class=""><br class=""></div><div class="">1. Lets name it '_schema_version' - it is internal field.</div><br class=""><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">+</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">/* space._space */</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_pushstring(L, "_space");</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_pushlightuserdata(L, space);</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_settable(L, i);</span><br style="font-family: PTMono-Regular;" class=""></blockquote><div class=""><br class=""></div><div class="">2. Please, find a better name for the pointer. For example, 'ptr' - then it</div><div class="">will be accessed as space.ptr, that looks slightly better than space._space.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">+static int</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+lbox_space_ptr_cached(struct lua_State *L)</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+{</span></blockquote><br class=""></div><div class="">3. Add a comment, what this function does, why, and what arguments on a stack</div><div class="">it expects, what and home many values returns.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">// take care about stack to call this function in frommap directly</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_getfield(L, 1, "schema_version");</span></blockquote><br class=""></div><div class="">4. A comment seems to be not linked with the code below. And please, do not</div><div class="">use '//' comments. In our code we use '/* ... */' for comments inside a</div><div class="">function. Note, that comment max length is 66 symbols, including indentation</div><div class="">before a comment.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">+</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">void *space = nullptr;</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">if (schema_version == global_shema_version) {</span></blockquote><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">+</span></blockquote><br class=""></div><div class="">5. Do not use nullptr. Everywhere in Tarantool code only NULL is used.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">bool tuple = false;</span></blockquote><br class=""></div><div class="">6. Lets return tuple by default, and name the option 'table' instead of</div><div class="">tuple. If a user wants a table, then he must pass the option {table = true}.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">if (argc == 3) {</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">if (!lua_istable(L, 3))</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">goto error;</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_getfield(L, 3, "tuple");</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">if (!lua_isboolean(L, -1) && !lua_isnil(L, -1))</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">goto error;</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">tuple = lua_toboolean(L, -1);</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">}</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span></blockquote><br class=""></div><div class="">7. Double tabs prior to 'goto error'.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">if (tuple_fieldno_by_name(dict, key, key_len, key_hash, &fieldno))</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">luaL_error(L, "Unknown field '%s'", key);</span></blockquote></div><div class=""><br class=""></div><div class="">8. According to our Lua error returning convention, you can raise an error either</div><div class="">on incorrect usage (bad argument type, for example), or on OOM(Out Of Memory)</div><div class="">errors. On other errors you must return two values: nil and error object or</div><div class="">description. In this example you must return nil and <span style="font-family: PTMono-Regular;" class="">"Unknown field '%s'" string.</span></div><div class=""><span style="font-family: PTMono-Regular;" class=""><br class=""></span></div><div class=""><blockquote type="cite" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_replace(L, 1);</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">lua_settop(L, 1);</span><br style="font-family: PTMono-Regular;" class=""><span style="font-family: PTMono-Regular;" class="">+</span><span class="Apple-tab-span" style="font-family: PTMono-Regular; white-space: pre;"> </span><span style="font-family: PTMono-Regular;" class="">return lbox_tuple_new(L);</span></blockquote><br class=""></div><div class="">9. How about a comment what is happening here?</div><div class=""><br class=""></div><div class="">10. Add a test on nil and box.NULL fields. And test, that a result of</div><div class="">frommap() actually can be inserted or replaced into a space.</div><div><br class=""></div><br class=""></body></html>