Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH v1 0/3] box: persistent Lua functions
@ 2019-05-14 13:29 Kirill Shcherbatov
  2019-05-14 13:29 ` [PATCH v1 1/3] box: refactor box_lua_find helper Kirill Shcherbatov
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2019-05-14 13:29 UTC (permalink / raw)
  To: tarantool-patches, vdavydov.dev; +Cc: Kirill Shcherbatov

Lua functions managed by Tarantool are called 'persistent'.
They are stored in snapshoot and are available after server
restart. No regular Lua functions, nor C functions can't be
'persistent' because they depend on non-only snapshot environment
(libc, global variables etc.).
Some restrictions must be accounted writing such routines:
1. only limited set of Lua functions and modules are available:
-assert -error -pairs -ipairs -next -pcall -xpcall -type
-print -select -string -tonumber -tostring -unpack -math -utf8;
2. global variables are forbidden.

Persistent Lua functions are exported in box.schema.func.persistent
folder. They may be called via .call in exported object or via
netbox:call.

In scope of this patch, refactored func module API: in case of
persistent Lua function update may be insecure operation, so
we need a machinery to transactionally update the object.

http://github.com/tarantool/tarantool/tree/kshch/gh-4182-persistent-lua-functions
https://github.com/tarantool/tarantool/issues/4182

Kirill Shcherbatov (3):
  box: refactor box_lua_find helper
  schema: extend _func space to persist lua functions
  box: extend box.schema.func with persistent folder

 src/box/CMakeLists.txt            |   1 +
 src/box/alter.cc                  |  87 ++++++++----
 src/box/bootstrap.snap            | Bin 4374 -> 4393 bytes
 src/box/call.c                    |   2 +-
 src/box/func.c                    | 225 +++++++++++++++++++++++++++++-
 src/box/func.h                    |  21 ++-
 src/box/func_def.h                |  10 +-
 src/box/lua/call.c                | 122 +++++++---------
 src/box/lua/call.h                |   4 +-
 src/box/lua/init.c                |   2 +
 src/box/lua/schema.lua            |   8 +-
 src/box/lua/upgrade.lua           |  15 ++
 src/box/lua/util.c                | 102 ++++++++++++++
 src/box/lua/util.h                |  59 ++++++++
 src/box/schema.cc                 |  47 +++----
 src/box/schema.h                  |  31 ++--
 src/box/schema_def.h              |   1 +
 test/box-py/bootstrap.result      |   6 +-
 test/box/access_misc.result       |   4 +-
 test/box/persistent_func.result   | 185 ++++++++++++++++++++++++
 test/box/persistent_func.test.lua |  83 +++++++++++
 21 files changed, 858 insertions(+), 157 deletions(-)
 create mode 100644 src/box/lua/util.c
 create mode 100644 src/box/lua/util.h
 create mode 100644 test/box/persistent_func.result
 create mode 100644 test/box/persistent_func.test.lua

-- 
2.21.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 1/3] box: refactor box_lua_find helper
  2019-05-14 13:29 [PATCH v1 0/3] box: persistent Lua functions Kirill Shcherbatov
@ 2019-05-14 13:29 ` Kirill Shcherbatov
  2019-05-17 12:33   ` [tarantool-patches] " Kirill Shcherbatov
  2019-05-14 13:29 ` [PATCH v1 2/3] schema: extend _func space to persist lua functions Kirill Shcherbatov
  2019-05-14 13:29 ` [PATCH v1 3/3] box: extend box.schema.func with persistent folder Kirill Shcherbatov
  2 siblings, 1 reply; 9+ messages in thread
From: Kirill Shcherbatov @ 2019-05-14 13:29 UTC (permalink / raw)
  To: tarantool-patches, vdavydov.dev; +Cc: Kirill Shcherbatov

The box_lua_find routine used to work with an empty stack only.
It is unacceptable in following patches because this helper
need to be reused in following patches for environment table
construction.
The definition also refactored to be global in a new header
box/lua/utils.h.

Needed for #4182
---
 src/box/CMakeLists.txt |   1 +
 src/box/lua/call.c     |  82 +++++----------------------------
 src/box/lua/util.c     | 102 +++++++++++++++++++++++++++++++++++++++++
 src/box/lua/util.h     |  59 ++++++++++++++++++++++++
 4 files changed, 174 insertions(+), 70 deletions(-)
 create mode 100644 src/box/lua/util.c
 create mode 100644 src/box/lua/util.h

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 0864c3433..91cc486c1 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -125,6 +125,7 @@ add_library(box STATIC
     call.c
     merger.c
     ${lua_sources}
+    lua/util.c
     lua/init.c
     lua/call.c
     lua/lua_sql.c
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 04020ef6f..1a0fb4df0 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -39,78 +39,12 @@
 #include "box/xrow.h"
 #include "box/port.h"
 #include "box/lua/tuple.h"
+#include "box/lua/util.h"
 #include "small/obuf.h"
 #include "lua_sql.h"
 #include "trivia/util.h"
 #include "mpstream.h"
 
-/**
- * A helper to find a Lua function by name and put it
- * on top of the stack.
- */
-static int
-box_lua_find(lua_State *L, const char *name, const char *name_end)
-{
-	int index = LUA_GLOBALSINDEX;
-	int objstack = 0;
-	const char *start = name, *end;
-
-	while ((end = (const char *) memchr(start, '.', name_end - start))) {
-		lua_checkstack(L, 3);
-		lua_pushlstring(L, start, end - start);
-		lua_gettable(L, index);
-		if (! lua_istable(L, -1)) {
-			diag_set(ClientError, ER_NO_SUCH_PROC,
-				 name_end - name, name);
-			luaT_error(L);
-		}
-		start = end + 1; /* next piece of a.b.c */
-		index = lua_gettop(L); /* top of the stack */
-	}
-
-	/* box.something:method */
-	if ((end = (const char *) memchr(start, ':', name_end - start))) {
-		lua_checkstack(L, 3);
-		lua_pushlstring(L, start, end - start);
-		lua_gettable(L, index);
-		if (! (lua_istable(L, -1) ||
-			lua_islightuserdata(L, -1) || lua_isuserdata(L, -1) )) {
-				diag_set(ClientError, ER_NO_SUCH_PROC,
-					  name_end - name, name);
-				luaT_error(L);
-		}
-		start = end + 1; /* next piece of a.b.c */
-		index = lua_gettop(L); /* top of the stack */
-		objstack = index;
-	}
-
-
-	lua_pushlstring(L, start, name_end - start);
-	lua_gettable(L, index);
-	if (!lua_isfunction(L, -1) && !lua_istable(L, -1)) {
-		/* lua_call or lua_gettable would raise a type error
-		 * for us, but our own message is more verbose. */
-		diag_set(ClientError, ER_NO_SUCH_PROC,
-			  name_end - name, name);
-		luaT_error(L);
-	}
-	/* setting stack that it would contain only
-	 * the function pointer. */
-	if (index != LUA_GLOBALSINDEX) {
-		if (objstack == 0) {        /* no object, only a function */
-			lua_replace(L, 1);
-		} else if (objstack == 1) { /* just two values, swap them */
-			lua_insert(L, -2);
-		} else {		    /* long path */
-			lua_insert(L, 1);
-			lua_insert(L, 2);
-			objstack = 1;
-		}
-		lua_settop(L, 1 + objstack);
-	}
-	return 1 + objstack;
-}
-
 /**
  * A helper to find lua stored procedures for box.call.
  * box.call iteslf is pure Lua, to avoid issues
@@ -124,7 +58,10 @@ lbox_call_loadproc(struct lua_State *L)
 	const char *name;
 	size_t name_len;
 	name = lua_tolstring(L, 1, &name_len);
-	return box_lua_find(L, name, name + name_len);
+	int count;
+	if (luaT_func_find(L, name, name + name_len, &count) != 0)
+		return luaT_error(L);
+	return count;
 }
 
 /*
@@ -292,9 +229,14 @@ execute_lua_call(lua_State *L)
 	const char *name = request->name;
 	uint32_t name_len = mp_decode_strl(&name);
 
-	int oc = 0; /* how many objects are on stack after box_lua_find */
+	/*
+	 * How many objects are on stack after
+	 * luaT_func_find call.
+	 */
+	int oc = 0;
 	/* Try to find a function by name in Lua */
-	oc = box_lua_find(L, name, name + name_len);
+	if (luaT_func_find(L, name, name + name_len, &oc) != 0)
+		return luaT_error(L);
 
 	/* Push the rest of args (a tuple). */
 	const char *args = request->args;
diff --git a/src/box/lua/util.c b/src/box/lua/util.c
new file mode 100644
index 000000000..72bd9ca3f
--- /dev/null
+++ b/src/box/lua/util.c
@@ -0,0 +1,102 @@
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "box/lua/util.h"
+#include "box/error.h"
+#include "fiber.h"
+#include "lua/msgpack.h"
+
+int
+luaT_func_find(struct lua_State *L, const char *name, const char *name_end,
+	       int *count)
+{
+	int index = LUA_GLOBALSINDEX;
+	int objstack = 0, top = lua_gettop(L);
+	const char *start = name, *end;
+
+	while ((end = (const char *) memchr(start, '.', name_end - start))) {
+		lua_checkstack(L, 3);
+		lua_pushlstring(L, start, end - start);
+		lua_gettable(L, index);
+		if (! lua_istable(L, -1)) {
+			diag_set(ClientError, ER_NO_SUCH_PROC,
+				 name_end - name, name);
+			return -1;
+		}
+		start = end + 1; /* next piece of a.b.c */
+		index = lua_gettop(L); /* top of the stack */
+	}
+
+	/* box.something:method */
+	if ((end = (const char *) memchr(start, ':', name_end - start))) {
+		lua_checkstack(L, 3);
+		lua_pushlstring(L, start, end - start);
+		lua_gettable(L, index);
+		if (! (lua_istable(L, -1) ||
+			lua_islightuserdata(L, -1) || lua_isuserdata(L, -1) )) {
+				diag_set(ClientError, ER_NO_SUCH_PROC,
+					  name_end - name, name);
+				return -1;
+		}
+		start = end + 1; /* next piece of a.b.c */
+		index = lua_gettop(L); /* top of the stack */
+		objstack = index - top;
+	}
+
+	lua_pushlstring(L, start, name_end - start);
+	lua_gettable(L, index);
+	if (!lua_isfunction(L, -1) && !lua_istable(L, -1)) {
+		/* lua_call or lua_gettable would raise a type error
+		 * for us, but our own message is more verbose. */
+		diag_set(ClientError, ER_NO_SUCH_PROC,
+			  name_end - name, name);
+		return -1;
+	}
+
+	/* setting stack that it would contain only
+	 * the function pointer. */
+	if (index != LUA_GLOBALSINDEX) {
+		if (objstack == 0) {        /* no object, only a function */
+			lua_replace(L, top + 1);
+			lua_pop(L, lua_gettop(L) - top - 1);
+		} else if (objstack == 1) { /* just two values, swap them */
+			lua_insert(L, -2);
+			lua_pop(L, lua_gettop(L) - top - 2);
+		} else {                    /* long path */
+			lua_insert(L, top + 1);
+			lua_insert(L, top + 2);
+			lua_pop(L, objstack - 1);
+			objstack = 1;
+		}
+	}
+	*count = 1 + objstack;
+	return 0;
+}
diff --git a/src/box/lua/util.h b/src/box/lua/util.h
new file mode 100644
index 000000000..d8f29c544
--- /dev/null
+++ b/src/box/lua/util.h
@@ -0,0 +1,59 @@
+#ifndef INCLUDES_TARANTOOL_MOD_BOX_LUA_UTILS_H
+#define INCLUDES_TARANTOOL_MOD_BOX_LUA_UTILS_H
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <stddef.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+struct lua_State;
+
+/**
+ * A helper to find a Lua function by name and put it
+ * on top of the stack.
+ * Returns 0 in case of succsess and -1 otherwise; in case of
+ * success also uses count[out] argument to return the how many
+ * objects are pushed on to stack.
+ */
+int
+luaT_func_find(struct lua_State *L, const char *name, const char *name_end,
+	       int *count);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif /* defined(__cplusplus) */
+
+#endif /* INCLUDES_TARANTOOL_MOD_BOX_LUA_UTILS_H */
-- 
2.21.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 2/3] schema: extend _func space to persist lua functions
  2019-05-14 13:29 [PATCH v1 0/3] box: persistent Lua functions Kirill Shcherbatov
  2019-05-14 13:29 ` [PATCH v1 1/3] box: refactor box_lua_find helper Kirill Shcherbatov
