Re: [tarantool-patches] Re: [PATCH v1 1/1] box: make functional index creation transactional

[Vladimir Davydov <vdavydov.dev@gmail.com>]

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.