[tarantool-patches] Re: [PATCH v3 2/6] box: rework box_lua_{call, eval} to use input port
Kirill Shcherbatov
kshcherbatov at tarantool.org
Wed Jun 19 18:51:15 MSK 2019
>> module_gc(module);
>> + 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(out_port);
>> + return -1;
>> + }
>
>Shouldn't you call region_truncate() here to clean up after the function
>and possibly port_get_msgpack()?
No, the func_c_call implementation must clean region by it's own.
======================================================
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.
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 | 30 +++++++++++++++---
14 files changed, 194 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 e81cc32aa..a91c939fc 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 8de7401fa..3058d8dac 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -61,8 +61,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;
@@ -72,6 +74,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 **/
@@ -109,7 +124,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..e25e8fc5e 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,13 @@ 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.
+ */
+ 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 +80,16 @@ 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.
+ * 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 +125,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 +136,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