@ 2019-05-14 13:29 ` Kirill Shcherbatov
  2019-05-14 18:21   ` [tarantool-patches] " Konstantin Osipov
  2019-05-14 13:29 ` [PATCH v1 3/3] box: extend box.schema.func with persistent folder Kirill Shcherbatov
  2 siblings, 1 reply; 9+ messages in thread
From: Kirill Shcherbatov @ 2019-05-14 13:29 UTC (permalink / raw)
  To: tarantool-patches, vdavydov.dev; +Cc: Kirill Shcherbatov

This patch extends _func system space with a new string field
lua_code to persist Lua functions.

Refactored func hash machinery to pass a new function object and
to return previous one. This allows to implement fault-tolerant
func_new call that constructs a lua function object.

Each persistent function may use limited amount of Lua functions
and modules:
-assert -error -pairs -ipairs -next -pcall -xpcall -type
-print -select -string -tonumber -tostring -unpack -math -utf8
Global variables are forbidden in persistent Lua functions.

This patch makes persistent Lua functions are available via
net.box.connect() :call method.
Example:
lua_code = [[function(a, b) return a + b end]]
box.schema.func.create('sum', {lua_code = lua_code})
conn:call("sum", {1, 3})

Part of #4182
Needed for #1260
---
 src/box/alter.cc                  |  85 +++++++++++-----
 src/box/bootstrap.snap            | Bin 4374 -> 4393 bytes
 src/box/call.c                    |   2 +-
 src/box/func.c                    | 109 ++++++++++++++++++--
 src/box/func.h                    |  15 ++-
 src/box/func_def.h                |  10 +-
 src/box/lua/call.c                |  40 +++++++-
 src/box/lua/call.h                |   4 +-
 src/box/lua/schema.lua            |   7 +-
 src/box/lua/upgrade.lua           |  15 +++
 src/box/schema.cc                 |  46 ++++-----
 src/box/schema.h                  |  15 +--
 src/box/schema_def.h              |   1 +
 test/box-py/bootstrap.result      |   6 +-
 test/box/access_misc.result       |   4 +-
 test/box/persistent_func.result   | 163 ++++++++++++++++++++++++++++++
 test/box/persistent_func.test.lua |  78 ++++++++++++++
 17 files changed, 517 insertions(+), 83 deletions(-)
 create mode 100644 test/box/persistent_func.result
 create mode 100644 test/box/persistent_func.test.lua

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 9279426d2..0d23dbf0e 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2402,21 +2402,41 @@ func_def_get_ids_from_tuple(struct tuple *tuple, uint32_t *fid, uint32_t *uid)
 static struct func_def *
 func_def_new_from_tuple(struct tuple *tuple)
 {
-	uint32_t len;
-	const char *name = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_NAME,
-					      &len);
-	if (len > BOX_NAME_MAX)
+	uint32_t name_len, lua_code_len;
+	const char *name, *lua_code;
+	name = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_NAME, &name_len);
+	if (name_len > BOX_NAME_MAX) {
 		tnt_raise(ClientError, ER_CREATE_FUNCTION,
 			  tt_cstr(name, BOX_INVALID_NAME_MAX),
 			  "function name is too long");
-	identifier_check_xc(name, len);
-	struct func_def *def = (struct func_def *) malloc(func_def_sizeof(len));
+	}
+	identifier_check_xc(name, name_len);
+	if (tuple_field_count(tuple) > BOX_FUNC_FIELD_LUA_CODE) {
+		lua_code = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_LUA_CODE,
+					      &lua_code_len);
+	} else {
+		lua_code = NULL;
+		lua_code_len = 0;
+	}
+
+	uint32_t def_sz = func_def_sizeof(name_len, lua_code_len);
+	struct func_def *def = (struct func_def *) malloc(def_sz);
 	if (def == NULL)
-		tnt_raise(OutOfMemory, func_def_sizeof(len), "malloc", "def");
+		tnt_raise(OutOfMemory, def_sz, "malloc", "def");
 	auto def_guard = make_scoped_guard([=] { free(def); });
 	func_def_get_ids_from_tuple(tuple, &def->fid, &def->uid);
-	memcpy(def->name, name, len);
-	def->name[len] = 0;
+
+	def->name = (char *)def + sizeof(struct func_def);
+	memcpy(def->name, name, name_len);
+	def->name[name_len] = 0;
+	if (lua_code_len > 0) {
+		def->lua_code = def->name + name_len + 1;
+		memcpy(def->lua_code, lua_code, lua_code_len);
+		def->lua_code[lua_code_len] = 0;
+	} else {
+		def->lua_code = NULL;
+	}
+
 	if (tuple_field_count(tuple) > BOX_FUNC_FIELD_SETUID)
 		def->setuid = tuple_field_u32_xc(tuple, BOX_FUNC_FIELD_SETUID);
 	else
@@ -2433,30 +2453,32 @@ func_def_new_from_tuple(struct tuple *tuple)
 		/* Lua is the default. */
 		def->language = FUNC_LANGUAGE_LUA;
 	}
+	if (def->language != FUNC_LANGUAGE_LUA && lua_code_len > 0) {
+		tnt_raise(ClientError, ER_CREATE_FUNCTION, name,
+			  "function lua_code may be specified only for "
+			  "Lua language");
+	}
 	def_guard.is_active = false;
 	return def;
 }
 
 /** Remove a function from function cache */
 static void
-func_cache_remove_func(struct trigger * /* trigger */, void *event)
+func_cache_remove_func(struct trigger *trigger, void * /* event */)
 {
-	struct txn_stmt *stmt = txn_last_stmt((struct txn *) event);
-	uint32_t fid = tuple_field_u32_xc(stmt->old_tuple ?
-				       stmt->old_tuple : stmt->new_tuple,
-				       BOX_FUNC_FIELD_ID);
-	func_cache_delete(fid);
+	struct func *old_func = (struct func *) trigger->data;
+	func_cache_delete(old_func->def->fid);
+	func_delete(old_func);
 }
 
 /** Replace a function in the function cache */
 static void
-func_cache_replace_func(struct trigger * /* trigger */, void *event)
+func_cache_replace_func(struct trigger *trigger, void * /* event */)
 {
-	struct txn_stmt *stmt = txn_last_stmt((struct txn*) event);
-	struct func_def *def = func_def_new_from_tuple(stmt->new_tuple);
-	auto def_guard = make_scoped_guard([=] { free(def); });
-	func_cache_replace(def);
-	def_guard.is_active = false;
+	struct func *new_func = (struct func *) trigger->data;
+	struct func *old_func;
+	func_cache_replace(new_func, &old_func);
+	func_delete(old_func);
 }
 
 /**
@@ -2477,13 +2499,20 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 	struct func *old_func = func_by_id(fid);
 	if (new_tuple != NULL && old_func == NULL) { /* INSERT */
 		struct func_def *def = func_def_new_from_tuple(new_tuple);
+		auto def_guard = make_scoped_guard([=] { free(def); });
 		access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
 				 PRIV_C);
-		auto def_guard = make_scoped_guard([=] { free(def); });
-		func_cache_replace(def);
+		struct func *func = func_new(def);
+		if (func == NULL)
+			diag_raise();
+		auto func_guard = make_scoped_guard([=] { func_delete(func); });
 		def_guard.is_active = false;
+		struct func *old_func = NULL;
+		func_cache_replace(func, &old_func);
+		assert(old_func == NULL);
+		func_guard.is_active = false;
 		struct trigger *on_rollback =
-			txn_alter_trigger_new(func_cache_remove_func, NULL);
+			txn_alter_trigger_new(func_cache_remove_func, func);
 		txn_on_rollback(txn, on_rollback);
 	} else if (new_tuple == NULL) {         /* DELETE */
 		uint32_t uid;
@@ -2501,15 +2530,19 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 				  "function has grants");
 		}
 		struct trigger *on_commit =
-			txn_alter_trigger_new(func_cache_remove_func, NULL);
+			txn_alter_trigger_new(func_cache_remove_func, old_func);
 		txn_on_commit(txn, on_commit);
 	} else {                                /* UPDATE, REPLACE */
 		struct func_def *def = func_def_new_from_tuple(new_tuple);
 		auto def_guard = make_scoped_guard([=] { free(def); });
 		access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
 				 PRIV_A);
+		struct func *func = func_new(def);
+		if (func == NULL)
+			diag_raise();
+		def_guard.is_active = false;
 		struct trigger *on_commit =
-			txn_alter_trigger_new(func_cache_replace_func, NULL);
+			txn_alter_trigger_new(func_cache_replace_func, func);
 		txn_on_commit(txn, on_commit);
 	}
 }

diff --git a/src/box/call.c b/src/box/call.c
index 56da53fb3..773b914b1 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -202,7 +202,7 @@ box_process_call(struct call_request *request, struct port *port)
 	if (func && func->def->language == FUNC_LANGUAGE_C) {
 		rc = box_c_call(func, request, port);
 	} else {
-		rc = box_lua_call(request, port);
+		rc = box_lua_call(func, request, port);
 	}
 	/* Restore the original user */
 	if (orig_credentials)
diff --git a/src/box/func.c b/src/box/func.c
index a817851fd..b68c089ad 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -29,11 +29,13 @@
  * SUCH DAMAGE.
  */
 #include "func.h"
+#include "box/lua/util.h"
 #include "trivia/config.h"
 #include "assoc.h"
 #include "lua/utils.h"
 #include "error.h"
 #include "diag.h"
+#include "fiber.h"
 #include <dlfcn.h>
 
 /**
@@ -355,6 +357,88 @@ restore:
 	return -1;
 }
 
+/**
+ * Assemble a Lua function object on Lua stack and return
+ * the reference.
+ * Returns func object reference on success, -1 otherwise.
+ */
+static int
+func_lua_code_load(struct func_def *def)
+{
+	int rc = -1;
+	struct region *region = &fiber()->gc;
+	size_t region_svp = region_used(region);
+	struct lua_State *L = lua_newthread(tarantool_L);
+	int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX);
+
+	const char *load_pref = "return ";
+	uint32_t load_str_sz =
+		strlen(load_pref) + strlen(def->lua_code) + 1;
+	char *load_str = region_alloc(region, load_str_sz);
+	if (load_str == NULL) {
+		diag_set(OutOfMemory, load_str_sz, "region",
+				"load_str");
+		goto end;
+	}
+	sprintf(load_str, "%s%s", load_pref, def->lua_code);
+	if (luaL_loadstring(L, load_str) != 0 ||
+	    lua_pcall(L, 0, 1, 0) != 0 || !lua_isfunction(L, -1)) {
+		diag_set(ClientError, ER_LOAD_FUNCTION, def->name,
+			def->lua_code);
+		goto end;
+	}
+	/*
+	 * Allow assembled function to use only a limited number
+	 * of functions and modules. All modules are exported
+	 * via deepcopy to prevent the user from overriding
+	 * the system context.
+	 *
+	 * The debug.getinfo() inspection for upvalues is
+	 * redundant, because no one can create function that
+	 * doesn't return function and that has such env_exports
+	 * restrictions.
+	 */
+	int count;
+	const char *deepcopy = "table.deepcopy";
+	if (luaT_func_find(L, deepcopy, deepcopy + strlen(deepcopy),
+			   &count) != 0)
+		goto end;
+	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);
+	luaL_unref(L, LUA_REGISTRYINDEX, deepcopy_ref);
+	rc = luaL_ref(L, LUA_REGISTRYINDEX);
+end:
+	region_truncate(region, region_svp);
+	luaL_unref(L, LUA_REGISTRYINDEX, coro_ref);
+	return rc;
+}
+
 struct func *
 func_new(struct func_def *def)
 {
@@ -380,6 +464,15 @@ func_new(struct func_def *def)
 	func->owner_credentials.auth_token = BOX_USER_MAX; /* invalid value */
 	func->func = NULL;
 	func->module = NULL;
+	if (func->def->lua_code != NULL) {
+		func->lua_func_ref = func_lua_code_load(def);
+		if (func->lua_func_ref == -1) {
+			free(func);
+			return NULL;
+		}
+	} else {
+		func->lua_func_ref = -1;
+	}
 	return func;
 }
 
@@ -395,8 +488,11 @@ func_unload(struct func *func)
 		}
 		module_gc(func->module);
 	}
+	if (func->lua_func_ref != -1)
+		luaL_unref(tarantool_L, LUA_REGISTRYINDEX, func->lua_func_ref);
 	func->module = NULL;
 	func->func = NULL;
+	func->lua_func_ref = -1;
 }
 
 /**
@@ -452,17 +548,18 @@ func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
 }
 
 void
-func_update(struct func *func, struct func_def *def)
+func_delete(struct func *func)
 {
 	func_unload(func);
 	free(func->def);
-	func->def = def;
+	free(func);
 }
 
 void
-func_delete(struct func *func)
+func_capture_module(struct func *new_func, struct func *old_func)
 {
-	func_unload(func);
-	free(func->def);
-	free(func);
+	new_func->module = old_func->module;
+	new_func->func = old_func->func;
+	old_func->module = NULL;
+	old_func->func = NULL;
 }
diff --git a/src/box/func.h b/src/box/func.h
index 8dcd61d7b..b00ed5690 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -65,6 +65,11 @@ struct func {
 	 * Anchor for module membership.
 	 */
 	struct rlist item;
+	/**
+	 * The reference index of Lua function object.
+	 * Is equal to -1 when undefined.
+	 */
+	int lua_func_ref;
 	/**
 	 * For C functions, the body of the function.
 	 */
@@ -100,9 +105,6 @@ module_free(void);
 struct func *
 func_new(struct func_def *def);
 
-void
-func_update(struct func *func, struct func_def *def);
-
 void
 func_delete(struct func *func);
 
@@ -113,6 +115,13 @@ int
 func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
 	  const char *args_end);
 
+/**
+ * Take over the modules that belonged to the old function.
+ * Reset module and function pointers in onl function.
+*/
+void
+func_capture_module(struct func *new_func, struct func *old_func);
+
 /**
  * Reload dynamically loadable module.
  *
diff --git a/src/box/func_def.h b/src/box/func_def.h
index 5b52ab498..19a4df959 100644
--- a/src/box/func_def.h
+++ b/src/box/func_def.h
@@ -54,6 +54,10 @@ struct func_def {
 	uint32_t fid;
 	/** Owner of the function. */
 	uint32_t uid;
+	/** Function name. */
+	char *name;
+	/** The Lua function code.  */
+	char *lua_code;
 	/**
 	 * True if the function requires change of user id before
 	 * invocation.
@@ -63,8 +67,6 @@ struct func_def {
 	 * The language of the stored function.
 	 */
 	enum func_language language;
-	/** Function name. */
-	char name[0];
 };
 
 /**
@@ -73,10 +75,10 @@ struct func_def {
  * for a function of length @a a name_len.
  */
 static inline size_t
-func_def_sizeof(uint32_t name_len)
+func_def_sizeof(uint32_t name_len, uint32_t lua_code_len)
 {
 	/* +1 for '\0' name terminating. */
-	return sizeof(struct func_def) + name_len + 1;
+	return sizeof(struct func_def) + name_len + 1 + lua_code_len + 1;
 }
 
 /**
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 1a0fb4df0..496c00e83 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -32,6 +32,8 @@
 #include "box/call.h"
 #include "box/error.h"
 #include "fiber.h"
+#include "box/func.h"
+#include "box/schema.h"
 
 #include "lua/utils.h"
 #include "lua/msgpack.h"
@@ -250,6 +252,37 @@ execute_lua_call(lua_State *L)
 	return lua_gettop(L);
 }
 
+/**
+ * Execute the persistent Lua function.
+ * Assemble Lua object when required.
+ */
+static int
+execute_lua_ref_call(lua_State *L)
+{
+	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);
+}
+
 static int
 execute_lua_eval(lua_State *L)
 {
@@ -393,9 +426,12 @@ box_process_lua(struct call_request *request, struct port *base,
 }
 
 int
-box_lua_call(struct call_request *request, struct port *port)
+box_lua_call(struct func *func, struct call_request *request, struct port *port)
 {
-	return box_process_lua(request, port, execute_lua_call);
+	if (func != NULL && func->def->lua_code != NULL)
+		return box_process_lua(request, port, execute_lua_ref_call);
+	else
+		return box_process_lua(request, port, execute_lua_call);
 }
 
 int
diff --git a/src/box/lua/call.h b/src/box/lua/call.h
index 0542123da..d8ef65900 100644
--- a/src/box/lua/call.h
+++ b/src/box/lua/call.h
@@ -42,13 +42,15 @@ box_lua_call_init(struct lua_State *L);
 
 struct port;
 struct call_request;
+struct func;
 
 /**
  * Invoke a Lua stored procedure from the binary protocol
  * (implementation of 'CALL' command code).
  */
 int
-box_lua_call(struct call_request *request, struct port *port);
+box_lua_call(struct func *func, struct call_request *request,
+	     struct port *port);
 
 int
 box_lua_eval(struct call_request *request, struct port *port);
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index f31cf7f2c..ca7da6501 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1981,7 +1981,8 @@ box.schema.func.create = function(name, opts)
     opts = opts or {}
     check_param_table(opts, { setuid = 'boolean',
                               if_not_exists = 'boolean',
-                              language = 'string'})
+                              language = 'string',
+                              lua_code = 'string' })
     local _func = box.space[box.schema.FUNC_ID]
     local _vfunc = box.space[box.schema.VFUNC_ID]
     local func = _vfunc.index.name:get{name}
@@ -1991,10 +1992,10 @@ box.schema.func.create = function(name, opts)
         end
         return
     end
-    opts = update_param_table(opts, { setuid = false, language = 'lua'})
+    opts = update_param_table(opts, { setuid = false, language = 'lua', lua_code = ''})
     opts.language = string.upper(opts.language)
     opts.setuid = opts.setuid and 1 or 0
-    _func:auto_increment{session.euid(), name, opts.setuid, opts.language}
+    _func:auto_increment{session.euid(), name, opts.setuid, opts.language, opts.lua_code}
 end
 
 box.schema.func.drop = function(name, opts)
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 89d6e3d52..c5296fc69 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -737,6 +737,20 @@ local function upgrade_to_2_1_3()
     end
 end
 
+local function upgrade_to_2_2_0()
+    local _func = box.space[box.schema.FUNC_ID]
+    local format = {}
+    format[1] = {name='id', type='unsigned'}
+    format[2] = {name='owner', type='unsigned'}
+    format[3] = {name='name', type='string'}
+    format[4] = {name='setuid', type='unsigned'}
+    format[5] = {name='lua_code', type='string'}
+    for _, v in box.space._func:pairs() do
+        box.space._func:replace(v:update{{'=', #v + 1, ''}})
+    end
+    _func:format(format)
+end
+
 local function get_version()
     local version = box.space._schema:get{'version'}
     if version == nil then
@@ -768,6 +782,7 @@ local function upgrade(options)
         {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
         {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
         {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
+        {version = mkversion(2, 2, 0), func = upgrade_to_2_2_0, auto = true},
     }
 
     for _, handler in ipairs(handlers) do
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 9a55c2f14..aae92874c 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -533,40 +533,35 @@ schema_free(void)
 }
 
 void
-func_cache_replace(struct func_def *def)
+func_cache_replace(struct func *new_func, struct func **old_func)
 {
-	struct func *old = func_by_id(def->fid);
-	if (old) {
-		func_update(old, def);
-		return;
-	}
 	if (mh_size(funcs) >= BOX_FUNCTION_MAX)
 		tnt_raise(ClientError, ER_FUNCTION_MAX, BOX_FUNCTION_MAX);
-	struct func *func = func_new(def);
-	if (func == NULL) {
-error:
-		panic_syserror("Out of memory for the data "
-			       "dictionary cache (stored function).");
-	}
-	const struct mh_i32ptr_node_t node = { def->fid, func };
-	mh_int_t k1 = mh_i32ptr_put(funcs, &node, NULL, NULL);
-	if (k1 == mh_end(funcs)) {
-		func->def = NULL;
-		func_delete(func);
-		goto error;
-	}
-	size_t def_name_len = strlen(func->def->name);
-	uint32_t name_hash = mh_strn_hash(func->def->name, def_name_len);
+
+	mh_int_t k1, k2;
+	struct mh_i32ptr_node_t *old_node = NULL;
+	const struct mh_i32ptr_node_t node = { new_func->def->fid, new_func };
+	size_t def_name_len = strlen(new_func->def->name);
+	uint32_t name_hash = mh_strn_hash(new_func->def->name, def_name_len);
 	const struct mh_strnptr_node_t strnode = {
-		func->def->name, def_name_len, name_hash, func };
+		new_func->def->name, def_name_len, name_hash, new_func };
 
-	mh_int_t k2 = mh_strnptr_put(funcs_by_name, &strnode, NULL, NULL);
+	k1 = mh_i32ptr_put(funcs, &node, &old_node, NULL);
+	if (k1 == mh_end(funcs))
+		goto error;
+	if (old_node != NULL)
+		*old_func = (struct func *)old_node->val;
+	k2 = mh_strnptr_put(funcs_by_name, &strnode, NULL, NULL);
 	if (k2 == mh_end(funcs_by_name)) {
 		mh_i32ptr_del(funcs, k1, NULL);
-		func->def = NULL;
-		func_delete(func);
 		goto error;
 	}
+	if (*old_func != NULL)
+		func_capture_module(new_func, *old_func);
+	return;
+error:
+	panic_syserror("Out of memory for the data dictionary cache "
+		       "(stored function).");
 }
 
 void
@@ -582,7 +577,6 @@ func_cache_delete(uint32_t fid)
 				strlen(func->def->name));
 	if (k != mh_end(funcs))
 		mh_strnptr_del(funcs_by_name, k, NULL);
-	func_delete(func);
 }
 
 struct func *
diff --git a/src/box/schema.h b/src/box/schema.h
index 6f9a96117..bc15c8a2c 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -163,14 +163,17 @@ schema_free();
 struct space *schema_space(uint32_t id);
 
 /**
- * Insert a new function or update the old one.
- *
- * @param def Function definition. In a case of success the ownership
- *        of @a def is transfered to the data dictionary, thus the caller
- *        must not delete it.
+ * Replace an existent (if any) function or insert a new one
+ * in function cache.
+ * @param func_new Function object to insert. In case of success,
+ *                 when old function object is exists, all loaded
+ *                 modules data are inherent with
+ *                 func_capture_module.
+ * @param func_old[out] The replaced function object if any.
+ * @retval Returns 0 on success, -1 otherwise.
  */
 void
-func_cache_replace(struct func_def *def);
+func_cache_replace(struct func *new_func, struct func **old_func);
 
 void
 func_cache_delete(uint32_t fid);
diff --git a/src/box/schema_def.h b/src/box/schema_def.h
index eeeeb950b..31d2a594a 100644
--- a/src/box/schema_def.h
+++ b/src/box/schema_def.h
@@ -163,6 +163,7 @@ enum {
 	BOX_FUNC_FIELD_NAME = 2,
 	BOX_FUNC_FIELD_SETUID = 3,
 	BOX_FUNC_FIELD_LANGUAGE = 4,
+	BOX_FUNC_FIELD_LUA_CODE = 5,
 };
 
 /** _collation fields. */
diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
index 379f6c51f..ea8ae0984 100644
--- a/test/box-py/bootstrap.result
+++ b/test/box-py/bootstrap.result
@@ -4,7 +4,7 @@ box.internal.bootstrap()
 box.space._schema:select{}
 ---
 - - ['max_id', 511]
-  - ['version', 2, 1, 3]
+  - ['version', 2, 2, 0]
 ...
 box.space._cluster:select{}
 ---
@@ -49,7 +49,7 @@ box.space._space:select{}
         'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]]
   - [296, 1, '_func', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
         'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
-        'type': 'unsigned'}]]
+        'type': 'unsigned'}, {'name': 'lua_code', 'type': 'string'}]]
   - [297, 1, '_vfunc', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
         'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
         'type': 'unsigned'}]]
@@ -141,7 +141,7 @@ box.space._user:select{}
 ...
 box.space._func:select{}
 ---
-- - [1, 1, 'box.schema.user.info', 1, 'LUA']
+- - [1, 1, 'box.schema.user.info', 1, 'LUA', '']
 ...
 box.space._priv:select{}
 ---
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 36ebfae09..e1dd9b1cb 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -789,7 +789,7 @@ box.space._space:select()
         'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]]
   - [296, 1, '_func', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
         'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
-        'type': 'unsigned'}]]
+        'type': 'unsigned'}, {'name': 'lua_code', 'type': 'string'}]]
   - [297, 1, '_vfunc', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
         'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
         'type': 'unsigned'}]]
@@ -821,7 +821,7 @@ box.space._space:select()
 ...
 box.space._func:select()
 ---
-- - [1, 1, 'box.schema.user.info', 1, 'LUA']
+- - [1, 1, 'box.schema.user.info', 1, 'LUA', '']
 ...
 session = nil
 ---
diff --git a/test/box/persistent_func.result b/test/box/persistent_func.result
new file mode 100644
index 000000000..ba544394e
--- /dev/null
+++ b/test/box/persistent_func.result
@@ -0,0 +1,163 @@
+env = require('test_run')
+---
+...
+test_run = env.new()
+---
+...
+--
+-- gh-4182: Add persistent LUA functions.
+--
+box.schema.user.grant('guest', 'execute', 'universe')
+---
+...
+net = require('net.box')
+---
+...
+conn = net.connect(box.cfg.listen)
+---
+...
+-- Test valid function.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+lua_code = [[function(tuple)
+	if type(tuple.address) ~= 'string' then return nil, 'Invalid field type' end
+	local t = tuple.address:lower():split()
+	for k,v in pairs(t) do t[k] = {v} end
+	return t
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('test', {lua_code = lua_code, language = "C"})
+---
+- error: 'Failed to create function ''test'': function lua_code may be specified only
+    for Lua language'
+...
+box.schema.func.create('test', {lua_code = lua_code})
+---
+...
+box.schema.func.exists('test')
+---
+- true
+...
+conn:call("test", {{address = "Moscow Dolgoprudny"}})
+---
+- [['moscow'], ['dolgoprudny']]
+...
+-- Test that monkey-patch attack is not possible.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+lua_code_monkey = [[function(tuple)
+	math.abs = math.log
+	return tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('lua_code_monkey', {lua_code = lua_code_monkey})
+---
+...
+conn:call("lua_code_monkey", {{address = "Moscow Dolgoprudny"}})
+---
+- {'address': 'Moscow Dolgoprudny'}
+...
+math.abs(-666.666)
+---
+- 666.666
+...
+-- Test taht 'require' is forbidden.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+lua_code_bad1 = [[function(tuple)
+	local json = require('json')
+	return json.encode(tuple)
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('json_serializer', {lua_code = lua_code_bad1})
+---
+...
+conn:call("json_serializer", {{address = "Moscow Dolgoprudny"}})
+---
+- error: '[string "return function(tuple) ..."]:1: attempt to call global ''require''
+    (a nil value)'
+...
+-- Test function with spell error - case 1.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+lua_code_bad2 = [[function(tuple)
+	ret tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('lua_code_bad2', {lua_code = lua_code_bad2})
+---
+- error: "Failed to dynamically load function 'lua_code_bad2': function(tuple) \tret
+    tuple end "
+...
+-- Test function with spell error - case 2.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+lua_code_bad3 = [[func(tuple)
+	return tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('lua_code_bad3', {lua_code = lua_code_bad3})
+---
+- error: "Failed to dynamically load function 'lua_code_bad3': func(tuple) \treturn
+    tuple end "
+...
+conn:call("lua_code_bad3", {{address = "Moscow Dolgoprudny"}})
+---
+- error: Procedure 'lua_code_bad3' is not defined
+...
+conn:close()
+---
+...
+-- Restart server.
+test_run:cmd("restart server default")
+net = require('net.box')
+---
+...
+test_run = require('test_run').new()
+---
+...
+conn = net.connect(box.cfg.listen)
+---
+...
+conn:call("test", {{address = "Moscow Dolgoprudny"}})
+---
+- [['moscow'], ['dolgoprudny']]
+...
+conn:close()
+---
+...
+box.schema.func.drop('test')
+---
+...
+box.schema.func.exists('test')
+---
+- false
+...
+box.schema.user.revoke('guest', 'execute', 'universe')
+---
+...
diff --git a/test/box/persistent_func.test.lua b/test/box/persistent_func.test.lua
new file mode 100644
index 000000000..67a315197
--- /dev/null
+++ b/test/box/persistent_func.test.lua
@@ -0,0 +1,78 @@
+env = require('test_run')
+test_run = env.new()
+
+--
+-- gh-4182: Add persistent LUA functions.
+--
+box.schema.user.grant('guest', 'execute', 'universe')
+net = require('net.box')
+conn = net.connect(box.cfg.listen)
+
+-- Test valid function.
+test_run:cmd("setopt delimiter ';'")
+lua_code = [[function(tuple)
+	if type(tuple.address) ~= 'string' then return nil, 'Invalid field type' end
+	local t = tuple.address:lower():split()
+	for k,v in pairs(t) do t[k] = {v} end
+	return t
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('test', {lua_code = lua_code, language = "C"})
+box.schema.func.create('test', {lua_code = lua_code})
+box.schema.func.exists('test')
+conn:call("test", {{address = "Moscow Dolgoprudny"}})
+
+-- Test that monkey-patch attack is not possible.
+test_run:cmd("setopt delimiter ';'")
+lua_code_monkey = [[function(tuple)
+	math.abs = math.log
+	return tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('lua_code_monkey', {lua_code = lua_code_monkey})
+conn:call("lua_code_monkey", {{address = "Moscow Dolgoprudny"}})
+math.abs(-666.666)
+
+-- Test taht 'require' is forbidden.
+test_run:cmd("setopt delimiter ';'")
+lua_code_bad1 = [[function(tuple)
+	local json = require('json')
+	return json.encode(tuple)
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('json_serializer', {lua_code = lua_code_bad1})
+conn:call("json_serializer", {{address = "Moscow Dolgoprudny"}})
+
+-- Test function with spell error - case 1.
+test_run:cmd("setopt delimiter ';'")
+lua_code_bad2 = [[function(tuple)
+	ret tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('lua_code_bad2', {lua_code = lua_code_bad2})
+
+-- Test function with spell error - case 2.
+test_run:cmd("setopt delimiter ';'")
+lua_code_bad3 = [[func(tuple)
+	return tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('lua_code_bad3', {lua_code = lua_code_bad3})
+conn:call("lua_code_bad3", {{address = "Moscow Dolgoprudny"}})
+
+conn:close()
+-- Restart server.
+test_run:cmd("restart server default")
+net = require('net.box')
+test_run = require('test_run').new()
+conn = net.connect(box.cfg.listen)
+conn:call("test", {{address = "Moscow Dolgoprudny"}})
+conn:close()
+box.schema.func.drop('test')
+box.schema.func.exists('test')
+box.schema.user.revoke('guest', 'execute', 'universe')
-- 
2.21.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 3/3] box: extend box.schema.func with persistent folder
  2019-05-14 13:29 [PATCH v1 0/3] box: persistent Lua functions Kirill Shcherbatov
  2019-05-14 13:29 ` [PATCH v1 1/3] box: refactor box_lua_find helper Kirill Shcherbatov
  2019-05-14 13:29 ` [PATCH v1 2/3] schema: extend _func space to persist lua functions Kirill Shcherbatov
@ 2019-05-14 13:29 ` Kirill Shcherbatov
  2019-05-14 18:24   ` [tarantool-patches] " Konstantin Osipov
  2 siblings, 1 reply; 9+ messages in thread
From: Kirill Shcherbatov @ 2019-05-14 13:29 UTC (permalink / raw)
  To: tarantool-patches, vdavydov.dev; +Cc: Kirill Shcherbatov

Now all persistent Lua functions are available with
box.schema.func.persistent table.

@TarantoolBot document
Title: Persistent Lua functions in core

Lua functions managed by Tarantool are called 'persistent'.
They are stored in snapshoot and are available after server
restart.
Persistent Lua functions are exported in box.schema.func.persistent
folder. They may be called via .call exported object method or
via netbox:call.

Some restrictions must be accounted writing such routines:
1. only limited set of Lua functions and modules are available:
-assert -error -pairs -ipairs -next -pcall -xpcall -type
-print -select -string -tonumber -tostring -unpack -math -utf8;
2. global variables are forbidden.

Example:
lua_code = [[function(a, b) return a + b end]]
box.schema.func.create('sum', {lua_code = lua_code})
box.schema.func.persistent.sum
---
- call: 'function: 0x4014b7f8'
  name: sum
  id: 2
...
box.schema.func.persistent.sum.call(1, 2)
---
- 3
...

Closes #4182
Needed for #1260
---
 src/box/alter.cc                  |   6 +-
 src/box/func.c                    | 116 ++++++++++++++++++++++++++++++
 src/box/func.h                    |   6 ++
 src/box/lua/init.c                |   2 +
 src/box/lua/schema.lua            |   1 +
 src/box/schema.cc                 |   1 +
 src/box/schema.h                  |  16 +++--
 test/box/persistent_func.result   |  22 ++++++
 test/box/persistent_func.test.lua |   5 ++
 9 files changed, 169 insertions(+), 6 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 0d23dbf0e..ee944163c 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2467,7 +2467,9 @@ static void
 func_cache_remove_func(struct trigger *trigger, void * /* event */)
 {
 	struct func *old_func = (struct func *) trigger->data;
-	func_cache_delete(old_func->def->fid);
+	uint32_t fid = old_func->def->fid;
+	func_cache_delete(fid);
+	trigger_run_xc(&on_alter_func, (void *)(uintptr_t)fid);
 	func_delete(old_func);
 }
 
@@ -2479,6 +2481,7 @@ func_cache_replace_func(struct trigger *trigger, void * /* event */)
 	struct func *old_func;
 	func_cache_replace(new_func, &old_func);
 	func_delete(old_func);
+	trigger_run_xc(&on_alter_func, (void *)(uintptr_t)new_func->def->fid);
 }
 
 /**
@@ -2511,6 +2514,7 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 		func_cache_replace(func, &old_func);
 		assert(old_func == NULL);
 		func_guard.is_active = false;
+		trigger_run_xc(&on_alter_func, (void *)(uintptr_t)fid);
 		struct trigger *on_rollback =
 			txn_alter_trigger_new(func_cache_remove_func, func);
 		txn_on_rollback(txn, on_rollback);
diff --git a/src/box/func.c b/src/box/func.c
index b68c089ad..ee9edebb1 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -32,10 +32,12 @@
 #include "box/lua/util.h"
 #include "trivia/config.h"
 #include "assoc.h"
+#include "lua/trigger.h"
 #include "lua/utils.h"
 #include "error.h"
 #include "diag.h"
 #include "fiber.h"
+#include "schema.h"
 #include <dlfcn.h>
 
 /**
@@ -563,3 +565,117 @@ func_capture_module(struct func *new_func, struct func *old_func)
 	old_func->module = NULL;
 	old_func->func = NULL;
 }
+
+static void
+box_lua_func_new(struct lua_State *L, struct func *func)
+{
+	lua_getfield(L, LUA_GLOBALSINDEX, "box");
+	lua_getfield(L, -1, "schema");
+	if (!lua_istable(L, -1)) {
+		lua_pop(L, 1); /* pop nil */
+		lua_newtable(L);
+		lua_setfield(L, -2, "schema");
+		lua_getfield(L, -1, "schema");
+	}
+	lua_getfield(L, -1, "func");
+	if (!lua_istable(L, -1)) {
+		lua_pop(L, 1); /* pop nil */
+		lua_newtable(L);
+		lua_setfield(L, -2, "func");
+		lua_getfield(L, -1, "func");
+	}
+	lua_getfield(L, -1, "persistent");
+	if (!lua_istable(L, -1)) {
+		lua_pop(L, 1); /* pop nil */
+		lua_newtable(L);
+		lua_setfield(L, -2, "persistent");
+		lua_getfield(L, -1, "persistent");
+	}
+	lua_rawgeti(L, -1, func->def->fid);
+	if (lua_isnil(L, -1)) {
+		/*
+		 * If the function already exists, modify it,
+		 * rather than create a new one -- to not
+		 * invalidate Lua variable references to old func
+		 * outside the box.schema.func[].
+		 */
+		lua_pop(L, 1);
+		lua_newtable(L);
+		lua_rawseti(L, -2, func->def->fid);
+		lua_rawgeti(L, -1, func->def->fid);
+	} else {
+		/* Clear the reference to old space by old name. */
+		lua_getfield(L, -1, "name");
+		lua_pushnil(L);
+		lua_settable(L, -4);
+	}
+
+	int top = lua_gettop(L);
+	lua_pushstring(L, "id");
+	lua_pushnumber(L, func->def->fid);
+	lua_settable(L, top);
+
+	lua_pushstring(L, "name");
+	lua_pushstring(L, func->def->name);
+	lua_settable(L, top);
+
+	lua_pushstring(L, "call");
+	lua_rawgeti(L, LUA_REGISTRYINDEX, func->lua_func_ref);
+	lua_settable(L, top);
+
+	lua_setfield(L, -2, func->def->name);
+
+	lua_pop(L, 4); /* box, schema, func, persistent */
+}
+
+
+static void
+box_lua_func_delete(struct lua_State *L, uint32_t fid)
+{
+	lua_getfield(L, LUA_GLOBALSINDEX, "box");
+	lua_getfield(L, -1, "schema");
+	lua_getfield(L, -1, "func");
+	lua_getfield(L, -1, "persistent");
+	lua_rawgeti(L, -1, fid);
+	if (!lua_isnil(L, -1)) {
+		lua_getfield(L, -1, "name");
+		lua_pushnil(L);
+		lua_rawset(L, -4);
+		lua_pop(L, 1); /* pop func */
+
+		lua_pushnil(L);
+		lua_rawseti(L, -2, fid);
+	} else {
+		lua_pop(L, 1);
+	}
+	lua_pop(L, 4); /* box, schema, func, persistent */
+}
+
+static void
+box_lua_func_new_or_delete(struct trigger *trigger, void *event)
+{
+	struct lua_State *L = (struct lua_State *) trigger->data;
+	uint32_t fid = (uint32_t) event;
+	struct func *func = func_by_id(fid);
+	/* Export only persistent Lua functions. */
+	if (func != NULL && func->def->language == FUNC_LANGUAGE_LUA &&
+	    func->def->lua_code != NULL)
+		box_lua_func_new(L, func);
+	else
+		box_lua_func_delete(L, fid);
+}
+
+static struct trigger on_alter_func_in_lua = {
+	RLIST_LINK_INITIALIZER, box_lua_func_new_or_delete, NULL, NULL
+};
+
+void
+box_lua_func_init(struct lua_State *L)
+{
+	/*
+	 * Register the trigger that will push persistent
+	 * Lua functions objects to Lua.
+	 */
+	on_alter_func_in_lua.data = L;
+	trigger_add(&on_alter_func, &on_alter_func_in_lua);
+}
diff --git a/src/box/func.h b/src/box/func.h
index b00ed5690..21c1087cf 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -134,6 +134,12 @@ func_capture_module(struct func *new_func, struct func *old_func);
 int
 module_reload(const char *package, const char *package_end, struct module **module);
 
+struct lua_State;
+
+/** Initialize func export triggers. */
+void
+box_lua_func_init(struct lua_State *L);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 76b987b4b..67a822eb4 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -39,6 +39,7 @@
 
 #include "box/box.h"
 #include "box/txn.h"
+#include "box/func.h"
 #include "box/vclock.h"
 
 #include "box/lua/error.h"
@@ -315,6 +316,7 @@ box_lua_init(struct lua_State *L)
 	box_lua_xlog_init(L);
 	box_lua_execute_init(L);
 	luaopen_net_box(L);
+	box_lua_func_init(L);
 	lua_pop(L, 1);
 	tarantool_lua_console_init(L);
 	lua_pop(L, 1);
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index ca7da6501..94eb6af90 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1977,6 +1977,7 @@ local function object_name(object_type, object_id)
 end
 
 box.schema.func = {}
+box.schema.func.persistent = {}
 box.schema.func.create = function(name, opts)
     opts = opts or {}
     check_param_table(opts, { setuid = 'boolean',
diff --git a/src/box/schema.cc b/src/box/schema.cc
index aae92874c..97c994c26 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -72,6 +72,7 @@ uint32_t space_cache_version = 0;
 struct rlist on_schema_init = RLIST_HEAD_INITIALIZER(on_schema_init);
 struct rlist on_alter_space = RLIST_HEAD_INITIALIZER(on_alter_space);
 struct rlist on_alter_sequence = RLIST_HEAD_INITIALIZER(on_alter_sequence);
+struct rlist on_alter_func = RLIST_HEAD_INITIALIZER(on_alter_func);
 
 /**
  * Lock of scheme modification
diff --git a/src/box/schema.h b/src/box/schema.h
index bc15c8a2c..3a3daffcf 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -130,6 +130,11 @@ int
 schema_find_id(uint32_t system_space_id, uint32_t index_id, const char *name,
 	       uint32_t len, uint32_t *object_id);
 
+struct func;
+
+struct func *
+func_by_id(uint32_t fid);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 
@@ -178,11 +183,6 @@ func_cache_replace(struct func *new_func, struct func **old_func);
 void
 func_cache_delete(uint32_t fid);
 
-struct func;
-
-struct func *
-func_by_id(uint32_t fid);
-
 static inline struct func *
 func_cache_find(uint32_t fid)
 {
@@ -243,6 +243,12 @@ extern struct rlist on_alter_sequence;
  */
 extern struct rlist on_access_denied;
 
+/**
+ * Triggers fired after committing a change in _func space.
+ * It is passed the txn statement that altered the space.
+ */
+extern struct rlist on_alter_func;
+
 /**
  * Context passed to on_access_denied trigger.
  */
diff --git a/test/box/persistent_func.result b/test/box/persistent_func.result
index ba544394e..5455b7eb4 100644
--- a/test/box/persistent_func.result
+++ b/test/box/persistent_func.result
@@ -43,6 +43,15 @@ box.schema.func.exists('test')
 ---
 - true
 ...
+box.schema.func.persistent.test ~= nil
+---
+- true
+...
+box.schema.func.persistent.test.call({address = "Moscow Dolgoprudny"})
+---
+- - - moscow
+  - - dolgoprudny
+...
 conn:call("test", {{address = "Moscow Dolgoprudny"}})
 ---
 - [['moscow'], ['dolgoprudny']]
@@ -144,6 +153,15 @@ test_run = require('test_run').new()
 conn = net.connect(box.cfg.listen)
 ---
 ...
+box.schema.func.persistent.test ~= nil
+---
+- true
+...
+box.schema.func.persistent.test.call({address = "Moscow Dolgoprudny"})
+---
+- - - moscow
+  - - dolgoprudny
+...
 conn:call("test", {{address = "Moscow Dolgoprudny"}})
 ---
 - [['moscow'], ['dolgoprudny']]
@@ -154,6 +172,10 @@ conn:close()
 box.schema.func.drop('test')
 ---
 ...
+box.schema.func.persistent.test == nil
+---
+- true
+...
 box.schema.func.exists('test')
 ---
 - false
diff --git a/test/box/persistent_func.test.lua b/test/box/persistent_func.test.lua
index 67a315197..e87b366a9 100644
--- a/test/box/persistent_func.test.lua
+++ b/test/box/persistent_func.test.lua
@@ -21,6 +21,8 @@ test_run:cmd("setopt delimiter ''");
 box.schema.func.create('test', {lua_code = lua_code, language = "C"})
 box.schema.func.create('test', {lua_code = lua_code})
 box.schema.func.exists('test')
+box.schema.func.persistent.test ~= nil
+box.schema.func.persistent.test.call({address = "Moscow Dolgoprudny"})
 conn:call("test", {{address = "Moscow Dolgoprudny"}})
 
 -- Test that monkey-patch attack is not possible.
@@ -71,8 +73,11 @@ test_run:cmd("restart server default")
 net = require('net.box')
 test_run = require('test_run').new()
 conn = net.connect(box.cfg.listen)
+box.schema.func.persistent.test ~= nil
+box.schema.func.persistent.test.call({address = "Moscow Dolgoprudny"})
 conn:call("test", {{address = "Moscow Dolgoprudny"}})
 conn:close()
 box.schema.func.drop('test')
+box.schema.func.persistent.test == nil
 box.schema.func.exists('test')
 box.schema.user.revoke('guest', 'execute', 'universe')
-- 
2.21.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [tarantool-patches] [PATCH v1 2/3] schema: extend _func space to persist lua functions
  2019-05-14 13:29 ` [PATCH v1 2/3] schema: extend _func space to persist lua functions Kirill Shcherbatov
