[PATCH v2 5/9] schema: rework _func system space format
Vladimir Davydov
vdavydov.dev at gmail.com
Mon Jun 10 15:10:52 MSK 2019
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.
More information about the Tarantool-patches
mailing list