From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 3 Apr 2019 15:42:15 +0300 From: Vladimir Davydov Subject: Re: [PATCH v3 1/7] box: cleanup key_def virtual extract_key setter Message-ID: <20190403124215.7ngokicqvqceahid@esperanza> References: <781937f7536d5e70438177905f1365789303db1d.1554218695.git.kshcherbatov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <781937f7536d5e70438177905f1365789303db1d.1554218695.git.kshcherbatov@tarantool.org> To: Kirill Shcherbatov Cc: tarantool-patches@freelists.org List-ID: On Tue, Apr 02, 2019 at 06:49:32PM +0300, Kirill Shcherbatov wrote: > 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 | 98 ++++++++++++++++++------------------ > 1 file changed, 49 insertions(+), 49 deletions(-) > > diff --git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc > index 0a8337102..836d4e565 100644 > --- a/src/box/tuple_extract_key.cc > +++ b/src/box/tuple_extract_key.cc > @@ -341,62 +341,62 @@ 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 > + + has_optional_parts, false>; > + 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; > - } else { > - 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 > + + has_optional_parts, true>; > + def->tuple_extract_key_raw = tuple_extract_key_slowpath_raw > + ; > +} > + > +void > +key_def_set_extract_func(struct key_def *key_def) > +{ > + int func_idx = (key_def_contains_sequential_parts(key_def) ? 1 : 0) + > + 2 * (key_def->has_optional_parts ? 1 : 0); > + if (!key_def->has_json_paths) { > + void (*set_extract_func[])(struct key_def *) = { > + key_def_set_extract_func_plain, > + key_def_set_extract_func_plain, > + key_def_set_extract_func_plain, > + key_def_set_extract_func_plain, > + }; > + set_extract_func[func_idx](key_def); > } else { > - if (key_def->has_json_paths) { > - key_def->tuple_extract_key_raw = > - tuple_extract_key_slowpath_raw; > - } else { > - key_def->tuple_extract_key_raw = > - tuple_extract_key_slowpath_raw; > - } > + void (*set_extract_func[])(struct key_def *) = { > + key_def_set_extract_func_json, > + key_def_set_extract_func_json, > + key_def_set_extract_func_json, > + key_def_set_extract_func_json, > + }; > + set_extract_func[func_idx](key_def); > } > } 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.