@ 2019-05-14 18:21   ` Konstantin Osipov
  2019-05-17 12:33     ` [tarantool-patches] " Kirill Shcherbatov
  0 siblings, 1 reply; 9+ messages in thread
From: Konstantin Osipov @ 2019-05-14 18:21 UTC (permalink / raw)
  To: tarantool-patches; +Cc: vdavydov.dev, Kirill Shcherbatov

* 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [tarantool-patches] [PATCH v1 3/3] box: extend box.schema.func with persistent folder
  2019-05-14 13:29 ` [PATCH v1 3/3] box: extend box.schema.func with persistent folder Kirill Shcherbatov
@ 2019-05-14 18:24   ` Konstantin Osipov
  0 siblings, 0 replies; 9+ messages in thread
From: Konstantin Osipov @ 2019-05-14 18:24 UTC (permalink / raw)
  To: tarantool-patches; +Cc: vdavydov.dev, Kirill Shcherbatov

* Kirill Shcherbatov <kshcherbatov@tarantool.org> [19/05/14 16:31]:
> Now all persistent Lua functions are available with
> box.schema.func.persistent table.

> lua_code = [[function(a, b) return a + b end]]
> box.schema.func.create('sum', {lua_code = lua_code})
> box.schema.func.persistent.sum

> ---
> - call: 'function: 0x4014b7f8'
>   name: sum
>   id: 2
> ...
Why do we need this?  I would not do it unless there is a request. 
A function in Lua is a first class object, anyone could make it
from the text stored in the system table. What's the point of
making it for the user?

