Re: [tarantool-patches] Re: [PATCH v5 3/3] box: introduce func_index

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

>> +static inline void
>> +index_def_set_func(struct index_def *def, struct func *func)
>> +{
>> +	assert(def->opts.func_id > 0 &&
>> +	       def->key_def->for_func_index && def->key_def->for_func_index);
> 
> for_func_index && for_func_index
Fixed.

> 
>> +	/*
>> +	 * Set func_index_func for functional index key
>> +	 * definition. It is used in key_list module to extract
>> +	 * a key for given tuple.
>> +	 */
>> +	def->key_def->func_index_func = func;
>> +	def->cmp_def->func_index_func = NULL;
> 
> Hmm, why is cmp_def->func_index_func set to NULL?
Because we never use func pointer in cmp_def. Kostya supposed do not initialize a pointer
when it is not reasonable. 

>> +    _index:insert{_func_index.id, 0, 'primary', 'tree', {unique = true},
>> +                  {{0, 'unsigned'}, {1, 'unsigned'}, {2, 'unsigned'}}}
> 
> Why do we need to make all the three fields parts of the primary index?
> Shouldn't space and index id be enough?
Okey, fixed.

> Other than that, the patch is fine by me.