From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [tarantool-patches] Re: [PATCH v3 1/7] box: cleanup key_def virtual extract_key setter References: <781937f7536d5e70438177905f1365789303db1d.1554218695.git.kshcherbatov@tarantool.org> <20190403124215.7ngokicqvqceahid@esperanza> From: Kirill Shcherbatov Message-ID: <7f1136a6-055a-7713-cb6c-91a51a53ec8d@tarantool.org> Date: Wed, 3 Apr 2019 19:22:56 +0300 MIME-Version: 1.0 In-Reply-To: <20190403124215.7ngokicqvqceahid@esperanza> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit To: tarantool-patches@freelists.org, Vladimir Davydov List-ID: > Yeah, this looks better. However, I'd also replace set_extract_func with > plain if-else-if - it would take only a couple lines longer, but look > more straightforward IMO. I don't mind. Updated version is on the branch. ====================================================== This patch inspired by 082cffca4dba attempts to simplify setting appropriate tuple_extract_key pointer for plain and json indexes in key_def_set_extract_func routine. Being split to plain and json blocks this code becomes easier to understand and extend. In further patches we need to introduce is_multikey branch and without this refactoring required amendments turn the key_def_set_extract_func code into a mess. Needed for #1257 --- src/box/tuple_extract_key.cc | 97 +++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 45 deletions(-) diff --git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc index 0a8337102..28ca80cf8 100644 --- a/src/box/tuple_extract_key.cc +++ b/src/box/tuple_extract_key.cc @@ -341,61 +341,68 @@ tuple_extract_key_slowpath_raw(const char *data, const char *data_end, return key; } -static const tuple_extract_key_t extract_key_slowpath_funcs[] = { - tuple_extract_key_slowpath, - tuple_extract_key_slowpath, - tuple_extract_key_slowpath, - tuple_extract_key_slowpath, - tuple_extract_key_slowpath, - tuple_extract_key_slowpath, - tuple_extract_key_slowpath, - tuple_extract_key_slowpath -}; - /** * Initialize tuple_extract_key() and tuple_extract_key_raw() */ -void -key_def_set_extract_func(struct key_def *key_def) +template +static void +key_def_set_extract_func_plain(struct key_def *def) { - if (key_def_is_sequential(key_def)) { - if (key_def->has_optional_parts) { - assert(key_def->is_nullable); - key_def->tuple_extract_key = - tuple_extract_key_sequential; - key_def->tuple_extract_key_raw = - tuple_extract_key_sequential_raw; - } else { - key_def->tuple_extract_key = - tuple_extract_key_sequential; - key_def->tuple_extract_key_raw = - tuple_extract_key_sequential_raw; - } + assert(!def->has_json_paths); + if (key_def_is_sequential(def)) { + assert(contains_sequential_parts || def->part_count == 1); + def->tuple_extract_key = tuple_extract_key_sequential + ; + def->tuple_extract_key_raw = tuple_extract_key_sequential_raw + ; } else { - int func_idx = - (key_def_contains_sequential_parts(key_def) ? 1 : 0) + - 2 * (key_def->has_optional_parts ? 1 : 0) + - 4 * (key_def->has_json_paths ? 1 : 0); - key_def->tuple_extract_key = - extract_key_slowpath_funcs[func_idx]; - assert(!key_def->has_optional_parts || key_def->is_nullable); + def->tuple_extract_key = tuple_extract_key_slowpath + ; + def->tuple_extract_key_raw = tuple_extract_key_slowpath_raw + ; } - if (key_def->has_optional_parts) { - assert(key_def->is_nullable); - if (key_def->has_json_paths) { - key_def->tuple_extract_key_raw = - tuple_extract_key_slowpath_raw; +} + +template +static void +key_def_set_extract_func_json(struct key_def *def) +{ + assert(def->has_json_paths); + def->tuple_extract_key = tuple_extract_key_slowpath + ; + def->tuple_extract_key_raw = tuple_extract_key_slowpath_raw + ; +} + +void +key_def_set_extract_func(struct key_def *key_def) +{ + bool contains_sequential_parts = + key_def_contains_sequential_parts(key_def); + bool has_optional_parts = key_def->has_optional_parts; + if (!key_def->has_json_paths) { + if (!contains_sequential_parts && !has_optional_parts) { + key_def_set_extract_func_plain(key_def); + } else if (!contains_sequential_parts && has_optional_parts) { + key_def_set_extract_func_plain(key_def); + } else if (contains_sequential_parts && !has_optional_parts) { + key_def_set_extract_func_plain(key_def); } else { - key_def->tuple_extract_key_raw = - tuple_extract_key_slowpath_raw; + assert(contains_sequential_parts && has_optional_parts); + key_def_set_extract_func_plain(key_def); } } else { - if (key_def->has_json_paths) { - key_def->tuple_extract_key_raw = - tuple_extract_key_slowpath_raw; + if (!contains_sequential_parts && !has_optional_parts) { + key_def_set_extract_func_json(key_def); + } else if (!contains_sequential_parts && has_optional_parts) { + key_def_set_extract_func_json(key_def); + } else if (contains_sequential_parts && !has_optional_parts) { + key_def_set_extract_func_json(key_def); } else { - key_def->tuple_extract_key_raw = - tuple_extract_key_slowpath_raw; + assert(contains_sequential_parts && has_optional_parts); + key_def_set_extract_func_json(key_def); } } } -- 2.21.0