Re: [tarantool-patches] Re: [PATCH v4 4/4] box: introduce functional indexes

[Kirill Shcherbatov <kshcherbatov@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.