From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [tarantool-patches] Re: [PATCH v5 3/3] box: introduce func_index References: <38c49e4a186f40139488ad743fb0c37e458d5a65.1564134114.git.kshcherbatov@tarantool.org> <20190726121536.GA4659@esperanza> From: Kirill Shcherbatov Message-ID: Date: Fri, 26 Jul 2019 16:58:25 +0300 MIME-Version: 1.0 In-Reply-To: <20190726121536.GA4659@esperanza> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit To: tarantool-patches@freelists.org, Vladimir Davydov Cc: kostja@tarantool.org List-ID: >> +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.