A useful information would be, for procedures at least, the count
and types of function arguments. But it is even better if we make
this information available right in the system space, not in Lua.

Please keep in mind two things:

- Lua is just one  language among others, SQL is the other. 
- the feature should work at the core level, e.g. system spaces,
  and Lua level is just sugar wrapping around the core.
  

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [tarantool-patches] Re: [PATCH v1 2/3] schema: extend _func space to persist lua functions
  2019-05-14 18:21   ` [tarantool-patches] " Konstantin Osipov
@ 2019-05-17 12:33     ` Kirill Shcherbatov
  2019-05-17 15:29       ` Konstantin Osipov
  0 siblings, 1 reply; 9+ messages in thread
From: Kirill Shcherbatov @ 2019-05-17 12:33 UTC (permalink / raw)
  To: tarantool-patches, Konstantin Osipov; +Cc: vdavydov.dev

Hi! Thank you for your feedback.

> 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'.

This field is called 'body' now.

> 
> 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).

Thank you for this reference.
I think we'll need 'is_deterministic' flag soon enough (in scope of
functional indexes), so I've introduced it.
To make the process of introducing such options extensible,
@locker and I decided to put 'is_deterministic' and other new
flags in the 'opts' map field. I've reused existent opts_decode
mechanism in scope of this patch.

> 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.

I've introduced luaT_get_sandbox public method that clones an existent
'defaullt' sandbox and pushes it on the top of Lua stack in utils.h.
They use private references luaL_default_sandbox_ref and
luaL_deepcopy_func_ref are initialized on first demand.

I've discussed this problem with @Totktonada an he approved my
concept.

> 
>> +/**
>> + * 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. 

This allows backward compatibility with reusing loaded modules when
updating the function definition, as is done in Tarantool now.

/**
 * Take over the modules that belonged to the old function.
 * Reset module and function pointers in old function.
 * This helper allows you to inherit already loaded function
 * objects, that is useful on function object update.
*/
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?

The new name is
static int
execute_persistent_function(lua_State *L)

=================================================
This patch extends _func system space with a new string fields
body and opts to persist Lua functions.

Refactored func hash machinery to pass a new function object and
to return previous one. This allows to implement fault-tolerant
func_new call that constructs a lua function object.

Each persistent function may use limited amount of Lua functions
and modules:
-assert -error -pairs -ipairs -next -pcall -xpcall -type
-print -select -string -tonumber -tostring -unpack -math -utf8
Global variables are forbidden in persistent Lua functions.

This patch makes persistent Lua functions are available via
net.box.connect() :call method.
Example:
lua_code = [[function(a, b) return a + b end]]
box.schema.func.create('sum', {body = lua_code,
                       opts = {is_deterministic = true}})
conn:call("sum", {1, 3})

Part of #4182
Needed for #1260
---
 src/box/alter.cc                  |  94 +++++++++++-----
 src/box/bootstrap.snap            | Bin 4374 -> 4403 bytes
 src/box/call.c                    |   2 +-
 src/box/func.c                    |  69 +++++++++++-
 src/box/func.h                    |  17 ++-
 src/box/func_def.c                |   9 ++
 src/box/func_def.h                |  36 +++++-
 src/box/lua/call.c                |  36 +++++-
 src/box/lua/call.h                |   4 +-
 src/box/lua/schema.lua            |   9 +-
 src/box/lua/upgrade.lua           |  21 +++-
 src/box/schema.cc                 |  46 ++++----
 src/box/schema.h                  |  15 ++-
 src/box/schema_def.h              |   2 +
 src/lua/utils.c                   |  67 +++++++++++
 src/lua/utils.h                   |   8 ++
 test/box-py/bootstrap.result      |   6 +-
 test/box/access_misc.result       |   4 +-
 test/box/persistent_func.result   | 179 ++++++++++++++++++++++++++++++
 test/box/persistent_func.test.lua |  83 ++++++++++++++
 20 files changed, 623 insertions(+), 84 deletions(-)
 create mode 100644 test/box/persistent_func.result
 create mode 100644 test/box/persistent_func.test.lua

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 9279426d2..e3fd2e7ba 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2402,21 +2402,42 @@ func_def_get_ids_from_tuple(struct tuple *tuple, uint32_t *fid, uint32_t *uid)
 static struct func_def *
 func_def_new_from_tuple(struct tuple *tuple)
 {
-	uint32_t len;
-	const char *name = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_NAME,
-					      &len);
-	if (len > BOX_NAME_MAX)
+	uint32_t name_len, body_len;
+	const char *name, *body;
+	name = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_NAME, &name_len);
+	if (name_len > BOX_NAME_MAX) {
 		tnt_raise(ClientError, ER_CREATE_FUNCTION,
 			  tt_cstr(name, BOX_INVALID_NAME_MAX),
 			  "function name is too long");
-	identifier_check_xc(name, len);
-	struct func_def *def = (struct func_def *) malloc(func_def_sizeof(len));
+	}
+	identifier_check_xc(name, name_len);
+	if (tuple_field_count(tuple) > BOX_FUNC_FIELD_BODY) {
+		body = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_BODY,
+					  &body_len);
+	} else {
+		body = NULL;
+		body_len = 0;
+	}
+
+	uint32_t def_sz = func_def_sizeof(name_len, body_len);
+	struct func_def *def = (struct func_def *) malloc(def_sz);
 	if (def == NULL)
-		tnt_raise(OutOfMemory, func_def_sizeof(len), "malloc", "def");
+		tnt_raise(OutOfMemory, def_sz, "malloc", "def");
 	auto def_guard = make_scoped_guard([=] { free(def); });
+	func_opts_create(&def->opts);
 	func_def_get_ids_from_tuple(tuple, &def->fid, &def->uid);
-	memcpy(def->name, name, len);
-	def->name[len] = 0;
+
+	def->name = (char *)def + sizeof(struct func_def);
+	memcpy(def->name, name, name_len);
+	def->name[name_len] = 0;
+	if (body_len > 0) {
+		def->body = def->name + name_len + 1;
+		memcpy(def->body, body, body_len);
+		def->body[body_len] = 0;
+	} else {
+		def->body = NULL;
+	}
+
 	if (tuple_field_count(tuple) > BOX_FUNC_FIELD_SETUID)
 		def->setuid = tuple_field_u32_xc(tuple, BOX_FUNC_FIELD_SETUID);
 	else
@@ -2433,30 +2454,40 @@ func_def_new_from_tuple(struct tuple *tuple)
 		/* Lua is the default. */
 		def->language = FUNC_LANGUAGE_LUA;
 	}
+	if (def->language != FUNC_LANGUAGE_LUA && body_len > 0) {
+		tnt_raise(ClientError, ER_CREATE_FUNCTION, name,
+			  "function body may be specified only for "
+			  "Lua language");
+	}
+	if (tuple_field_count(tuple) > BOX_FUNC_FIELD_OPTS) {
+		const char *opts = tuple_field(tuple, BOX_FUNC_FIELD_OPTS);
+		if (opts_decode(&def->opts, func_opts_reg, &opts,
+				ER_WRONG_SPACE_OPTIONS, BOX_FUNC_FIELD_OPTS,
+				NULL) != 0)
+			diag_raise();
+	}
 	def_guard.is_active = false;
 	return def;
 }
 
 /** Remove a function from function cache */
 static void
-func_cache_remove_func(struct trigger * /* trigger */, void *event)
+func_cache_remove_func(struct trigger *trigger, void * /* event */)
 {
-	struct txn_stmt *stmt = txn_last_stmt((struct txn *) event);
-	uint32_t fid = tuple_field_u32_xc(stmt->old_tuple ?
-				       stmt->old_tuple : stmt->new_tuple,
-				       BOX_FUNC_FIELD_ID);
-	func_cache_delete(fid);
+	struct func *old_func = (struct func *) trigger->data;
+	func_cache_delete(old_func->def->fid);
+	func_delete(old_func);
 }
 
 /** Replace a function in the function cache */
 static void
-func_cache_replace_func(struct trigger * /* trigger */, void *event)
+func_cache_replace_func(struct trigger *trigger, void * /* event */)
 {
-	struct txn_stmt *stmt = txn_last_stmt((struct txn*) event);
-	struct func_def *def = func_def_new_from_tuple(stmt->new_tuple);
-	auto def_guard = make_scoped_guard([=] { free(def); });
-	func_cache_replace(def);
-	def_guard.is_active = false;
+	struct func *new_func = (struct func *) trigger->data;
+	struct func *old_func;
+	func_cache_replace(new_func, &old_func);
+	assert(old_func != NULL);
+	func_delete(old_func);
 }
 
 /**
@@ -2477,13 +2508,20 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 	struct func *old_func = func_by_id(fid);
 	if (new_tuple != NULL && old_func == NULL) { /* INSERT */
 		struct func_def *def = func_def_new_from_tuple(new_tuple);
+		auto def_guard = make_scoped_guard([=] { free(def); });
 		access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
 				 PRIV_C);
-		auto def_guard = make_scoped_guard([=] { free(def); });
-		func_cache_replace(def);
+		struct func *func = func_new(def);
+		if (func == NULL)
+			diag_raise();
+		auto func_guard = make_scoped_guard([=] { func_delete(func); });
 		def_guard.is_active = false;
+		struct func *old_func = NULL;
+		func_cache_replace(func, &old_func);
+		assert(old_func == NULL);
+		func_guard.is_active = false;
 		struct trigger *on_rollback =
-			txn_alter_trigger_new(func_cache_remove_func, NULL);
+			txn_alter_trigger_new(func_cache_remove_func, func);
 		txn_on_rollback(txn, on_rollback);
 	} else if (new_tuple == NULL) {         /* DELETE */
 		uint32_t uid;
@@ -2501,15 +2539,19 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 				  "function has grants");
 		}
 		struct trigger *on_commit =
-			txn_alter_trigger_new(func_cache_remove_func, NULL);
+			txn_alter_trigger_new(func_cache_remove_func, old_func);
 		txn_on_commit(txn, on_commit);
 	} else {                                /* UPDATE, REPLACE */
 		struct func_def *def = func_def_new_from_tuple(new_tuple);
 		auto def_guard = make_scoped_guard([=] { free(def); });
 		access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
 				 PRIV_A);
+		struct func *func = func_new(def);
+		if (func == NULL)
+			diag_raise();
+		def_guard.is_active = false;
 		struct trigger *on_commit =
-			txn_alter_trigger_new(func_cache_replace_func, NULL);
+			txn_alter_trigger_new(func_cache_replace_func, func);
 		txn_on_commit(txn, on_commit);
 	}
 }

diff --git a/src/box/call.c b/src/box/call.c
index 56da53fb3..773b914b1 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -202,7 +202,7 @@ box_process_call(struct call_request *request, struct port *port)
 	if (func && func->def->language == FUNC_LANGUAGE_C) {
 		rc = box_c_call(func, request, port);
 	} else {
-		rc = box_lua_call(request, port);
+		rc = box_lua_call(func, request, port);
 	}
 	/* Restore the original user */
 	if (orig_credentials)
diff --git a/src/box/func.c b/src/box/func.c
index a817851fd..d650b0144 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -34,6 +34,7 @@
 #include "lua/utils.h"
 #include "error.h"
 #include "diag.h"
+#include "fiber.h"
 #include <dlfcn.h>
 
 /**
@@ -355,6 +356,49 @@ restore:
 	return -1;
 }
 
+/**
+ * Assemble a Lua function object on Lua stack and return
+ * the reference.
+ * Returns func object reference on success, LUA_REFNIL otherwise.
+ */
+static int
+func_lua_code_load(struct func_def *def)
+{
+	int rc = LUA_REFNIL;
+	struct region *region = &fiber()->gc;
+	size_t region_svp = region_used(region);
+	struct lua_State *L = lua_newthread(tarantool_L);
+	int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX);
+
+	/*
+	 * Assemble a Lua function object with luaL_loadstring of
+	 * special 'return FUNCTION_BODY' expression and call it.
+	 * Set default sandbox to configure it to use only a
+	 * limited number of functions and modules.
+	 */
+	const char *load_pref = "return ";
+	uint32_t load_str_sz = strlen(load_pref) + strlen(def->body) + 1;
+	char *load_str = region_alloc(region, load_str_sz);
+	if (load_str == NULL) {
+		diag_set(OutOfMemory, load_str_sz, "region", "load_str");
+		goto end;
+	}
+	sprintf(load_str, "%s%s", load_pref, def->body);
+	if (luaL_loadstring(L, load_str) != 0 ||
+	    lua_pcall(L, 0, 1, 0) != 0 || !lua_isfunction(L, -1) ||
+	    luaT_get_sandbox(L) != 0) {
+		diag_set(ClientError, ER_LOAD_FUNCTION, def->name,
+			def->body);
+		goto end;
+	}
+	lua_setfenv(L, -2);
+	rc = luaL_ref(L, LUA_REGISTRYINDEX);
+end:
+	region_truncate(region, region_svp);
+	luaL_unref(L, LUA_REGISTRYINDEX, coro_ref);
+	return rc;
+}
+
 struct func *
 func_new(struct func_def *def)
 {
@@ -380,6 +424,15 @@ func_new(struct func_def *def)
 	func->owner_credentials.auth_token = BOX_USER_MAX; /* invalid value */
 	func->func = NULL;
 	func->module = NULL;
+	if (func->def->body != NULL) {
+		func->lua_func_ref = func_lua_code_load(def);
+		if (func->lua_func_ref == LUA_REFNIL) {
+			free(func);
+			return NULL;
+		}
+	} else {
+		func->lua_func_ref = LUA_REFNIL;
+	}
 	return func;
 }
 
@@ -395,8 +448,11 @@ func_unload(struct func *func)
 		}
 		module_gc(func->module);
 	}
+	if (func->lua_func_ref != -1)
+		luaL_unref(tarantool_L, LUA_REGISTRYINDEX, func->lua_func_ref);
 	func->module = NULL;
 	func->func = NULL;
+	func->lua_func_ref = -1;
 }
 
 /**
@@ -452,17 +508,18 @@ func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
 }
 
 void
-func_update(struct func *func, struct func_def *def)
+func_delete(struct func *func)
 {
 	func_unload(func);
 	free(func->def);
-	func->def = def;
+	free(func);
 }
 
 void
-func_delete(struct func *func)
+func_capture_module(struct func *new_func, struct func *old_func)
 {
-	func_unload(func);
-	free(func->def);
-	free(func);
+	new_func->module = old_func->module;
+	new_func->func = old_func->func;
+	old_func->module = NULL;
+	old_func->func = NULL;
 }
diff --git a/src/box/func.h b/src/box/func.h
index 8dcd61d7b..eaf5a3902 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -65,6 +65,11 @@ struct func {
 	 * Anchor for module membership.
 	 */
 	struct rlist item;
+	/**
+	 * The reference index of Lua function object.
+	 * Is equal to LUA_REFNIL when undefined.
+	 */
+	int lua_func_ref;
 	/**
 	 * For C functions, the body of the function.
 	 */
@@ -100,9 +105,6 @@ module_free(void);
 struct func *
 func_new(struct func_def *def);
 
-void
-func_update(struct func *func, struct func_def *def);
-
 void
 func_delete(struct func *func);
 
@@ -113,6 +115,15 @@ int
 func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
 	  const char *args_end);
 
+/**
+ * Take over the modules that belonged to the old function.
+ * Reset module and function pointers in old function.
+ * This helper allows you to inherit already loaded function
+ * objects, that can is useful on update.
+*/
+void
+func_capture_module(struct func *new_func, struct func *old_func);
+
 /**
  * Reload dynamically loadable module.
  *
diff --git a/src/box/func_def.c b/src/box/func_def.c
index 76ed77b24..6c143f075 100644
--- a/src/box/func_def.c
+++ b/src/box/func_def.c
@@ -1,3 +1,12 @@
 #include "func_def.h"
+#include "opt_def.h"
 
 const char *func_language_strs[] = {"LUA", "C"};
+
+const struct func_opts func_opts_default = {
+	/* .is_deterministic = */ false,
+};
+
+const struct opt_def func_opts_reg[] = {
+	OPT_DEF("is_deterministic", OPT_BOOL, struct func_opts, is_deterministic),
+};
diff --git a/src/box/func_def.h b/src/box/func_def.h
index 5b52ab498..6c35a684b 100644
--- a/src/box/func_def.h
+++ b/src/box/func_def.h
@@ -32,6 +32,7 @@
  */
 
 #include "trivia/util.h"
+#include "opt_def.h"
 #include <stdbool.h>
 
 /**
@@ -45,6 +46,19 @@ enum func_language {
 
 extern const char *func_language_strs[];
 
+/** Function options. */
+struct func_opts {
+	/**
+	 * Whether the routine is deterministic (can produce
+	 * only one result for a given list of parameters)
+	 * or not.
+	 */
+	bool is_deterministic;
+};
+
+extern const struct func_opts func_opts_default;
+extern const struct opt_def func_opts_reg[];
+
 /**
  * Definition of a function. Function body is not stored
  * or replicated (yet).
@@ -54,6 +68,10 @@ struct func_def {
 	uint32_t fid;
 	/** Owner of the function. */
 	uint32_t uid;
+	/** Function name. */
+	char *name;
+	/** Definition of the routine. */
+	char *body;
 	/**
 	 * True if the function requires change of user id before
 	 * invocation.
@@ -63,8 +81,8 @@ struct func_def {
 	 * The language of the stored function.
 	 */
 	enum func_language language;
-	/** Function name. */
-	char name[0];
+	/** The function options. */
+	struct func_opts opts;
 };
 
 /**
@@ -73,10 +91,20 @@ struct func_def {
  * for a function of length @a a name_len.
  */
 static inline size_t
-func_def_sizeof(uint32_t name_len)
+func_def_sizeof(uint32_t name_len, uint32_t body_len)
 {
 	/* +1 for '\0' name terminating. */
-	return sizeof(struct func_def) + name_len + 1;
+	size_t sz = sizeof(struct func_def) + name_len + 1;
+	if (body_len > 0)
+		sz += body_len + 1;
+	return sz;
+}
+
+/** Create index options using default values. */
+static inline void
+func_opts_create(struct func_opts *opts)
+{
+	*opts = func_opts_default;
 }
 
 /**
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index c729778c4..dcd505cc3 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -32,6 +32,8 @@
 #include "box/call.h"
 #include "box/error.h"
 #include "fiber.h"
+#include "box/func.h"
+#include "box/schema.h"
 
 #include "lua/utils.h"
 #include "lua/msgpack.h"
@@ -253,6 +255,33 @@ execute_lua_call(lua_State *L)
 	return lua_gettop(L);
 }
 
+static int
+execute_persistent_function(lua_State *L)
+{
+	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 != LUA_REFNIL);
+	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);
+}
+
 static int
 execute_lua_eval(lua_State *L)
 {
@@ -396,9 +425,12 @@ box_process_lua(struct call_request *request, struct port *base,
 }
 
 int
-box_lua_call(struct call_request *request, struct port *port)
+box_lua_call(struct func *func, struct call_request *request, struct port *port)
 {
-	return box_process_lua(request, port, execute_lua_call);
+	if (func != NULL && func->def->body != NULL)
+		return box_process_lua(request, port, execute_persistent_function);
+	else
+		return box_process_lua(request, port, execute_lua_call);
 }
 
 int
diff --git a/src/box/lua/call.h b/src/box/lua/call.h
index 0542123da..d8ef65900 100644
--- a/src/box/lua/call.h
+++ b/src/box/lua/call.h
@@ -42,13 +42,15 @@ box_lua_call_init(struct lua_State *L);
 
 struct port;
 struct call_request;
+struct func;
 
 /**
  * Invoke a Lua stored procedure from the binary protocol
  * (implementation of 'CALL' command code).
  */
 int
-box_lua_call(struct call_request *request, struct port *port);
+box_lua_call(struct func *func, struct call_request *request,
+	     struct port *port);
 
 int
 box_lua_eval(struct call_request *request, struct port *port);
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index f31cf7f2c..8cf3867f4 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1981,7 +1981,8 @@ box.schema.func.create = function(name, opts)
     opts = opts or {}
     check_param_table(opts, { setuid = 'boolean',
                               if_not_exists = 'boolean',
-                              language = 'string'})
+                              language = 'string', body = 'string',
+                              opts = 'table'})
     local _func = box.space[box.schema.FUNC_ID]
     local _vfunc = box.space[box.schema.VFUNC_ID]
     local func = _vfunc.index.name:get{name}
@@ -1991,10 +1992,12 @@ box.schema.func.create = function(name, opts)
         end
         return
     end
-    opts = update_param_table(opts, { setuid = false, language = 'lua'})
+    opts = update_param_table(opts, { setuid = false, language = 'lua',
+                                      body = '', opts = setmap{}})
     opts.language = string.upper(opts.language)
     opts.setuid = opts.setuid and 1 or 0
-    _func:auto_increment{session.euid(), name, opts.setuid, opts.language}
+    _func:auto_increment{session.euid(), name, opts.setuid, opts.language,
+                         opts.body, opts.opts}
 end
 
 box.schema.func.drop = function(name, opts)
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 89d6e3d52..ef3d23c6b 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -324,7 +324,7 @@ local function initial_1_7_5()
 
     -- create "box.schema.user.info" function
     log.info('create function "box.schema.user.info" with setuid')
-    _func:replace{1, ADMIN, 'box.schema.user.info', 1, 'LUA'}
+    _func:replace{1, ADMIN, 'box.schema.user.info', 1, 'LUA', '', MAP}
 
     -- grant 'public' role access to 'box.schema.user.info' function
     log.info('grant execute on function "box.schema.user.info" to public')
@@ -737,6 +737,24 @@ local function upgrade_to_2_1_3()
     end
 end
 
+local function upgrade_to_2_2_0()
+    log.info("Update _func format")
+    local _func = box.space[box.schema.FUNC_ID]
+    local format = {}
+    format[1] = {name='id', type='unsigned'}
+    format[2] = {name='owner', type='unsigned'}
+    format[3] = {name='name', type='string'}
+    format[4] = {name='setuid', type='unsigned'}
+    format[5] = {name='language', type='string'}
+    format[6] = {name='body', type='string'}
+    format[7] = {name='opts', type='map'}
+    for _, v in box.space._func:pairs() do
+        _ = box.space._func:replace({v.id, v.owner, v.name, v.setuid,
+                                     v[5] or 'LUA', '', setmap({})})
+    end
+    _func:format(format)
+end
+
 local function get_version()
     local version = box.space._schema:get{'version'}
     if version == nil then
@@ -768,6 +786,7 @@ local function upgrade(options)
         {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
         {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
         {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
+        {version = mkversion(2, 2, 0), func = upgrade_to_2_2_0, auto = true},
     }
 
     for _, handler in ipairs(handlers) do
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 9a55c2f14..b70992297 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -533,40 +533,35 @@ schema_free(void)
 }
 
 void
-func_cache_replace(struct func_def *def)
+func_cache_replace(struct func *new_func, struct func **old_func)
 {
-	struct func *old = func_by_id(def->fid);
-	if (old) {
-		func_update(old, def);
-		return;
-	}
 	if (mh_size(funcs) >= BOX_FUNCTION_MAX)
 		tnt_raise(ClientError, ER_FUNCTION_MAX, BOX_FUNCTION_MAX);
-	struct func *func = func_new(def);
-	if (func == NULL) {
-error:
-		panic_syserror("Out of memory for the data "
-			       "dictionary cache (stored function).");
-	}
-	const struct mh_i32ptr_node_t node = { def->fid, func };
-	mh_int_t k1 = mh_i32ptr_put(funcs, &node, NULL, NULL);
-	if (k1 == mh_end(funcs)) {
-		func->def = NULL;
-		func_delete(func);
-		goto error;
-	}
-	size_t def_name_len = strlen(func->def->name);
-	uint32_t name_hash = mh_strn_hash(func->def->name, def_name_len);
+
+	mh_int_t k1, k2;
+	struct mh_i32ptr_node_t old_node, *old_node_ptr = &old_node;
+	const struct mh_i32ptr_node_t node = { new_func->def->fid, new_func };
+	size_t def_name_len = strlen(new_func->def->name);
+	uint32_t name_hash = mh_strn_hash(new_func->def->name, def_name_len);
 	const struct mh_strnptr_node_t strnode = {
-		func->def->name, def_name_len, name_hash, func };
+		new_func->def->name, def_name_len, name_hash, new_func };
 
-	mh_int_t k2 = mh_strnptr_put(funcs_by_name, &strnode, NULL, NULL);
+	k1 = mh_i32ptr_put(funcs, &node, &old_node_ptr, NULL);
+	if (k1 == mh_end(funcs))
+		goto error;
+	if (old_node_ptr != NULL)
+		*old_func = (struct func *)old_node_ptr->val;
+	k2 = mh_strnptr_put(funcs_by_name, &strnode, NULL, NULL);
 	if (k2 == mh_end(funcs_by_name)) {
 		mh_i32ptr_del(funcs, k1, NULL);
-		func->def = NULL;
-		func_delete(func);
 		goto error;
 	}
+	if (*old_func != NULL)
+		func_capture_module(new_func, *old_func);
+	return;
+error:
+	panic_syserror("Out of memory for the data dictionary cache "
+		       "(stored function).");
 }
 
 void
@@ -582,7 +577,6 @@ func_cache_delete(uint32_t fid)
 				strlen(func->def->name));
 	if (k != mh_end(funcs))
 		mh_strnptr_del(funcs_by_name, k, NULL);
-	func_delete(func);
 }
 
 struct func *
diff --git a/src/box/schema.h b/src/box/schema.h
index 6f9a96117..ffc41f401 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -163,14 +163,17 @@ schema_free();
 struct space *schema_space(uint32_t id);
 
 /**
- * Insert a new function or update the old one.
- *
- * @param def Function definition. In a case of success the ownership
- *        of @a def is transfered to the data dictionary, thus the caller
- *        must not delete it.
+ * Replace an existent (if any) function or insert a new one
+ * in function cache.
+ * @param func_new Function object to insert. In case of success,
+ *                 when old function object is exists, all loaded
+ *                 modules data are inherent with
+ *                 func_capture_module() when possible.
+ * @param func_old[out] The replaced function object if any.
+ * @retval Returns 0 on success, -1 otherwise.
  */
 void
-func_cache_replace(struct func_def *def);
+func_cache_replace(struct func *new_func, struct func **old_func);
 
 void
 func_cache_delete(uint32_t fid);
diff --git a/src/box/schema_def.h b/src/box/schema_def.h
index eeeeb950b..8cabc7e24 100644
--- a/src/box/schema_def.h
+++ b/src/box/schema_def.h
@@ -163,6 +163,8 @@ enum {
 	BOX_FUNC_FIELD_NAME = 2,
 	BOX_FUNC_FIELD_SETUID = 3,
 	BOX_FUNC_FIELD_LANGUAGE = 4,
+	BOX_FUNC_FIELD_BODY = 5,
+	BOX_FUNC_FIELD_OPTS = 6,
 };
 
 /** _collation fields. */
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 27ff6b396..973d8b15c 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -46,6 +46,14 @@ static uint32_t CTID_STRUCT_IBUF_PTR;
 uint32_t CTID_CHAR_PTR;
 uint32_t CTID_CONST_CHAR_PTR;
 
+static const char *default_sandbox_exports[] =
+	{"assert", "error", "ipairs", "math", "next", "pairs", "pcall",
+	"print", "select", "string", "table", "tonumber", "tostring",
+	"type", "unpack", "xpcall", "utf8"};
+
+static int luaL_deepcopy_func_ref = LUA_REFNIL;
+static int luaL_default_sandbox_ref = LUA_REFNIL;
+
 void *
 luaL_pushcdata(struct lua_State *L, uint32_t ctypeid)
 {
@@ -1248,6 +1256,65 @@ luaT_func_find(struct lua_State *L, const char *name, const char *name_end,
 	return 0;
 }
 
+/**
+ * Assemble a new sandbox with given exports table on top of the
+ * Lua stack. All modules in exports list are copying deeply
+ * to ensure the immutablility of this system object.
+ */
+static int
+luaT_prepare_sandbox(struct lua_State *L, const char *exports[],
+		     uint32_t exports_count)
+{
+	assert(luaL_deepcopy_func_ref != LUA_REFNIL);
+	lua_createtable(L, exports_count, 0);
+	for (unsigned i = 0; i < exports_count; i++) {
+		int count;
+		uint32_t name_len = strlen(exports[i]);
+		if (luaT_func_find(L, exports[i], exports[i] + name_len,
+				   &count) != 0)
+			return -1;
+		switch (lua_type(L, -1)) {
+		case LUA_TTABLE:
+			lua_rawgeti(L, LUA_REGISTRYINDEX,
+				    luaL_deepcopy_func_ref);
+			lua_insert(L, -2);
+			lua_call(L, 1, LUA_MULTRET);
+			FALLTHROUGH;
+		case LUA_TFUNCTION:
+			break;
+		default:
+			unreachable();
+		}
+		lua_setfield(L, -2, exports[i]);
+	}
+	return 0;
+}
+
+int
+luaT_get_sandbox(struct lua_State *L)
+{
+	if (unlikely(luaL_deepcopy_func_ref == LUA_REFNIL)) {
+		int count;
+		const char *deepcopy = "table.deepcopy";
+		if (luaT_func_find(L, deepcopy, deepcopy + strlen(deepcopy),
+				&count) != 0)
+			return -1;
+		luaL_deepcopy_func_ref = luaL_ref(L, LUA_REGISTRYINDEX);
+		assert(luaL_deepcopy_func_ref != LUA_REFNIL);
+	}
+	if (unlikely(luaL_default_sandbox_ref == LUA_REFNIL)) {
+		if (luaT_prepare_sandbox(L, default_sandbox_exports,
+					 nelem(default_sandbox_exports)) != 0)
+			return -1;
+		luaL_default_sandbox_ref = luaL_ref(L, LUA_REGISTRYINDEX);
+		assert(luaL_default_sandbox_ref != LUA_REFNIL);
+	}
+	lua_rawgeti(L, LUA_REGISTRYINDEX, luaL_deepcopy_func_ref);
+	lua_rawgeti(L, LUA_REGISTRYINDEX, luaL_default_sandbox_ref);
+	lua_call(L, 1, LUA_MULTRET);
+	return 0;
+}
+
 int
 tarantool_lua_utils_init(struct lua_State *L)
 {
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 81e936bee..36bb2e53b 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -603,6 +603,14 @@ int
 luaT_func_find(struct lua_State *L, const char *name, const char *name_end,
 	       int *count);
 
+/**
+ * Prepare a new 'default' sandbox table on the top of the Lua
+ * stack. The 'default' sandbox consists of a minimum set of
+ * functions that are sufficient to serve persistent functions.
+ */
+int
+luaT_get_sandbox(struct lua_State *L);
+
 int
 tarantool_lua_utils_init(struct lua_State *L);
 
diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
index 379f6c51f..ea8ae0984 100644
--- a/test/box-py/bootstrap.result
+++ b/test/box-py/bootstrap.result
@@ -4,7 +4,7 @@ box.internal.bootstrap()
 box.space._schema:select{}
 ---
 - - ['max_id', 511]
-  - ['version', 2, 1, 3]
+  - ['version', 2, 2, 0]
 ...
 box.space._cluster:select{}
 ---
@@ -49,7 +49,7 @@ box.space._space:select{}
         'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]]
   - [296, 1, '_func', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
         'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
-        'type': 'unsigned'}]]
+        'type': 'unsigned'}, {'name': 'lua_code', 'type': 'string'}]]
   - [297, 1, '_vfunc', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
         'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
         'type': 'unsigned'}]]
@@ -141,7 +141,7 @@ box.space._user:select{}
 ...
 box.space._func:select{}
 ---
-- - [1, 1, 'box.schema.user.info', 1, 'LUA']
+- - [1, 1, 'box.schema.user.info', 1, 'LUA', '']
 ...
 box.space._priv:select{}
 ---
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 36ebfae09..514cfaab0 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -789,7 +789,7 @@ box.space._space:select()
         'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]]
   - [296, 1, '_func', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
         'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
-        'type': 'unsigned'}]]
+        'type': 'unsigned'}, {'name': 'body', 'type': 'string'}, {'name': 'opts', 'type': 'map'}]]
   - [297, 1, '_vfunc', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
         'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
         'type': 'unsigned'}]]
@@ -821,7 +821,7 @@ box.space._space:select()
 ...
 box.space._func:select()
 ---
-- - [1, 1, 'box.schema.user.info', 1, 'LUA']
+- - [1, 1, 'box.schema.user.info', 1, 'LUA', '']
 ...
 session = nil
 ---
diff --git a/test/box/persistent_func.result b/test/box/persistent_func.result
new file mode 100644
index 000000000..0aae0a4ee
--- /dev/null
+++ b/test/box/persistent_func.result
@@ -0,0 +1,179 @@
+env = require('test_run')
+---
+...
+test_run = env.new()
+---
+...
+--
+-- gh-4182: Add persistent LUA functions.
+--
+box.schema.user.grant('guest', 'execute', 'universe')
+---
+...
+net = require('net.box')
+---
+...
+conn = net.connect(box.cfg.listen)
+---
+...
+-- Test valid function.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+body = [[function(tuple)
+	if type(tuple.address) ~= 'string' then return nil, 'Invalid field type' end
+	local t = tuple.address:lower():split()
+	for k,v in pairs(t) do t[k] = {v} end
+	return t
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('test', {body = body, language = "C"})
+---
+- error: 'Failed to create function ''test'': function body may be specified only
+    for Lua language'
+...
+box.schema.func.create('test', {body = body})
+---
+...
+box.schema.func.exists('test')
+---
+- true
+...
+conn:call("test", {{address = "Moscow Dolgoprudny"}})
+---
+- [['moscow'], ['dolgoprudny']]
+...
+box.schema.func.create('test2', {body = body, opts = {is_deterministic = true}})
+---
+...
+box.schema.func.create('test3', {body = body, opts = {is_deterministic = true, extra = true}})
+---
+- error: 'Wrong space options (field 6): unexpected option ''extra'''
+...
+-- Test that monkey-patch attack is not possible.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+body_monkey = [[function(tuple)
+	math.abs = math.log
+	return tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('body_monkey', {body = body_monkey})
+---
+...
+conn:call("body_monkey", {{address = "Moscow Dolgoprudny"}})
+---
+- {'address': 'Moscow Dolgoprudny'}
+...
+math.abs(-666.666)
+---
+- 666.666
+...
+-- Test taht 'require' is forbidden.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+body_bad1 = [[function(tuple)
+	local json = require('json')
+	return json.encode(tuple)
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('json_serializer', {body = body_bad1})
+---
+...
+conn:call("json_serializer", {{address = "Moscow Dolgoprudny"}})
+---
+- error: '[string "return function(tuple) ..."]:1: attempt to call global ''require''
+    (a nil value)'
+...
+-- Test function with spell error - case 1.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+body_bad2 = [[function(tuple)
+	ret tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('body_bad2', {body = body_bad2})
+---
+- error: "Failed to dynamically load function 'body_bad2': function(tuple) \tret tuple
+    end "
+...
+-- Test function with spell error - case 2.
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+body_bad3 = [[func(tuple)
+	return tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+---
+...
+box.schema.func.create('body_bad3', {body = body_bad3})
+---
+- error: "Failed to dynamically load function 'body_bad3': func(tuple) \treturn tuple
+    end "
+...
+conn:call("body_bad3", {{address = "Moscow Dolgoprudny"}})
+---
+- error: Procedure 'body_bad3' is not defined
+...
+conn:close()
+---
+...
+-- Restart server.
+test_run:cmd("restart server default")
+net = require('net.box')
+---
+...
+test_run = require('test_run').new()
+---
+...
+conn = net.connect(box.cfg.listen)
+---
+...
+conn:call("test", {{address = "Moscow Dolgoprudny"}})
+---
+- [['moscow'], ['dolgoprudny']]
+...
+conn:close()
+---
+...
+box.schema.func.drop('test')
+---
+...
+box.schema.func.exists('test')
+---
+- false
+...
+box.schema.func.drop('body_monkey')
+---
+...
+box.schema.func.drop('json_serializer')
+---
+...
+box.schema.func.drop('test2')
+---
+...
+box.schema.user.revoke('guest', 'execute', 'universe')
+---
+...
diff --git a/test/box/persistent_func.test.lua b/test/box/persistent_func.test.lua
new file mode 100644
index 000000000..7e321fb5c
--- /dev/null
+++ b/test/box/persistent_func.test.lua
@@ -0,0 +1,83 @@
+env = require('test_run')
+test_run = env.new()
+
+--
+-- gh-4182: Add persistent LUA functions.
+--
+box.schema.user.grant('guest', 'execute', 'universe')
+net = require('net.box')
+conn = net.connect(box.cfg.listen)
+
+-- Test valid function.
+test_run:cmd("setopt delimiter ';'")
+body = [[function(tuple)
+	if type(tuple.address) ~= 'string' then return nil, 'Invalid field type' end
+	local t = tuple.address:lower():split()
+	for k,v in pairs(t) do t[k] = {v} end
+	return t
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('test', {body = body, language = "C"})
+box.schema.func.create('test', {body = body})
+box.schema.func.exists('test')
+conn:call("test", {{address = "Moscow Dolgoprudny"}})
+box.schema.func.create('test2', {body = body, opts = {is_deterministic = true}})
+box.schema.func.create('test3', {body = body, opts = {is_deterministic = true, extra = true}})
+
+-- Test that monkey-patch attack is not possible.
+test_run:cmd("setopt delimiter ';'")
+body_monkey = [[function(tuple)
+	math.abs = math.log
+	return tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('body_monkey', {body = body_monkey})
+conn:call("body_monkey", {{address = "Moscow Dolgoprudny"}})
+math.abs(-666.666)
+
+-- Test taht 'require' is forbidden.
+test_run:cmd("setopt delimiter ';'")
+body_bad1 = [[function(tuple)
+	local json = require('json')
+	return json.encode(tuple)
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('json_serializer', {body = body_bad1})
+conn:call("json_serializer", {{address = "Moscow Dolgoprudny"}})
+
+-- Test function with spell error - case 1.
+test_run:cmd("setopt delimiter ';'")
+body_bad2 = [[function(tuple)
+	ret tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('body_bad2', {body = body_bad2})
+
+-- Test function with spell error - case 2.
+test_run:cmd("setopt delimiter ';'")
+body_bad3 = [[func(tuple)
+	return tuple
+end
+]]
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('body_bad3', {body = body_bad3})
+conn:call("body_bad3", {{address = "Moscow Dolgoprudny"}})
+
+conn:close()
+-- Restart server.
+test_run:cmd("restart server default")
+net = require('net.box')
+test_run = require('test_run').new()
+conn = net.connect(box.cfg.listen)
+conn:call("test", {{address = "Moscow Dolgoprudny"}})
+conn:close()
+box.schema.func.drop('test')
+box.schema.func.exists('test')
+box.schema.func.drop('body_monkey')
+box.schema.func.drop('json_serializer')
+box.schema.func.drop('test2')
+box.schema.user.revoke('guest', 'execute', 'universe')
-- 
2.21.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [tarantool-patches] [PATCH v1 1/3] box: refactor box_lua_find helper
  2019-05-14 13:29 ` [PATCH v1 1/3] box: refactor box_lua_find helper Kirill Shcherbatov
