From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <kshcherbatov@tarantool.org>
Subject: Re: [tarantool-patches] Re: [PATCH v4 4/4] box: introduce functional
 indexes
References: <cover.1563953154.git.kshcherbatov@tarantool.org>
 <e244e92d21226c8a869d9d87987ccc9cf21af803.1563953154.git.kshcherbatov@tarantool.org>
 <20190724215618.GC11790@atlas>
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Message-ID: <e3d493ee-6b55-aaae-1fe6-ddb7dc5a9144@tarantool.org>
Date: Thu, 25 Jul 2019 11:33:32 +0300
MIME-Version: 1.0
In-Reply-To: <20190724215618.GC11790@atlas>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
To: tarantool-patches@freelists.org, Konstantin Osipov <kostja@tarantool.org>
Cc: vdavydov.dev@gmail.com
List-ID: <tarantool-patches.dev.tarantool.org>

>> +	/**
>> +	 * Count of parts in functional index defintion.
>> +	 * All functional_part_count key_part(s) of an
>> +	 * initialized key def instance have func != NULL pointer.
>> +	 * != 0 iff it is functional index definition.
>> +	*/
>> +	uint32_t functional_part_count;
> 
> I don't get this comment, we don't store func pointer in key_part.
> Why do you need this at all? part_count should be enough.
In case of secondary key it's definition consists of two parts (produced with
key_def_merge) - functional index key parts that refer to hint_t memory and
tuple parts are specified in primary key.
   
>> +		if (index_opts->functional_fid > 0) {
>> +			lua_pushnumber(L, index_opts->functional_fid);
>> +			lua_setfield(L, -2, "func_id");
>> +		}
> 
> Why not function name?
Function name is not defined during recovery. I may print it in other cases, if it is ok for you

if (index_opts->func_id > 0) {
	lua_pushstring(L, "func");
	lua_newtable(L);

	lua_pushnumber(L, index_opts->func_id);
	lua_setfield(L, -2, "fid");

	lua_pushboolean(L, index_opts->is_multikey);
	lua_setfield(L, -2, "is_multikey");

	struct func *func = func_by_id(index_opts->func_id);
	if (func != NULL) {
		lua_pushstring(L, func->def->name);
		lua_setfield(L, -2, "name");
	}

	lua_settable(L, -3);
}

>> +static char *
>> +tuple_extract_key_stub(struct tuple *tuple, struct key_def *key_def,
>> +			     int multikey_idx, uint32_t *key_size)
>> +{
>> +	(void)tuple; (void)key_def; (void)multikey_idx; (void)key_size;
>> +	unreachable();
>> +	return NULL;
>> +}
>> +
>> +static char *
>> +tuple_extract_key_raw_stub(const char *data, const char *data_end,
>> +			   struct key_def *key_def, int multikey_idx,
>> +			   uint32_t *key_size)
>> +{
>> +	(void)data; (void)data_end;
>> +	(void)key_def; (void)multikey_idx; (void)key_size;
>> +	unreachable();
>> +	return NULL;
>> +}
> 
> Why do you need these?
> 

This code is really unreachable now. However this produce assertion error in case of
further invalid refactoring.