[tarantool-patches] Re: [PATCH v1 1/1] box: make functional index creation transactional
Vladimir Davydov
vdavydov.dev at gmail.com
Tue Aug 6 16:19:52 MSK 2019
The patch is good, just a nit pick below.
On Mon, Aug 05, 2019 at 06:38:51PM +0300, Kirill Shcherbatov wrote:
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 92f1d5b22..de778eefd 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -4725,12 +4763,11 @@ 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);
> + if (index->def->opts.func_id != func->def->fid) {
> + tnt_raise(ClientError, ER_FUNC_INDEX_PARTS,
> + "functional index function id doesn't match "
> + "the fid defined in index");
Why ER_FUNC_INDEX_PARTS? Why not ER_WRONG_INDEX_OPTIONS like in
the error right above? BTW passing 0 to ER_WRONG_INDEX_OPTIONS looks
confusing.
Also, using "function id" and "fid" in the same error message looks ugly.
Please rephrase it. Something like this maybe:
"Function ids defined in _index and _func_index don't match"
Also, please add a test case covering this particular error.
More information about the Tarantool-patches
mailing list