@ 2019-05-17 12:33   ` Kirill Shcherbatov
  0 siblings, 0 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2019-05-17 12:33 UTC (permalink / raw)
  To: tarantool-patches, Konstantin Osipov; +Cc: vdavydov.dev

The box_lua_find routine used to work with an empty stack only.
It is unacceptable in following patches because this helper
need to be reused in following patches for environment table
construction.
The definition also is moved to lua/utils.h.

Needed for #4182
---
 src/box/lua/call.c | 85 ++++++++--------------------------------------
 src/lua/utils.c    | 59 ++++++++++++++++++++++++++++++++
 src/lua/utils.h    | 11 ++++++
 3 files changed, 85 insertions(+), 70 deletions(-)

diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 04020ef6f..c729778c4 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -44,73 +44,6 @@
 #include "trivia/util.h"
 #include "mpstream.h"
 
-/**
- * A helper to find a Lua function by name and put it
- * on top of the stack.
- */
-static int
-box_lua_find(lua_State *L, const char *name, const char *name_end)
-{
-	int index = LUA_GLOBALSINDEX;
-	int objstack = 0;
-	const char *start = name, *end;
-
-	while ((end = (const char *) memchr(start, '.', name_end - start))) {
-		lua_checkstack(L, 3);
-		lua_pushlstring(L, start, end - start);
-		lua_gettable(L, index);
-		if (! lua_istable(L, -1)) {
-			diag_set(ClientError, ER_NO_SUCH_PROC,
-				 name_end - name, name);
-			luaT_error(L);
-		}
-		start = end + 1; /* next piece of a.b.c */
-		index = lua_gettop(L); /* top of the stack */
-	}
-
-	/* box.something:method */
-	if ((end = (const char *) memchr(start, ':', name_end - start))) {
-		lua_checkstack(L, 3);
-		lua_pushlstring(L, start, end - start);
-		lua_gettable(L, index);
-		if (! (lua_istable(L, -1) ||
-			lua_islightuserdata(L, -1) || lua_isuserdata(L, -1) )) {
-				diag_set(ClientError, ER_NO_SUCH_PROC,
-					  name_end - name, name);
-				luaT_error(L);
-		}
-		start = end + 1; /* next piece of a.b.c */
-		index = lua_gettop(L); /* top of the stack */
-		objstack = index;
-	}
-
-
-	lua_pushlstring(L, start, name_end - start);
-	lua_gettable(L, index);
-	if (!lua_isfunction(L, -1) && !lua_istable(L, -1)) {
-		/* lua_call or lua_gettable would raise a type error
-		 * for us, but our own message is more verbose. */
-		diag_set(ClientError, ER_NO_SUCH_PROC,
-			  name_end - name, name);
-		luaT_error(L);
-	}
-	/* setting stack that it would contain only
-	 * the function pointer. */
-	if (index != LUA_GLOBALSINDEX) {
-		if (objstack == 0) {        /* no object, only a function */
-			lua_replace(L, 1);
-		} else if (objstack == 1) { /* just two values, swap them */
-			lua_insert(L, -2);
-		} else {		    /* long path */
-			lua_insert(L, 1);
-			lua_insert(L, 2);
-			objstack = 1;
-		}
-		lua_settop(L, 1 + objstack);
-	}
-	return 1 + objstack;
-}
-
 /**
  * A helper to find lua stored procedures for box.call.
  * box.call iteslf is pure Lua, to avoid issues
@@ -124,7 +57,12 @@ lbox_call_loadproc(struct lua_State *L)
 	const char *name;
 	size_t name_len;
 	name = lua_tolstring(L, 1, &name_len);
-	return box_lua_find(L, name, name + name_len);
+	int count;
+	if (luaT_func_find(L, name, name + name_len, &count) != 0) {
+		diag_set(ClientError, ER_NO_SUCH_PROC, name_len, name);
+		return luaT_error(L);
+	}
+	return count;
 }
 
 /*
@@ -292,9 +230,16 @@ execute_lua_call(lua_State *L)
 	const char *name = request->name;
 	uint32_t name_len = mp_decode_strl(&name);
 
-	int oc = 0; /* how many objects are on stack after box_lua_find */
+	/*
+	 * How many objects are on stack after
+	 * luaT_func_find call.
+	 */
+	int oc = 0;
 	/* Try to find a function by name in Lua */
-	oc = box_lua_find(L, name, name + name_len);
+	if (luaT_func_find(L, name, name + name_len, &oc) != 0) {
+		diag_set(ClientError, ER_NO_SUCH_PROC, name_len, name);
+		return luaT_error(L);
+	}
 
 	/* Push the rest of args (a tuple). */
 	const char *args = request->args;
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 01a0cd894..27ff6b396 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -1189,6 +1189,65 @@ void luaL_iterator_delete(struct luaL_iterator *it)
 
 /* }}} */
 
+int
+luaT_func_find(struct lua_State *L, const char *name, const char *name_end,
+	       int *count)
+{
+	int index = LUA_GLOBALSINDEX;
+	int objstack = 0, top = lua_gettop(L);
+	const char *start = name, *end;
+
+	while ((end = (const char *) memchr(start, '.', name_end - start))) {
+		lua_checkstack(L, 3);
+		lua_pushlstring(L, start, end - start);
+		lua_gettable(L, index);
+		if (! lua_istable(L, -1))
+			return -1;
+		start = end + 1; /* next piece of a.b.c */
+		index = lua_gettop(L); /* top of the stack */
+	}
+
+	/* box.something:method */
+	if ((end = (const char *) memchr(start, ':', name_end - start))) {
+		lua_checkstack(L, 3);
+		lua_pushlstring(L, start, end - start);
+		lua_gettable(L, index);
+		if (! (lua_istable(L, -1) ||
+			lua_islightuserdata(L, -1) || lua_isuserdata(L, -1) ))
+				return -1;
+		start = end + 1; /* next piece of a.b.c */
+		index = lua_gettop(L); /* top of the stack */
+		objstack = index - top;
+	}
+
+	lua_pushlstring(L, start, name_end - start);
+	lua_gettable(L, index);
+	if (!lua_isfunction(L, -1) && !lua_istable(L, -1)) {
+		/* lua_call or lua_gettable would raise a type error
+		 * for us, but our own message is more verbose. */
+		return -1;
+	}
+
+	/* setting stack that it would contain only
+	 * the function pointer. */
+	if (index != LUA_GLOBALSINDEX) {
+		if (objstack == 0) {        /* no object, only a function */
+			lua_replace(L, top + 1);
+			lua_pop(L, lua_gettop(L) - top - 1);
+		} else if (objstack == 1) { /* just two values, swap them */
+			lua_insert(L, -2);
+			lua_pop(L, lua_gettop(L) - top - 2);
+		} else {                    /* long path */
+			lua_insert(L, top + 1);
+			lua_insert(L, top + 2);
+			lua_pop(L, objstack - 1);
+			objstack = 1;
+		}
+	}
+	*count = 1 + objstack;
+	return 0;
+}
+
 int
 tarantool_lua_utils_init(struct lua_State *L)
 {
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 943840ec0..81e936bee 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -592,6 +592,17 @@ void luaL_iterator_delete(struct luaL_iterator *it);
 
 /* }}} */
 
+/**
+ * A helper to find a Lua function by name and put it
+ * on top of the stack.
+ * Returns 0 in case of succsess and -1 otherwise; in case of
+ * success also uses count[out] argument to return the how many
+ * objects are pushed on to stack.
+ */
+int
+luaT_func_find(struct lua_State *L, const char *name, const char *name_end,
+	       int *count);
+
 int
 tarantool_lua_utils_init(struct lua_State *L);
 
-- 
2.21.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [tarantool-patches] Re: [PATCH v1 2/3] schema: extend _func space to persist lua functions
  2019-05-17 12:33     ` [tarantool-patches] " Kirill Shcherbatov
