From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH v2 5/9] schema: rework _func system space format
Date: Mon, 10 Jun 2019 15:10:52 +0300 [thread overview]
Message-ID: <20190610121052.wyoncz3l53pg5ngl@esperanza> (raw)
In-Reply-To: <e68feb176c3de81a30e1433a77eb91d75a7207e8.1559822429.git.kshcherbatov@tarantool.org>
On Thu, Jun 06, 2019 at 03:04:01PM +0300, Kirill Shcherbatov wrote:
> This patch updates a _func system space to prepare it for
> working with persistent functions. The format of the _func
> system space is
> [<id> UINT, <owner> UINT, <name> STR, <setuid> UINT,
> <language> STR, <body> STR, <returns> STR,
> <is_deterministic> BOOL, <opts> MAP]
>
> It is preparational step to introduce persistent Lua function
> in Tarantool.
>
> The new <body> field is a string that represents the function
> body, returns is the type of value that it returns, and the
> is_deterministic flag is responsible whether this routine
> deterministic or not (can produce only one result for a given
> list of parameters).
>
> Updated function definition structure, decode operation and
> migration script correspondingly.
>
> Part of #4182
> Needed for #1260
> ---
> src/box/alter.cc | 61 ++++++++++++++++++++++++++++-------
> src/box/bootstrap.snap | Bin 4393 -> 4449 bytes
> src/box/func_def.h | 14 ++++++--
> src/box/lua/schema.lua | 13 ++++++--
> src/box/lua/upgrade.lua | 25 +++++++++++++-
> src/box/schema_def.h | 4 +++
> test/box-py/bootstrap.result | 6 ++--
> test/box/access_misc.result | 6 ++--
> 8 files changed, 108 insertions(+), 21 deletions(-)
>
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 3b57a7d82..11cad77c3 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -2426,26 +2426,45 @@ 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 field_count = tuple_field_count(tuple);
> + 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 (field_count > 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_def_get_ids_from_tuple(tuple, &def->fid, &def->uid);
> - memcpy(def->name, name, len);
> - def->name[len] = 0;
> - if (tuple_field_count(tuple) > BOX_FUNC_FIELD_SETUID)
> + 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 (field_count > BOX_FUNC_FIELD_SETUID)
> def->setuid = tuple_field_u32_xc(tuple, BOX_FUNC_FIELD_SETUID);
> else
> def->setuid = false;
> - if (tuple_field_count(tuple) > BOX_FUNC_FIELD_LANGUAGE) {
> + if (field_count > BOX_FUNC_FIELD_LANGUAGE) {
> const char *language =
> tuple_field_cstr_xc(tuple, BOX_FUNC_FIELD_LANGUAGE);
> def->language = STR2ENUM(func_language, language);
> @@ -2457,6 +2476,26 @@ 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");
> + }
This check should live in the specific func constructor so that we don't
need to touch it when we introduce SQL functions.
> + if (field_count > BOX_FUNC_FIELD_BODY) {
> + assert(field_count > BOX_FUNC_FIELD_OPTS);
> + const char *type =
> + tuple_field_cstr_xc(tuple, BOX_FUNC_FIELD_RETURNS);
> + def->returns = STR2ENUM(field_type, type);
> + if (def->returns == field_type_MAX) {
> + tnt_raise(ClientError, ER_CREATE_FUNCTION, name,
> + "function return value has unknown field type");
> + }
> + def->is_deterministic =
> + tuple_field_bool_xc(tuple, BOX_FUNC_FIELD_IS_DETERMINISTIC);
> + } else {
> + def->returns = FIELD_TYPE_ANY;
> + def->is_deterministic = false;
> + }
> def_guard.is_active = false;
> return def;
> }
> diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap
> index bb8fbeba114b1e72a5585548fb7f22796931d90f..440495684401541854e9d9180f55cc9a4bc45b25 100644
>
> diff --git a/src/box/func_def.h b/src/box/func_def.h
> index 5b52ab498..7a920f65e 100644
> --- a/src/box/func_def.h
> +++ b/src/box/func_def.h
> @@ -32,6 +32,7 @@
> */
>
> #include "trivia/util.h"
> +#include "field_def.h"
> #include <stdbool.h>
>
> /**
> @@ -54,11 +55,17 @@ struct func_def {
> uint32_t fid;
> /** Owner of the function. */
> uint32_t uid;
> + /** Definition of the persistent function. */
> + char *body;
> /**
> * True if the function requires change of user id before
> * invocation.
> */
> bool setuid;
> + /** The type of the returned value. */
> + enum field_type returns;
As discussed verbally, the only reason we need 'returns' now is to
differentiate between multikey and unikey functional indexes and so it
can be either 'array' or something else we don't care what exactly. We
don't check function return value against this field. This doesn't look
right to me.
IMO creating a multikey index judging by a function definition looks
indirect and confusing - after all whether an index is multikey or not
is a property of the index itself or, to be more exact, of the
relationship between the index and the function. That being said, I
think we better drop 'returns' property for now and instead allow to
specify is_multikey flag when creating a functional index, like this:
box.space.my_space:create_index('my_index',
{function = {id = 'my_func', is_multikey = true}})
Let's add 'returns' (and possibly 'args') when we really need to check
function return value.
> + /** Whether this function is deterministic. */
> + bool is_deterministic;
Please try to keep bools together unless it hurts readability as this
reduces the struct size.
next prev parent reply other threads:[~2019-06-10 12:10 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-06 12:03 [PATCH v2 0/9] box: rework functions machinery Kirill Shcherbatov
2019-06-06 12:03 ` [PATCH v2 1/9] box: refactor box_lua_find helper Kirill Shcherbatov
2019-06-10 9:17 ` Vladimir Davydov
2019-06-06 12:03 ` [PATCH v2 2/9] box: move box_module_reload routine to func.c Kirill Shcherbatov
2019-06-10 9:19 ` Vladimir Davydov
2019-06-06 12:03 ` [PATCH v2 3/9] box: rework func cache update machinery Kirill Shcherbatov
2019-06-10 9:44 ` Vladimir Davydov
2019-06-06 12:04 ` [PATCH v2 4/9] box: rework func object as a function frontend Kirill Shcherbatov
2019-06-10 10:32 ` Vladimir Davydov
2019-06-06 12:04 ` [PATCH v2 5/9] schema: rework _func system space format Kirill Shcherbatov
2019-06-10 12:10 ` Vladimir Davydov [this message]
2019-06-06 12:04 ` [PATCH v2 6/9] box: load persistent Lua functions on creation Kirill Shcherbatov
2019-06-10 12:19 ` Vladimir Davydov
2019-06-06 12:04 ` [PATCH v2 7/9] box: sandbox option for persistent functions Kirill Shcherbatov
2019-06-10 14:06 ` Vladimir Davydov
2019-06-10 14:15 ` [tarantool-patches] " Kirill Shcherbatov
2019-06-10 14:41 ` Vladimir Davydov
2019-06-06 12:04 ` [PATCH v2 8/9] box: implement lua_port dump to region and to Lua Kirill Shcherbatov
2019-06-10 14:24 ` Vladimir Davydov
2019-06-06 12:04 ` [PATCH v2 9/9] box: export _func functions with box.func folder Kirill Shcherbatov
2019-06-10 15:18 ` Vladimir Davydov
2019-06-10 9:14 ` [PATCH v2 0/9] box: rework functions machinery Vladimir Davydov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190610121052.wyoncz3l53pg5ngl@esperanza \
--to=vdavydov.dev@gmail.com \
--cc=kshcherbatov@tarantool.org \
--cc=tarantool-patches@freelists.org \
--subject='Re: [PATCH v2 5/9] schema: rework _func system space format' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox