From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Kirill Shcherbatov Subject: [PATCH v3 2/6] box: rework box_lua_{call, eval} to use input port Date: Thu, 13 Jun 2019 17:08:22 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: tarantool-patches@freelists.org, vdavydov.dev@gmail.com Cc: Kirill Shcherbatov List-ID: 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 | 46 ++++++---------- src/box/call.h | 4 -- src/box/execute.c | 1 + src/box/func.c | 27 ++++++++-- src/box/func.h | 11 ++-- src/box/lua/call.c | 121 +++++++++++++++++++++++++++++++----------- src/box/lua/call.h | 8 ++- src/box/lua/execute.c | 7 +-- src/box/lua/execute.h | 4 +- src/box/lua/misc.cc | 5 +- src/box/port.c | 3 +- src/box/port.h | 15 ++++++ src/lib/core/port.h | 15 ++++-- 13 files changed, 184 insertions(+), 83 deletions(-) diff --git a/src/box/call.c b/src/box/call.c index 56da53fb3..1ab3993da 100644 --- a/src/box/call.c +++ b/src/box/call.c @@ -105,33 +105,6 @@ access_check_func(const char *name, uint32_t name_len, struct func **funcp) return 0; } -static int -box_c_call(struct func *func, struct call_request *request, struct port *port) -{ - 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); - 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; -} - int box_module_reload(const char *name) { @@ -199,11 +172,15 @@ box_process_call(struct call_request *request, struct port *port) } int rc; + struct port in_port; + port_msgpack_create(&in_port, request->args, + request->args_end - request->args); if (func && func->def->language == FUNC_LANGUAGE_C) { - rc = box_c_call(func, request, port); + rc = func_call(func, &in_port, port); } else { - rc = box_lua_call(request, port); + rc = box_lua_call(name, name_len, &in_port, port); } + port_destroy(&in_port); /* Restore the original user */ if (orig_credentials) fiber_set_user(fiber(), orig_credentials); @@ -229,16 +206,23 @@ 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 in_port; + port_msgpack_create(&in_port, 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, &in_port, port) != 0) { + port_destroy(&in_port); txn_rollback(); return -1; } if (in_txn()) { diag_set(ClientError, ER_FUNCTION_TX_ACTIVE); + port_destroy(&in_port); txn_rollback(); return -1; } - + port_destroy(&in_port); 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..740fad3d7 100644 --- a/src/box/func.c +++ b/src/box/func.c @@ -34,6 +34,9 @@ #include "lua/utils.h" #include "error.h" #include "diag.h" +#include "fiber.h" +#include "port.h" +#include "txn.h" #include /** @@ -433,21 +436,39 @@ 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 *in_port, struct port *out_port) { + assert(func != NULL && func->def->language == FUNC_LANGUAGE_C); + /* Transaction is not started. */ + assert(!in_txn()); if (func->func == NULL) { if (func_load(func) != 0) return -1; } + uint32_t args_sz; + const char *args = port_get_msgpack(in_port, &args_sz); + if (args == NULL) + return -1; + + port_tuple_create(out_port); + box_function_ctx_t ctx = { out_port }; + /* 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, args, args + args_sz); --module->calls; 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; + } return rc; } diff --git a/src/box/func.h b/src/box/func.h index fa4d738ab..d38a2b283 100644 --- a/src/box/func.h +++ b/src/box/func.h @@ -42,6 +42,12 @@ extern "C" { #endif /* defined(__cplusplus) */ +struct port; + +struct box_function_ctx { + struct port *port; +}; + /** * Dynamic shared module. */ @@ -104,11 +110,10 @@ void func_delete(struct func *func); /** - * Call stored C function using @a args. + * Call function with arguments represented with given in_port. */ 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 *in_port, struct port *out_port); /** * Reload dynamically loadable module. diff --git a/src/box/lua/call.c b/src/box/lua/call.c index 04020ef6f..e32845095 100644 --- a/src/box/lua/call.c +++ b/src/box/lua/call.c @@ -266,6 +266,55 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg, return root.size; } +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 void +port_msgpack_dump_lua(struct port *base, struct lua_State *L, bool is_flat) +{ + assert(is_flat == true); + struct port_msgpack *port = (struct port_msgpack *) base; + assert(port->vtab == &port_msgpack_vtab); + + 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); +} + +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; +} + +static void +port_msgpack_destroy(struct port *base) +{ + (void) base; +} + +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 = port_msgpack_destroy, +}; + static const struct port_vtab port_lua_vtab; void @@ -282,28 +331,31 @@ port_lua_create(struct port *port, struct lua_State *L) port_lua->ref = -1; } +struct lua_process_ctx { + const char *name; + uint32_t name_len; +}; + static int execute_lua_call(lua_State *L) { - struct call_request *request = (struct call_request *) - lua_topointer(L, 1); + struct port *in_port = (struct port *) lua_topointer(L, 1); + struct lua_process_ctx *ctx = + (struct lua_process_ctx *) lua_topointer(L, 2); 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(in_port, 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 +363,23 @@ 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 port *in_port = (struct port *) lua_topointer(L, 1); + struct lua_process_ctx *ctx = + (struct lua_process_ctx *) lua_topointer(L, 2); 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(in_port, L, true); + int arg_count = lua_gettop(L) - top; /* Call compiled code */ lua_call(L, arg_count, LUA_MULTRET); @@ -429,37 +480,47 @@ 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, void *arg, struct port *in_port, + struct port *out_port) { 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(out_port, L); + ((struct port_lua *) out_port)->ref = coro_ref; lua_pushcfunction(L, handler); - lua_pushlightuserdata(L, request); - if (luaT_call(L, 1, LUA_MULTRET) != 0) { - port_lua_destroy(base); + lua_pushlightuserdata(L, in_port); + lua_pushlightuserdata(L, arg); + if (luaT_call(L, 2, LUA_MULTRET) != 0) { + port_lua_destroy(out_port); 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 *in_port, struct port *out_port) { - return box_process_lua(request, port, execute_lua_call); + struct lua_process_ctx ctx; + ctx.name = name; + ctx.name_len = name_len; + return box_process_lua(execute_lua_call, &ctx, in_port, out_port); } int -box_lua_eval(struct call_request *request, struct port *port) +box_lua_eval(const char *expr, uint32_t expr_len, + struct port *in_port, struct port *out_port) { - return box_process_lua(request, port, execute_lua_eval); + struct lua_process_ctx ctx; + ctx.name = expr; + ctx.name_len = expr_len; + return box_process_lua(execute_lua_eval, &ctx, in_port, out_port); } static int diff --git a/src/box/lua/call.h b/src/box/lua/call.h index 0542123da..1801d5090 100644 --- a/src/box/lua/call.h +++ b/src/box/lua/call.h @@ -35,6 +35,8 @@ extern "C" { #endif /* defined(__cplusplus) */ +#include + struct lua_State; void @@ -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 *in_port, struct port *out_port); int -box_lua_eval(struct call_request *request, struct port *port); +box_lua_eval(const char *expr, uint32_t expr_len, + struct port *in_port, struct port *out_port); #if defined(__cplusplus) } /* extern "C" */ diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c index 239aba47b..187bdb73d 100644 --- a/src/box/lua/execute.c +++ b/src/box/lua/execute.c @@ -39,8 +39,9 @@ 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) { + 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 +50,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, is_flat); lua_setfield(L, -2, "rows"); } else { assert(((struct port_tuple *)port)->size == 0); @@ -262,7 +263,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 + 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..15681c377 100644 --- a/src/box/lua/misc.cc +++ b/src/box/lua/misc.cc @@ -61,8 +61,9 @@ 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) { + assert(is_flat == false); struct port_tuple *port = port_tuple(base); lua_createtable(L, port->size, 0); struct port_tuple_entry *pe = port->first; @@ -109,7 +110,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..4c0c3cf7a 100644 --- a/src/box/port.h +++ b/src/box/port.h @@ -32,6 +32,7 @@ */ #include "trivia/util.h" #include +#include #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 row 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..2f1bc5ed1 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 #include #if defined(__cplusplus) @@ -63,7 +64,7 @@ struct port_vtab { */ 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); + 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 +75,8 @@ 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. */ + const char *(*get_msgpack)(struct port *port, uint32_t *size); /** Destroy a port and release associated resources. */ void (*destroy)(struct port *port); }; @@ -109,9 +112,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 +123,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