[Tarantool-patches] [PATCH 1/4] box: replace session_settings modules with a single array

[Chris Sosnin <k.sosnin@tarantool.org>]

Currently we have an array of modules and each module has
its own array of session_settings. This patch merges these
into one array, as long as it turned out, that we cannot
represent every setting as a part of some module. This
change is also needed for implementing binary search for
_session_settings space.

Part of #4711, #4712
---
 src/box/session_settings.c | 121 ++++++++++++++-----------------------
 src/box/session_settings.h |  50 +++++++++------
 src/box/sql/build.c        |  79 +++++++-----------------
 3 files changed, 101 insertions(+), 149 deletions(-)

diff --git a/src/box/session_settings.c b/src/box/session_settings.c
index 37d2a3e85..2ecf71f2d 100644
--- a/src/box/session_settings.c
+++ b/src/box/session_settings.c
@@ -38,8 +38,20 @@
 #include "xrow.h"
 #include "sql.h"
 
-struct session_setting_module
-	session_setting_modules[session_setting_type_MAX] = {};
+struct session_setting session_settings[SESSION_SETTING_COUNT] = {};
+
+/** Corresponding names of session settings. */
+const char *session_setting_strs[SESSION_SETTING_COUNT] = {
+	"sql_default_engine",
+	"sql_defer_foreign_keys",
+	"sql_full_column_names",
+	"sql_full_metadata",
+	"sql_parser_debug",
+	"sql_recursive_triggers",
+	"sql_reverse_unordered_selects",
+	"sql_select_debug",
+	"sql_vdbe_debug",
+};
 
 struct session_settings_index {
 	/** Base index. Must be the first member. */
@@ -61,12 +73,7 @@ struct session_settings_iterator {
 	 * a format for selected tuples.
 	 */
 	struct tuple_format *format;
-	/**
-	 * ID of the current session settings module in the global
-	 * list of the modules.
-	 */
-	int module_id;
-	/** ID of the setting in current module. */
+	/** ID of the setting. */
 	int setting_id;
 	/** Decoded key. */
 	char *key;
@@ -86,46 +93,40 @@ session_settings_iterator_free(struct iterator *ptr)
 }
 
 static int
-session_settings_next_in_module(const struct session_setting_module *module,
-				int *sid, const char *key, bool is_eq,
-				bool is_including)
+session_settings_next(int *sid, const char *key, bool is_eq, bool is_including)
 {
 	int i = *sid;
-	int count = module->setting_count;
-	if (i >= count)
+	if (i >= SESSION_SETTING_COUNT)
 		return -1;
 	if (key == NULL)
 		return 0;
 	assert(i >= 0);
-	const char **name = &module->settings[i];
-	for (; i < count; ++i, ++name) {
-		int cmp = strcmp(*name, key);
+	for (; i < SESSION_SETTING_COUNT; ++i) {
+		const char *name = session_setting_strs[i];
+		int cmp = strcmp(name, key);
 		if ((cmp == 0 && is_including) ||
 		    (cmp > 0 && !is_eq)) {
 			*sid = i;
 			return 0;
 		}
 	}
-	*sid = count;
+	*sid = SESSION_SETTING_COUNT;
 	return -1;
 }
 
 static int
-session_settings_prev_in_module(const struct session_setting_module *module,
-				int *sid, const char *key, bool is_eq,
-				bool is_including)
+session_settings_prev(int *sid, const char *key, bool is_eq, bool is_including)
 {
 	int i = *sid;
-	int count = module->setting_count;
 	if (i < 0)
 		return -1;
 	if (key == NULL)
 		return 0;
-	if (i >= count)
-		i = count - 1;
-	const char **name = &module->settings[i];
-	for (; i >= 0; --i, --name) {
-		int cmp = strcmp(*name, key);
+	if (i >= SESSION_SETTING_COUNT)
+		i = SESSION_SETTING_COUNT - 1;
+	for (; i >= 0; --i) {
+		const char *name = session_setting_strs[i];
+		int cmp = strcmp(name, key);
 		if ((cmp == 0 && is_including) ||
 		    (cmp < 0 && !is_eq)) {
 			*sid = i;
@@ -141,27 +142,19 @@ session_settings_iterator_next(struct iterator *iterator, struct tuple **result)
 {
 	struct session_settings_iterator *it =
 		(struct session_settings_iterator *)iterator;
-	int mid = it->module_id, sid = it->setting_id;
-	struct session_setting_module *module;
+	int sid = it->setting_id;
 	const char *key = it->key;
 	bool is_including = it->is_including, is_eq = it->is_eq;
 	bool is_found = false;
-	for (; mid < session_setting_type_MAX; ++mid, sid = 0) {
-		module = &session_setting_modules[mid];
-		if (session_settings_next_in_module(module, &sid, key, is_eq,
-						    is_including) == 0) {
-			is_found = true;
-			break;
-		}
-	}
-	it->module_id = mid;
+	if (session_settings_next(&sid, key, is_eq, is_including) == 0)
+		is_found = true;
 	it->setting_id = sid + 1;
 	if (!is_found) {
 		*result = NULL;
 		return 0;
 	}
 	const char *mp_pair, *mp_pair_end;
-	module->get(sid, &mp_pair, &mp_pair_end);
+	session_settings[sid].get(sid, &mp_pair, &mp_pair_end);
 	*result = box_tuple_new(it->format, mp_pair, mp_pair_end);
 	return *result != NULL ? 0 : -1;
 }
@@ -171,27 +164,19 @@ session_settings_iterator_prev(struct iterator *iterator, struct tuple **result)
 {
 	struct session_settings_iterator *it =
 		(struct session_settings_iterator *)iterator;
-	int mid = it->module_id, sid = it->setting_id;
-	struct session_setting_module *module;
+	int sid = it->setting_id;
 	const char *key = it->key;
 	bool is_including = it->is_including, is_eq = it->is_eq;
 	bool is_found = false;
-	for (; mid >= 0; --mid, sid = INT_MAX) {
-		module = &session_setting_modules[mid];
-		if (session_settings_prev_in_module(module, &sid, key, is_eq,
-						    is_including) == 0) {
-			is_found = true;
-			break;
-		}
-	}
-	it->module_id = mid;
+	if (session_settings_prev(&sid, key, is_eq, is_including) == 0)
+		is_found = true;
 	it->setting_id = sid - 1;
 	if (!is_found) {
 		*result = NULL;
 		return 0;
 	}
 	const char *mp_pair, *mp_pair_end;
-	module->get(sid, &mp_pair, &mp_pair_end);
+	session_settings[sid].get(sid, &mp_pair, &mp_pair_end);
 	*result = box_tuple_new(it->format, mp_pair, mp_pair_end);
 	return *result != NULL ? 0 : -1;
 }
@@ -239,14 +224,10 @@ session_settings_index_create_iterator(struct index *base,
 	it->format = index->format;
 	if (!iterator_type_is_reverse(type)) {
 		it->base.next = session_settings_iterator_next;
-		it->module_id = 0;
 		it->setting_id = 0;
 	} else {
 		it->base.next = session_settings_iterator_prev;
-		it->module_id = session_setting_type_MAX - 1;
-		struct session_setting_module *module =
-			&session_setting_modules[it->module_id];
-		it->setting_id = module->setting_count - 1;
+		it->setting_id = SESSION_SETTING_COUNT - 1;
 	}
 	return (struct iterator *)it;
 }
@@ -262,20 +243,14 @@ session_settings_index_get(struct index *base, const char *key,
 	uint32_t len;
 	key = mp_decode_str(&key, &len);
 	key = tt_cstr(key, len);
-	struct session_setting_module *module = &session_setting_modules[0];
-	struct session_setting_module *end = module + session_setting_type_MAX;
 	int sid = 0;
-	for (; module < end; ++module, sid = 0) {
-		if (session_settings_next_in_module(module, &sid, key, true,
-						    true) == 0)
-			goto found;
+	if (session_settings_next(&sid, key, true, true) != 0) {
+		*result = NULL;
+		return 0;
 	}
-	*result = NULL;
-	return 0;
-found:;
 	const char *mp_pair;
 	const char *mp_pair_end;
-	module->get(sid, &mp_pair, &mp_pair_end);
+	session_settings[sid].get(sid, &mp_pair, &mp_pair_end);
 	*result = box_tuple_new(index->format, mp_pair, mp_pair_end);
 	return *result != NULL ? 0 : -1;
 }
@@ -370,17 +345,11 @@ session_settings_space_execute_update(struct space *space, struct txn *txn,
 	}
 	key = mp_decode_str(&key, &key_len);
 	key = tt_cstr(key, key_len);
-	struct session_setting_module *module = &session_setting_modules[0];
-	struct session_setting_module *end = module + session_setting_type_MAX;
-	for (; module < end; ++module, sid = 0) {
-		if (session_settings_next_in_module(module, &sid, key, true,
-						    true) == 0)
-			goto found;
+	if (session_settings_next(&sid, key, true, true) != 0) {
+		*result = NULL;
+		return 0;
 	}
-	*result = NULL;
-	return 0;
-found:
-	module->get(sid, &old_data, &old_data_end);
+	session_settings[sid].get(sid, &old_data, &old_data_end);
 	new_data = xrow_update_execute(request->tuple, request->tuple_end,
 				       old_data, old_data_end, format,
 				       &new_size, request->index_base,
@@ -402,7 +371,7 @@ found:
 			goto finish;
 		}
 	}
-	if (module->set(sid, new_data) != 0)
+	if (session_settings[sid].set(sid, new_data) != 0)
 		goto finish;
 	rc = 0;
 finish:
diff --git a/src/box/session_settings.h b/src/box/session_settings.h
index 25490a7e3..de24e3c6f 100644
--- a/src/box/session_settings.h
+++ b/src/box/session_settings.h
@@ -30,29 +30,44 @@
  * SUCH DAMAGE.
  */
 
+#include "field_def.h"
+
 /**
- * Session has settings. Settings belong to different subsystems,
- * such as SQL. Each subsystem registers here its session setting
- * type and a set of settings with getter and setter functions.
- * The self-registration of modules allows session setting code
- * not to depend on all the subsystems.
+ * Identifiers of all session settings. The identifier of the
+ * option is equal to its place in the sorted list of session
+ * options.
  *
- * The types should be ordered in alphabetical order, because the
- * type list is used by setting iterators.
+ * It is IMPORTANT that these options are sorted by name. If this
+ * is not the case, the result returned by the _session_settings
+ * space iterator will not be sorted properly.
  */
-enum session_setting_type {
-	SESSION_SETTING_SQL,
-	session_setting_type_MAX,
+enum {
+	SESSION_SETTING_SQL_BEGIN,
+	SESSION_SETTING_SQL_DEFAULT_ENGINE = SESSION_SETTING_SQL_BEGIN,
+	SESSION_SETTING_SQL_DEFER_FOREIGN_KEYS,
+	SESSION_SETTING_SQL_FULL_COLUMN_NAMES,
+	SESSION_SETTING_SQL_FULL_METADATA,
+	SESSION_SETTING_SQL_PARSER_DEBUG,
+	SESSION_SETTING_SQL_RECURSIVE_TRIGGERS,
+	SESSION_SETTING_SQL_REVERSE_UNORDERED_SELECTS,
+	SESSION_SETTING_SQL_SELECT_DEBUG,
+	SESSION_SETTING_SQL_VDBE_DEBUG,
+	SESSION_SETTING_SQL_END,
+	/**
+	 * Follow the pattern for groups of settings:
+	 * SESSION_SETTING_<N>_BEGIN = SESSION_SETTING_<N-1>_END,
+	 * ...
+	 * SESSION_SETTING_<N>_END,
+	 */
+	SESSION_SETTING_COUNT = SESSION_SETTING_SQL_END,
 };
 
-struct session_setting_module {
+struct session_setting {
 	/**
-	 * An array of setting names. All of them should have the
-	 * same prefix.
+	 * Setting's value type. Used for error checking and
+	 * reporting only.
 	 */
-	const char **settings;
-	/** Count of settings. */
-	int setting_count;
+	enum field_type field_type;
 	/**
 	 * Get a MessagePack encoded pair [name, value] for a
 	 * setting having index @a id. Index is from the settings
@@ -67,4 +82,5 @@ struct session_setting_module {
 	int (*set)(int id, const char *mp_value);
 };
 
-extern struct session_setting_module session_setting_modules[];
+extern struct session_setting session_settings[SESSION_SETTING_COUNT];
+extern const char *session_setting_strs[SESSION_SETTING_COUNT];
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 7511fad37..a00da31f9 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3322,40 +3322,6 @@ sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
 	return 0;
 }
 
-/**
- * Identifiers of all SQL session setings. The identifier of the
- * option is equal to its place in the sorted list of session
- * options of current module.
- *
- * It is IMPORTANT that these options are sorted by name. If this
- * is not the case, the result returned by the _session_settings
- * space iterator will not be sorted properly.
- */
-enum {
-	SQL_SESSION_SETTING_DEFAULT_ENGINE = 0,
-	SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS,
-	SQL_SESSION_SETTING_FULL_COLUMN_NAMES,
-	SQL_SESSION_SETTING_FULL_METADATA,
-	SQL_SESSION_SETTING_PARSER_DEBUG,
-	SQL_SESSION_SETTING_RECURSIVE_TRIGGERS,
-	SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS,
-	SQL_SESSION_SETTING_SELECT_DEBUG,
-	SQL_SESSION_SETTING_VDBE_DEBUG,
-	sql_session_setting_MAX,
-};
-
-static const char *sql_session_setting_strs[sql_session_setting_MAX] = {
-	"sql_default_engine",
-	"sql_defer_foreign_keys",
-	"sql_full_column_names",
-	"sql_full_metadata",
-	"sql_parser_debug",
-	"sql_recursive_triggers",
-	"sql_reverse_unordered_selects",
-	"sql_select_debug",
-	"sql_vdbe_debug",
-};
-
 /**
  * A local structure that allows to establish a connection between
  * parameter and its field type and mask, if it has one.
@@ -3373,24 +3339,24 @@ struct sql_option_metadata
  * It is IMPORTANT that these options sorted by name.
  */
 static struct sql_option_metadata sql_session_opts[] = {
-	/** SQL_SESSION_SETTING_DEFAULT_ENGINE */
+	/** SESSION_SETTING_SQL_DEFAULT_ENGINE */
 	{FIELD_TYPE_STRING, 0},
-	/** SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS */
+	/** SESSION_SETTING_SQL_DEFER_FOREIGN_KEYS */
 	{FIELD_TYPE_BOOLEAN, SQL_DeferFKs},
-	/** SQL_SESSION_SETTING_FULL_COLUMN_NAMES */
+	/** SESSION_SETTING_SQL_FULL_COLUMN_NAMES */
 	{FIELD_TYPE_BOOLEAN, SQL_FullColNames},
-	/** SQL_SESSION_SETTING_FULL_METADATA */
+	/** SESSION_SETTING_SQL_FULL_METADATA */
 	{FIELD_TYPE_BOOLEAN, SQL_FullMetadata},
-	/** SQL_SESSION_SETTING_PARSER_DEBUG */
+	/** SESSION_SETTING_SQL_PARSER_DEBUG */
 	{FIELD_TYPE_BOOLEAN, SQL_SqlTrace | PARSER_TRACE_FLAG},
-	/** SQL_SESSION_SETTING_RECURSIVE_TRIGGERS */
+	/** SESSION_SETTING_SQL_RECURSIVE_TRIGGERS */
 	{FIELD_TYPE_BOOLEAN, SQL_RecTriggers},
-	/** SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS */
+	/** SESSION_SETTING_SQL_REVERSE_UNORDERED_SELECTS */
 	{FIELD_TYPE_BOOLEAN, SQL_ReverseOrder},
-	/** SQL_SESSION_SETTING_SELECT_DEBUG */
+	/** SESSION_SETTING_SQL_SELECT_DEBUG */
 	{FIELD_TYPE_BOOLEAN,
 	 SQL_SqlTrace | SQL_SelectTrace | SQL_WhereTrace},
-	/** SQL_SESSION_SETTING_VDBE_DEBUG */
+	/** SESSION_SETTING_SQL_VDBE_DEBUG */
 	{FIELD_TYPE_BOOLEAN,
 	 SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace},
 };
@@ -3398,12 +3364,12 @@ static struct sql_option_metadata sql_session_opts[] = {
 static void
 sql_session_setting_get(int id, const char **mp_pair, const char **mp_pair_end)
 {
-	assert(id >= 0 && id < sql_session_setting_MAX);
+	assert(id >= SESSION_SETTING_SQL_BEGIN && id < SESSION_SETTING_SQL_END);
 	struct session *session = current_session();
 	uint32_t flags = session->sql_flags;
 	struct sql_option_metadata *opt = &sql_session_opts[id];
 	uint32_t mask = opt->mask;
-	const char *name = sql_session_setting_strs[id];
+	const char *name = session_setting_strs[id];
 	size_t name_len = strlen(name);
 	size_t engine_len;
 	const char *engine;
@@ -3416,7 +3382,7 @@ sql_session_setting_get(int id, const char **mp_pair, const char **mp_pair_end)
 	if (is_bool) {
 		size += mp_sizeof_bool(true);
 	} else {
-		assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE);
+		assert(id == SESSION_SETTING_SQL_DEFAULT_ENGINE);
 		engine = sql_storage_engine_strs[session->sql_default_engine];
 		engine_len = strlen(engine);
 		size += mp_sizeof_str(engine_len);
@@ -3452,7 +3418,7 @@ sql_set_boolean_option(int id, bool value)
 		session->sql_flags |= option->mask;
 	else
 		session->sql_flags &= ~option->mask;
-	if (id == SQL_SESSION_SETTING_PARSER_DEBUG) {
+	if (id == SESSION_SETTING_SQL_PARSER_DEBUG) {
 		if (value)
 			sqlParserTrace(stdout, "parser: ");
 		else
@@ -3466,7 +3432,7 @@ static int
 sql_set_string_option(int id, const char *value)
 {
 	assert(sql_session_opts[id].field_type = FIELD_TYPE_STRING);
-	assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE);
+	assert(id == SESSION_SETTING_SQL_DEFAULT_ENGINE);
 	(void)id;
 	enum sql_storage_engine engine = STR2ENUM(sql_storage_engine, value);
 	if (engine == sql_storage_engine_MAX) {
@@ -3480,7 +3446,7 @@ sql_set_string_option(int id, const char *value)
 static int
 sql_session_setting_set(int id, const char *mp_value)
 {
-	assert(id >= 0 && id < sql_session_setting_MAX);
+	assert(id >= SESSION_SETTING_SQL_BEGIN && id < SESSION_SETTING_SQL_END);
 	enum mp_type mtype = mp_typeof(*mp_value);
 	enum field_type stype = sql_session_opts[id].field_type;
 	uint32_t len;
@@ -3500,17 +3466,18 @@ sql_session_setting_set(int id, const char *mp_value)
 		unreachable();
 	}
 	diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE,
-		 sql_session_setting_strs[id], field_type_strs[stype]);
+		 session_setting_strs[id], field_type_strs[stype]);
 	return -1;
 }
 
 void
 sql_session_settings_init()
 {
-	struct session_setting_module *module =
-		&session_setting_modules[SESSION_SETTING_SQL];
-	module->settings = sql_session_setting_strs;
-	module->setting_count = sql_session_setting_MAX;
-	module->get = sql_session_setting_get;
-	module->set = sql_session_setting_set;
+	for (int id = SESSION_SETTING_SQL_BEGIN; id < SESSION_SETTING_SQL_END;
+	     ++id) {
+		struct session_setting *setting = &session_settings[id];
+		setting->field_type = sql_session_opts[id].field_type;
+		setting->get = sql_session_setting_get;
+		setting->set = sql_session_setting_set;
+	}
 }
-- 
2.21.1 (Apple Git-122.3)