From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 14 May 2019 21:21:03 +0300 From: Konstantin Osipov Subject: Re: [tarantool-patches] [PATCH v1 2/3] schema: extend _func space to persist lua functions Message-ID: <20190514182103.GA3801@atlas> References: <0162c0c953de0f34121e5deb3ebe54aa6ed752fa.1557839195.git.kshcherbatov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0162c0c953de0f34121e5deb3ebe54aa6ed752fa.1557839195.git.kshcherbatov@tarantool.org> To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com, Kirill Shcherbatov List-ID: * Kirill Shcherbatov [19/05/14 16:31]: > This patch extends _func system space with a new string field > lua_code to persist Lua functions. The column name should be 'code'. There is a separate column 'language' that defines function language. > box.schema.func.create('sum', {lua_code = lua_code}) bikeshed, but I would call both the column and the option 'body', or 'text', not 'code'. While we are at it, I would also add 'type' - function or procedure, and other related to SQL procedures fields. See for example here: https://mariadb.com/kb/en/library/mysqlproc-table/ (not all of these fields are relevant, and perhaps we need an 'options' map, to make the whole thing easily extensible in the future, but it's better to put as many fields as possible into columns, not options). The next thing we'll add is SQL functions and procedures. It's really easy, trust me :) > + int deepcopy_ref = luaL_ref(L, LUA_REGISTRYINDEX); > + struct { > + const char *name; > + bool deep_copy; > + } env_exports[] = { > + {"assert", false}, {"error", false}, {"ipairs", false}, > + {"math", true}, {"next", false}, {"pairs", false}, > + {"pcall", false}, {"print", false}, {"select", false}, > + {"string", true}, {"table", true}, {"tonumber", false}, > + {"tostring", false}, {"type", false}, {"unpack", false}, > + {"xpcall", false}, {"utf8", false} > + }; > + lua_createtable(L, nelem(env_exports), 0); > + for (unsigned i = 0; i < nelem(env_exports); i++) { > + uint32_t name_len = strlen(env_exports[i].name); > + if (env_exports[i].deep_copy) > + lua_rawgeti(L, LUA_REGISTRYINDEX, deepcopy_ref); > + if (luaT_func_find(L, env_exports[i].name, > + env_exports[i].name + name_len, > + &count) != 0) { > + luaL_unref(L, LUA_REGISTRYINDEX, deepcopy_ref); > + goto end; > + } > + if (env_exports[i].deep_copy) > + lua_call(L, 1, LUA_MULTRET); > + lua_setfield(L, -2, env_exports[i].name); > + } > + lua_setfenv(L, -2); This belongs to an own function in util.c, luaT_prepare_sandbox(). It could perhaps be done entirely in Lua and in a more efficient way. Perhaps you could have a prepared sandbox which you could just clone. Please investigate. > +/** > + * Take over the modules that belonged to the old function. > + * Reset module and function pointers in onl function. > +*/ I don't understand this comment, please rephrase, and please do not forget to describe the usage and the purpose, not just what the function does. > +void > +func_capture_module(struct func *new_func, struct func *old_func); > +/** > + * Execute the persistent Lua function. > + * Assemble Lua object when required. > + */ > +static int > +execute_lua_ref_call(lua_State *L) What's a ref-call? Why the name of the function does not match the comment? > +{ > + struct call_request *request = (struct call_request *) > + lua_topointer(L, 1); > + lua_settop(L, 0); > + > + const char *name = request->name; > + uint32_t name_len = mp_decode_strl(&name); > + > + struct func *func = func_by_name(name, name_len); > + assert(func != NULL); > + /* Construct Lua function if required. */ > + assert(func->lua_func_ref != -1); > + lua_rawgeti(L, LUA_REGISTRYINDEX, func->lua_func_ref); > + > + /* Push the rest of args (a tuple). */ > + const char *args = request->args; > + uint32_t arg_count = mp_decode_array(&args); > + luaL_checkstack(L, arg_count, "lua_func: out of stack"); > + for (uint32_t i = 0; i < arg_count; i++) > + luamp_decode(L, luaL_msgpack_default, &args); > + > + lua_call(L, arg_count, LUA_MULTRET); > + return lua_gettop(L); > +} > + thanks! -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32