[PATCH v1 1/1] box: make functional index creation transactional
Vladimir Davydov
vdavydov.dev at gmail.com
Mon Aug 5 13:20:45 MSK 2019
On Fri, Aug 02, 2019 at 04:05:06PM +0300, Kirill Shcherbatov wrote:
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 92f1d5b22..b92ed056f 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -296,6 +314,16 @@ index_def_new_from_tuple(struct tuple *tuple, struct space *space)
> auto index_def_guard = make_scoped_guard([=] { index_def_delete(index_def); });
> index_def_check_xc(index_def, space_name(space));
> space_check_index_def_xc(space, index_def);
> + /*
> + * In case of functional index key definition,
> + * initialize a function pointer, when possible
> + * (but not during recovery).
> + */
Please improve the comment so that it's clear what problem we solve
here.
> + struct func *func = NULL;
> + if (for_func_index && (func = func_by_id(opts.func_id)) != NULL) {
> + func_index_check_func(func);
> + index_def_set_func(index_def, func);
> + }
> if (index_def->iid == 0 && space->sequence != NULL)
> index_def_check_sequence(index_def, space->sequence_fieldno,
> space->sequence_path,
> @@ -4725,13 +4753,8 @@ on_replace_dd_func_index(struct trigger *trigger, void *event)
> space = space_cache_find_xc(space_id);
> index = index_find_xc(space, index_id);
> func = func_cache_find(fid);
> - if (func->def->language != FUNC_LANGUAGE_LUA ||
> - func->def->body == NULL || !func->def->is_deterministic ||
> - !func->def->is_sandboxed) {
> - tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS, 0,
> - "referenced function doesn't satisfy "
> - "functional index function constraints");
> - }
> + func_index_check_func(func);
> + assert(index->def->opts.func_id == func->def->fid);
Technically, an assertion isn't quite correct here, because one may
insert tuples into _index and _func_index that have mismatching function
ids. Please replace it with an error and make sure it's covered by a
test.
> } else if (old_tuple != NULL && new_tuple == NULL) {
> uint32_t space_id = tuple_field_u32_xc(old_tuple,
> BOX_FUNC_INDEX_FIELD_SPACE_ID);
> @@ -4746,6 +4769,13 @@ on_replace_dd_func_index(struct trigger *trigger, void *event)
> "functional index", "alter");
> }
>
> + /**
> + * Index is already initialized for corresponding
> + * function. Index rebuild is not required.
> + */
> + if (index_def_get_func(index->def) == func)
> + return;
> +
> alter = alter_space_new(space);
> auto scoped_guard = make_scoped_guard([=] {alter_space_delete(alter);});
> alter_space_move_indexes(alter, 0, index->def->iid);
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index 65017d391..89a4d25bc 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -1102,11 +1102,18 @@ box.schema.index.create = function(space_id, name, options)
> local sequence_proxy = space_sequence_alter_prepare(format, parts, options,
> space_id, iid,
> space.name, name)
> - _index:insert{space_id, iid, name, options.type, index_opts, parts}
> - space_sequence_alter_commit(sequence_proxy)
> - if index_opts.func ~= nil then
> - local _func_index = box.space[box.schema.FUNC_INDEX_ID]
> - _func_index:insert{space_id, iid, index_opts.func}
> + local create_f = function ()
> + _index:insert{space_id, iid, name, options.type, index_opts, parts}
> + space_sequence_alter_commit(sequence_proxy)
> + if index_opts.func ~= nil then
> + local _func_index = box.space[box.schema.FUNC_INDEX_ID]
> + _func_index:insert{space_id, iid, index_opts.func}
> + end
> + end
> + if not box.is_in_txn() then
> + box.atomic(create_f)
> + else
> + create_f()
All you need to do in the scope of this issue is allow to call
index.create from a user transaction. There's a separate issue for
making all Lua DDL operations transactional (see #4348) and it should be
fixed in a unified manner for all DDL-related functions (with the aid of
a function decorator that would add box.begin/commit if necessary, I
guess).
Please don't patch index.create/alter in this patch. The tests should
just check that those functions work fine in a user transaction. The
rest should be done in a separate patch in the scope of #4348.
More information about the Tarantool-patches
mailing list