[PATCH v4 2/6] box: rework box_lua_{call, eval} to use input port
Kirill Shcherbatov
kshcherbatov at tarantool.org
Sun Jun 23 16:57:53 MSK 2019
Re-factor box_lua_call and box_lua_eval so that they don't take
call_request. This approach is more scalable: in case of a
functional index, the user expects to see a tuple with field
names so we should be able to pass not only raw msgpack, but
also a tuple to a Lua call so we need an universal way to pass
arguments to _call methods.
To pass a tuple msgpack introduced a new port_msgpack: the port
class with dump_lua method.
A new method get_msgpack returns a content of a port as a
msgpack data. The lifecycle of the returned value is
implementation-specific: it may either be returned directly from
the port, in which case the data will stay alive as long as the
port is alive, or it may be allocated on the fiber()->gc, in
which case the caller is responsible for cleaning up.
Needed for #4182, #1260
---
src/box/call.c | 57 +++++++++++++++++++++++++++-------
src/box/call.h | 4 ---
src/box/execute.c | 1 +
src/box/func.c | 23 ++++++++++++--
src/box/func.h | 8 +++--
src/box/func_def.h | 7 +++++
src/box/lua/call.c | 71 +++++++++++++++++++++++++------------------
src/box/lua/call.h | 8 +++--
src/box/lua/execute.c | 8 +++--
src/box/lua/execute.h | 4 ++-
src/box/lua/misc.cc | 19 ++++++++++--
src/box/port.c | 3 +-
src/box/port.h | 15 +++++++++
src/lib/core/port.h | 38 ++++++++++++++++++++---
14 files changed, 202 insertions(+), 64 deletions(-)
diff --git a/src/box/call.c b/src/box/call.c
index 56da53fb3..573a0d698 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -43,6 +43,39 @@
#include "small/obuf.h"
#include "tt_static.h"
+static const struct port_vtab port_msgpack_vtab;
+
+void
+port_msgpack_create(struct port *base, const char *data, uint32_t data_sz)
+{
+ struct port_msgpack *port_msgpack = (struct port_msgpack *) base;
+ memset(port_msgpack, 0, sizeof(*port_msgpack));
+ port_msgpack->vtab = &port_msgpack_vtab;
+ port_msgpack->data = data;
+ port_msgpack->data_sz = data_sz;
+}
+
+static const char *
+port_msgpack_get_msgpack(struct port *base, uint32_t *size)
+{
+ struct port_msgpack *port = (struct port_msgpack *) base;
+ assert(port->vtab == &port_msgpack_vtab);
+ *size = port->data_sz;
+ return port->data;
+}
+
+extern void
+port_msgpack_dump_lua(struct port *base, struct lua_State *L, bool is_flat);
+
+static const struct port_vtab port_msgpack_vtab = {
+ .dump_msgpack = NULL,
+ .dump_msgpack_16 = NULL,
+ .dump_lua = port_msgpack_dump_lua,
+ .dump_plain = NULL,
+ .get_msgpack = port_msgpack_get_msgpack,
+ .destroy = NULL,
+};
+
/**
* Find a function by name and check "EXECUTE" permissions.
*
@@ -106,27 +139,22 @@ access_check_func(const char *name, uint32_t name_len, struct func **funcp)
}
static int
-box_c_call(struct func *func, struct call_request *request, struct port *port)
+box_c_call(struct func *func, struct port *args, struct port *ret)
{
assert(func != NULL && func->def->language == FUNC_LANGUAGE_C);
- /* Create a call context */
- port_tuple_create(port);
- box_function_ctx_t ctx = { port };
-
/* Clear all previous errors */
diag_clear(&fiber()->diag);
assert(!in_txn()); /* transaction is not started */
/* Call function from the shared library */
- int rc = func_call(func, &ctx, request->args, request->args_end);
+ int rc = func_call(func, args, ret);
func = NULL; /* May be deleted by DDL */
if (rc != 0) {
if (diag_last_error(&fiber()->diag) == NULL) {
/* Stored procedure forget to set diag */
diag_set(ClientError, ER_PROC_C, "unknown error");
}
- port_destroy(port);
return -1;
}
return 0;
@@ -199,10 +227,13 @@ box_process_call(struct call_request *request, struct port *port)
}
int rc;
+ struct port args;
+ port_msgpack_create(&args, request->args,
+ request->args_end - request->args);
if (func && func->def->language == FUNC_LANGUAGE_C) {
- rc = box_c_call(func, request, port);
+ rc = box_c_call(func, &args, port);
} else {
- rc = box_lua_call(request, port);
+ rc = box_lua_call(name, name_len, &args, port);
}
/* Restore the original user */
if (orig_credentials)
@@ -229,7 +260,12 @@ box_process_eval(struct call_request *request, struct port *port)
/* Check permissions */
if (access_check_universe(PRIV_X) != 0)
return -1;
- if (box_lua_eval(request, port) != 0) {
+ struct port args;
+ port_msgpack_create(&args, request->args,
+ request->args_end - request->args);
+ const char *expr = request->expr;
+ uint32_t expr_len = mp_decode_strl(&expr);
+ if (box_lua_eval(expr, expr_len, &args, port) != 0) {
txn_rollback();
return -1;
}
@@ -239,6 +275,5 @@ box_process_eval(struct call_request *request, struct port *port)
txn_rollback();
return -1;
}
-
return 0;
}
diff --git a/src/box/call.h b/src/box/call.h
index 1b54551be..45580bc9d 100644
--- a/src/box/call.h
+++ b/src/box/call.h
@@ -38,10 +38,6 @@ extern "C" {
struct port;
struct call_request;
-struct box_function_ctx {
- struct port *port;
-};
-
/**
* Reload loadable module by name.
*
diff --git a/src/box/execute.c b/src/box/execute.c
index 64ed3d46c..a9ca2e67a 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -108,6 +108,7 @@ const struct port_vtab port_sql_vtab = {
/* .dump_msgpack_16 = */ NULL,
/* .dump_lua = */ port_sql_dump_lua,
/* .dump_plain = */ NULL,
+ /* .get_msgpack = */ NULL,
/* .destroy = */ port_sql_destroy,
};
diff --git a/src/box/func.c b/src/box/func.c
index d1b8879ad..1c37f6523 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -29,11 +29,13 @@
* SUCH DAMAGE.
*/
#include "func.h"
+#include "fiber.h"
#include "trivia/config.h"
#include "assoc.h"
#include "lua/utils.h"
#include "error.h"
#include "diag.h"
+#include "port.h"
#include <dlfcn.h>
/**
@@ -433,21 +435,36 @@ func_load(struct func *func)
}
int
-func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
- const char *args_end)
+func_call(struct func *func, struct port *args, struct port *ret)
{
+ assert(func != NULL && func->def->language == FUNC_LANGUAGE_C);
if (func->func == NULL) {
if (func_load(func) != 0)
return -1;
}
+ struct region *region = &fiber()->gc;
+ size_t region_svp = region_used(region);
+ uint32_t data_sz;
+ const char *data = port_get_msgpack(args, &data_sz);
+ if (data == NULL)
+ return -1;
+
+ port_tuple_create(ret);
+ box_function_ctx_t ctx = { ret };
+
/* Module can be changed after function reload. */
struct module *module = func->module;
assert(module != NULL);
++module->calls;
- int rc = func->func(ctx, args, args_end);
+ int rc = func->func(&ctx, data, data + data_sz);
--module->calls;
module_gc(module);
+ region_truncate(region, region_svp);
+ if (rc != 0) {
+ port_destroy(ret);
+ return -1;
+ }
return rc;
}
diff --git a/src/box/func.h b/src/box/func.h
index fa4d738ab..f6494d064 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -42,6 +42,9 @@
extern "C" {
#endif /* defined(__cplusplus) */
+struct func;
+struct lua_State;
+
/**
* Dynamic shared module.
*/
@@ -104,11 +107,10 @@ void
func_delete(struct func *func);
/**
- * Call stored C function using @a args.
+ * Call function with arguments represented with given args.
*/
int
-func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
- const char *args_end);
+func_call(struct func *func, struct port *args, struct port *ret);
/**
* Reload dynamically loadable module.
diff --git a/src/box/func_def.h b/src/box/func_def.h
index 4c9738aea..ef33cbbaa 100644
--- a/src/box/func_def.h
+++ b/src/box/func_def.h
@@ -90,6 +90,13 @@ func_def_cmp(struct func_def *def1, struct func_def *def2);
/**
* API of C stored function.
*/
+
+struct port;
+
+struct box_function_ctx {
+ struct port *port;
+};
+
typedef struct box_function_ctx box_function_ctx_t;
typedef int (*box_function_f)(box_function_ctx_t *ctx,
const char *args, const char *args_end);
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 04020ef6f..7cac90f3b 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -36,7 +36,6 @@
#include "lua/utils.h"
#include "lua/msgpack.h"
-#include "box/xrow.h"
#include "box/port.h"
#include "box/lua/tuple.h"
#include "small/obuf.h"
@@ -282,28 +281,31 @@ port_lua_create(struct port *port, struct lua_State *L)
port_lua->ref = -1;
}
+struct execute_lua_ctx {
+ const char *name;
+ uint32_t name_len;
+ struct port *args;
+};
+
static int
execute_lua_call(lua_State *L)
{
- struct call_request *request = (struct call_request *)
- lua_topointer(L, 1);
+ struct execute_lua_ctx *ctx =
+ (struct execute_lua_ctx *) lua_topointer(L, 1);
lua_settop(L, 0); /* clear the stack to simplify the logic below */
- const char *name = request->name;
- uint32_t name_len = mp_decode_strl(&name);
+ const char *name = ctx->name;
+ uint32_t name_len = ctx->name_len;
int oc = 0; /* how many objects are on stack after box_lua_find */
/* Try to find a function by name in Lua */
oc = box_lua_find(L, name, name + name_len);
/* 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, "call: out of stack");
+ int top = lua_gettop(L);
+ port_dump_lua(ctx->args, L, true);
+ int arg_count = lua_gettop(L) - top;
- for (uint32_t i = 0; i < arg_count; i++)
- luamp_decode(L, luaL_msgpack_default, &args);
lua_call(L, arg_count + oc - 1, LUA_MULTRET);
return lua_gettop(L);
}
@@ -311,24 +313,22 @@ execute_lua_call(lua_State *L)
static int
execute_lua_eval(lua_State *L)
{
- struct call_request *request = (struct call_request *)
- lua_topointer(L, 1);
+ struct execute_lua_ctx *ctx =
+ (struct execute_lua_ctx *) lua_topointer(L, 1);
lua_settop(L, 0); /* clear the stack to simplify the logic below */
/* Compile expression */
- const char *expr = request->expr;
- uint32_t expr_len = mp_decode_strl(&expr);
+ const char *expr = ctx->name;
+ uint32_t expr_len = ctx->name_len;
if (luaL_loadbuffer(L, expr, expr_len, "=eval")) {
diag_set(LuajitError, lua_tostring(L, -1));
luaT_error(L);
}
/* Unpack arguments */
- const char *args = request->args;
- uint32_t arg_count = mp_decode_array(&args);
- luaL_checkstack(L, arg_count, "eval: out of stack");
- for (uint32_t i = 0; i < arg_count; i++)
- luamp_decode(L, luaL_msgpack_default, &args);
+ int top = lua_gettop(L);
+ port_dump_lua(ctx->args, L, true);
+ int arg_count = lua_gettop(L) - top;
/* Call compiled code */
lua_call(L, arg_count, LUA_MULTRET);
@@ -429,37 +429,48 @@ static const struct port_vtab port_lua_vtab = {
.dump_msgpack_16 = port_lua_dump_16,
.dump_lua = NULL,
.dump_plain = port_lua_dump_plain,
+ .get_msgpack = NULL,
.destroy = port_lua_destroy,
};
static inline int
-box_process_lua(struct call_request *request, struct port *base,
- lua_CFunction handler)
+box_process_lua(lua_CFunction handler, struct execute_lua_ctx *ctx,
+ struct port *ret)
{
lua_State *L = lua_newthread(tarantool_L);
int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX);
- port_lua_create(base, L);
- ((struct port_lua *) base)->ref = coro_ref;
+ port_lua_create(ret, L);
+ ((struct port_lua *) ret)->ref = coro_ref;
lua_pushcfunction(L, handler);
- lua_pushlightuserdata(L, request);
+ lua_pushlightuserdata(L, ctx);
if (luaT_call(L, 1, LUA_MULTRET) != 0) {
- port_lua_destroy(base);
+ port_lua_destroy(ret);
return -1;
}
return 0;
}
int
-box_lua_call(struct call_request *request, struct port *port)
+box_lua_call(const char *name, uint32_t name_len,
+ struct port *args, struct port *ret)
{
- return box_process_lua(request, port, execute_lua_call);
+ struct execute_lua_ctx ctx;
+ ctx.name = name;
+ ctx.name_len = name_len;
+ ctx.args = args;
+ return box_process_lua(execute_lua_call, &ctx, ret);
}
int
-box_lua_eval(struct call_request *request, struct port *port)
+box_lua_eval(const char *expr, uint32_t expr_len,
+ struct port *args, struct port *ret)
{
- return box_process_lua(request, port, execute_lua_eval);
+ struct execute_lua_ctx ctx;
+ ctx.name = expr;
+ ctx.name_len = expr_len;
+ ctx.args = args;
+ return box_process_lua(execute_lua_eval, &ctx, ret);
}
static int
diff --git a/src/box/lua/call.h b/src/box/lua/call.h
index 0542123da..d03bcd0f8 100644
--- a/src/box/lua/call.h
+++ b/src/box/lua/call.h
@@ -31,6 +31,8 @@
* SUCH DAMAGE.
*/
+#include <stdint.h>
+
#if defined(__cplusplus)
extern "C" {
#endif /* defined(__cplusplus) */
@@ -48,10 +50,12 @@ struct call_request;
* (implementation of 'CALL' command code).
*/
int
-box_lua_call(struct call_request *request, struct port *port);
+box_lua_call(const char *name, uint32_t name_len,
+ struct port *args, struct port *ret);
int
-box_lua_eval(struct call_request *request, struct port *port);
+box_lua_eval(const char *expr, uint32_t expr_len,
+ struct port *args, struct port *ret);
#if defined(__cplusplus)
} /* extern "C" */
diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
index 239aba47b..10fa35de2 100644
--- a/src/box/lua/execute.c
+++ b/src/box/lua/execute.c
@@ -39,8 +39,10 @@ lua_sql_get_metadata(struct sql_stmt *stmt, struct lua_State *L,
}
void
-port_sql_dump_lua(struct port *port, struct lua_State *L)
+port_sql_dump_lua(struct port *port, struct lua_State *L, bool is_flat)
{
+ (void) is_flat;
+ assert(is_flat == false);
assert(port->vtab == &port_sql_vtab);
struct sql *db = sql_get();
struct sql_stmt *stmt = ((struct port_sql *)port)->stmt;
@@ -49,7 +51,7 @@ port_sql_dump_lua(struct port *port, struct lua_State *L)
lua_createtable(L, 0, 2);
lua_sql_get_metadata(stmt, L, column_count);
lua_setfield(L, -2, "metadata");
- port_tuple_vtab.dump_lua(port, L);
+ port_tuple_vtab.dump_lua(port, L, false);
lua_setfield(L, -2, "rows");
} else {
assert(((struct port_tuple *)port)->size == 0);
@@ -262,7 +264,7 @@ lbox_execute(struct lua_State *L)
if (sql_prepare_and_execute(sql, length, bind, bind_count, &port,
&fiber()->gc) != 0)
return luaT_error(L);
- port_dump_lua(&port, L);
+ port_dump_lua(&port, L, false);
port_destroy(&port);
return 1;
}
diff --git a/src/box/lua/execute.h b/src/box/lua/execute.h
index bc4df19f5..23e193fa4 100644
--- a/src/box/lua/execute.h
+++ b/src/box/lua/execute.h
@@ -30,6 +30,8 @@
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
+#include <stdbool.h>
+
struct port;
struct sql_bind;
struct lua_State;
@@ -42,7 +44,7 @@ struct lua_State;
* @param L Lua stack.
*/
void
-port_sql_dump_lua(struct port *port, struct lua_State *L);
+port_sql_dump_lua(struct port *port, struct lua_State *L, bool is_flat);
/**
* Parse Lua table of SQL parameters.
diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index a835d3d6f..de9ed407b 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -65,8 +65,10 @@ lbox_encode_tuple_on_gc(lua_State *L, int idx, size_t *p_len)
* but implemented here to eliminate port.c dependency on Lua.
*/
extern "C" void
-port_tuple_dump_lua(struct port *base, struct lua_State *L)
+port_tuple_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
{
+ (void) is_flat;
+ assert(is_flat == false);
struct port_tuple *port = port_tuple(base);
lua_createtable(L, port->size, 0);
struct port_tuple_entry *pe = port->first;
@@ -76,6 +78,19 @@ port_tuple_dump_lua(struct port *base, struct lua_State *L)
}
}
+extern "C" void
+port_msgpack_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
+{
+ (void) is_flat;
+ assert(is_flat == true);
+ struct port_msgpack *port = (struct port_msgpack *) base;
+
+ const char *args = port->data;
+ uint32_t arg_count = mp_decode_array(&args);
+ for (uint32_t i = 0; i < arg_count; i++)
+ luamp_decode(L, luaL_msgpack_default, &args);
+}
+
/* }}} */
/** {{{ Lua/C implementation of index:select(): used only by Vinyl **/
@@ -113,7 +128,7 @@ lbox_select(lua_State *L)
* table always crashed the first (can't be fixed with pcall).
* https://github.com/tarantool/tarantool/issues/1182
*/
- port_dump_lua(&port, L);
+ port_dump_lua(&port, L, false);
port_destroy(&port);
return 1; /* lua table with tuples */
}
diff --git a/src/box/port.c b/src/box/port.c
index 99046449a..7f552bcfe 100644
--- a/src/box/port.c
+++ b/src/box/port.c
@@ -125,7 +125,7 @@ port_tuple_dump_msgpack(struct port *base, struct obuf *out)
}
extern void
-port_tuple_dump_lua(struct port *base, struct lua_State *L);
+port_tuple_dump_lua(struct port *base, struct lua_State *L, bool is_flat);
void
port_init(void)
@@ -145,5 +145,6 @@ const struct port_vtab port_tuple_vtab = {
.dump_msgpack_16 = port_tuple_dump_msgpack_16,
.dump_lua = port_tuple_dump_lua,
.dump_plain = NULL,
+ .get_msgpack = NULL,
.destroy = port_tuple_destroy,
};
diff --git a/src/box/port.h b/src/box/port.h
index f18803660..db93f8eea 100644
--- a/src/box/port.h
+++ b/src/box/port.h
@@ -32,6 +32,7 @@
*/
#include "trivia/util.h"
#include <port.h>
+#include <stdbool.h>
#if defined(__cplusplus)
extern "C" {
@@ -80,6 +81,20 @@ port_tuple_create(struct port *port);
int
port_tuple_add(struct port *port, struct tuple *tuple);
+/** Port implementation used for storing raw data. */
+struct port_msgpack {
+ const struct port_vtab *vtab;
+ const char *data;
+ uint32_t data_sz;
+};
+
+static_assert(sizeof(struct port_msgpack) <= sizeof(struct port),
+ "sizeof(struct port_msgpack) must be <= sizeof(struct port)");
+
+/** Initialize a port to dump raw data. */
+void
+port_msgpack_create(struct port *port, const char *data, uint32_t data_sz);
+
/** Port for storing the result of a Lua CALL/EVAL. */
struct port_lua {
const struct port_vtab *vtab;
diff --git a/src/lib/core/port.h b/src/lib/core/port.h
index 8ace40fc5..09a026df5 100644
--- a/src/lib/core/port.h
+++ b/src/lib/core/port.h
@@ -30,6 +30,7 @@
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
+#include <stdbool.h>
#include <stdint.h>
#if defined(__cplusplus)
@@ -62,8 +63,17 @@ struct port_vtab {
* header. Used by the legacy Tarantool 1.6 format.
*/
int (*dump_msgpack_16)(struct port *port, struct obuf *out);
- /** Dump the content of a port to Lua stack. */
- void (*dump_lua)(struct port *port, struct lua_State *L);
+ /**
+ * Dump the content of a port to a given Lua stack.
+ * When is_flat == true is specified, the data is dumped
+ * directly to Lua stack, item-by-item. Otherwise, a
+ * result table is created. The is_flat == true mode
+ * follows Lua functions convention to pass arguments
+ * and return a results, while is_flat == false follows
+ * Tarantool's :select convention when the table of
+ * results is returned.
+ */
+ void (*dump_lua)(struct port *port, struct lua_State *L, bool is_flat);
/**
* Dump a port content as a plain text into a buffer,
* allocated inside.
@@ -74,6 +84,20 @@ struct port_vtab {
* @retval not nil Plain text.
*/
const char *(*dump_plain)(struct port *port, uint32_t *size);
+ /**
+ * Get the content of a port as a msgpack data.
+ * By an obsolete convention, C stored routines expect
+ * msgpack data as input format for its arguments.
+ * This API is also usefull to process a function
+ * returned value as msgpack data in memory.
+ * The lifecycle of the returned value is
+ * implementation-specific: it may either be returned
+ * directly from the port, in which case the data will
+ * stay alive as long as the port is alive, or it may be
+ * allocated on the fiber()->gc, in which case the caller
+ * is responsible for cleaning up.
+ **/
+ const char *(*get_msgpack)(struct port *port, uint32_t *size);
/** Destroy a port and release associated resources. */
void (*destroy)(struct port *port);
};
@@ -109,9 +133,9 @@ port_dump_msgpack_16(struct port *port, struct obuf *out)
}
static inline void
-port_dump_lua(struct port *port, struct lua_State *L)
+port_dump_lua(struct port *port, struct lua_State *L, bool is_flat)
{
- port->vtab->dump_lua(port, L);
+ port->vtab->dump_lua(port, L, is_flat);
}
static inline const char *
@@ -120,6 +144,12 @@ port_dump_plain(struct port *port, uint32_t *size)
return port->vtab->dump_plain(port, size);
}
+static inline const char *
+port_get_msgpack(struct port *port, uint32_t *size)
+{
+ return port->vtab->get_msgpack(port, size);
+}
+
#if defined(__cplusplus)
} /* extern "C" */
#endif /* defined __cplusplus */
--
2.21.0
More information about the Tarantool-patches
mailing list