Re: [tarantool-patches] [PATCH v1 2/3] schema: extend _func space to persist lua functions

[Konstantin Osipov <kostja@tarantool.org>]

* Kirill Shcherbatov <kshcherbatov@tarantool.org> [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