[tarantool-patches] Re: [PATCH v3 1/7] box: cleanup key_def virtual extract_key setter
Kirill Shcherbatov
kshcherbatov at tarantool.org
Wed Apr 3 19:22:56 MSK 2019
> 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<false, false, false>,
- tuple_extract_key_slowpath<true, false, false>,
- tuple_extract_key_slowpath<false, true, false>,
- tuple_extract_key_slowpath<true, true, false>,
- tuple_extract_key_slowpath<false, false, true>,
- tuple_extract_key_slowpath<true, false, true>,
- tuple_extract_key_slowpath<false, true, true>,
- tuple_extract_key_slowpath<true, true, true>
-};
-
/**
* Initialize tuple_extract_key() and tuple_extract_key_raw()
*/
-void
-key_def_set_extract_func(struct key_def *key_def)
+template<bool contains_sequential_parts, bool has_optional_parts>
+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<true>;
- key_def->tuple_extract_key_raw =
- tuple_extract_key_sequential_raw<true>;
- } else {
- key_def->tuple_extract_key =
- tuple_extract_key_sequential<false>;
- key_def->tuple_extract_key_raw =
- tuple_extract_key_sequential_raw<false>;
- }
+ 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
+ <has_optional_parts>;
+ def->tuple_extract_key_raw = tuple_extract_key_sequential_raw
+ <has_optional_parts>;
} 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
+ <contains_sequential_parts,
+ has_optional_parts, false>;
+ def->tuple_extract_key_raw = tuple_extract_key_slowpath_raw
+ <has_optional_parts, false>;
}
- 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<true, true>;
+}
+
+template<bool contains_sequential_parts, bool has_optional_parts>
+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
+ <contains_sequential_parts,
+ has_optional_parts, true>;
+ def->tuple_extract_key_raw = tuple_extract_key_slowpath_raw
+ <has_optional_parts, true>;
+}
+
+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<false, false>(key_def);
+ } else if (!contains_sequential_parts && has_optional_parts) {
+ key_def_set_extract_func_plain<false, true>(key_def);
+ } else if (contains_sequential_parts && !has_optional_parts) {
+ key_def_set_extract_func_plain<true, false>(key_def);
} else {
- key_def->tuple_extract_key_raw =
- tuple_extract_key_slowpath_raw<true, false>;
+ assert(contains_sequential_parts && has_optional_parts);
+ key_def_set_extract_func_plain<true, true>(key_def);
}
} else {
- if (key_def->has_json_paths) {
- key_def->tuple_extract_key_raw =
- tuple_extract_key_slowpath_raw<false, true>;
+ if (!contains_sequential_parts && !has_optional_parts) {
+ key_def_set_extract_func_json<false, false>(key_def);
+ } else if (!contains_sequential_parts && has_optional_parts) {
+ key_def_set_extract_func_json<false, true>(key_def);
+ } else if (contains_sequential_parts && !has_optional_parts) {
+ key_def_set_extract_func_json<true, false>(key_def);
} else {
- key_def->tuple_extract_key_raw =
- tuple_extract_key_slowpath_raw<false, false>;
+ assert(contains_sequential_parts && has_optional_parts);
+ key_def_set_extract_func_json<true, true>(key_def);
}
}
}
--
2.21.0
More information about the Tarantool-patches
mailing list