@ 2019-05-17 15:29       ` Konstantin Osipov
  0 siblings, 0 replies; 9+ messages in thread
From: Konstantin Osipov @ 2019-05-17 15:29 UTC (permalink / raw)
  To: Kirill Shcherbatov; +Cc: tarantool-patches, vdavydov.dev

* Kirill Shcherbatov <kshcherbatov@tarantool.org> [19/05/17 15:37]:
> > 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).
> 
> Thank you for this reference.
> I think we'll need 'is_deterministic' flag soon enough (in scope of
> functional indexes), so I've introduced it.
> To make the process of introducing such options extensible,
> @locker and I decided to put 'is_deterministic' and other new
> flags in the 'opts' map field. I've reused existent opts_decode
> mechanism in scope of this patch.

I wrote earlier: it's better to put as many fields as possible to
columns, not options. 

If you disagree, please put forward your arguments first.

I don't mind options, but htey are hard to use, so should be
avoided if possible.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-05-17 15:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-14 13:29 [PATCH v1 0/3] box: persistent Lua functions Kirill Shcherbatov
2019-05-14 13:29 ` [PATCH v1 1/3] box: refactor box_lua_find helper Kirill Shcherbatov
2019-05-17 12:33   ` [tarantool-patches] " Kirill Shcherbatov
2019-05-14 13:29 ` [PATCH v1 2/3] schema: extend _func space to persist lua functions Kirill Shcherbatov
2019-05-14 18:21   ` [tarantool-patches] " Konstantin Osipov
2019-05-17 12:33     ` [tarantool-patches] " Kirill Shcherbatov
2019-05-17 15:29       ` Konstantin Osipov
2019-05-14 13:29 ` [PATCH v1 3/3] box: extend box.schema.func with persistent folder Kirill Shcherbatov
2019-05-14 18:24   ` [tarantool-patches] " Konstantin Osipov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox