Re: [tarantool-patches] Re: [PATCH v3 1/7] box: cleanup key_def virtual extract_key setter

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

> 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