From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Tue, 6 Aug 2019 16:19:52 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] Re: [PATCH v1 1/1] box: make functional
 index creation transactional
Message-ID: <20190806131952.GB4242@esperanza>
References: <1056e3dd6be0f9d03c6feff3651d41884cbd91c6.1564751087.git.kshcherbatov@tarantool.org>
 <20190805102045.GA4242@esperanza>
 <103f6c68-bf55-8459-bcc1-8ec6ccc2d475@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <103f6c68-bf55-8459-bcc1-8ec6ccc2d475@tarantool.org>
To: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

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.