From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B85FB25225 for ; Wed, 10 Jul 2019 15:26:40 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id DonDSdo9LKGs for ; Wed, 10 Jul 2019 15:26:40 -0400 (EDT) Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 770E825209 for ; Wed, 10 Jul 2019 15:26:40 -0400 (EDT) Date: Wed, 10 Jul 2019 22:26:38 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH v2 09/12] box: introduce Lua persistent functions Message-ID: <20190710192638.GK5619@atlas> References: <2f63c2c9773c448399d5b8fac08e282dabf57619.1562756438.git.kshcherbatov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2f63c2c9773c448399d5b8fac08e282dabf57619.1562756438.git.kshcherbatov@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Kirill Shcherbatov Cc: tarantool-patches@freelists.org, korablev@tarantool.org * Kirill Shcherbatov [19/07/10 14:02]: > diff --git a/src/box/func.c b/src/box/func.c > index 8227527ec..8d93a83b2 100644 > --- a/src/box/func.c > +++ b/src/box/func.c > @@ -34,7 +34,9 @@ > #include "assoc.h" > #include "lua/utils.h" > #include "lua/call.h" > +#include "lua/lua_sql.h" > #include "error.h" > +#include "sql.h" Why do you need these includes? Please watch for unnecessary includes when you do a self-review. > #include "diag.h" > #include "port.h" > #include "schema.h" > @@ -385,11 +387,18 @@ struct func * > func_new(struct func_def *def) > { > struct func *func; > - if (def->language == FUNC_LANGUAGE_C) { > + switch (def->language) { > + case FUNC_LANGUAGE_C: > func = func_c_new(def); > - } else { > - assert(def->language == FUNC_LANGUAGE_LUA); > + break; > + case FUNC_LANGUAGE_LUA: > func = func_lua_new(def); > + break; > + case FUNC_LANGUAGE_SQL_BUILTIN: > + func = func_sql_builtin_new(def); > + break; > + default: > + unreachable(); > } > if (func == NULL) > return NULL; > @@ -416,8 +425,13 @@ static struct func_vtab func_c_vtab; > static struct func * > func_c_new(struct func_def *def) > { > - (void) def; > assert(def->language == FUNC_LANGUAGE_C); > + if (def->body != NULL || def->is_sandboxed) { > + diag_set(ClientError, ER_CREATE_FUNCTION, def->name, > + "body and is_sandboxed options are not compatible " > + "with C language"); > + return NULL; > + } > struct func_c *func = (struct func_c *) malloc(sizeof(struct func_c)); > if (func == NULL) { > diag_set(OutOfMemory, sizeof(*func), "malloc", "func"); > diff --git a/src/box/func_def.c b/src/box/func_def.c > index 2b135e2d7..fb9f77df8 100644 > --- a/src/box/func_def.c > +++ b/src/box/func_def.c > +static int > +func_persistent_lua_load(struct func_lua *func) > +{ > + int rc = -1; > + int top = lua_gettop(tarantool_L); > + struct region *region = &fiber()->gc; > + size_t region_svp = region_used(region); > + const char *load_pref = "return "; load_prefix, pref is ambiguous The patch is LGTM. I would take another couple of hours to review it carefully and tidy up - including the comments. Unfortunately I don't have them the moment. I hope Vova can take the patch from here. -- Konstantin Osipov, Moscow, Russia