From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp37.i.mail.ru (smtp37.i.mail.ru [94.100.177.97]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id C1B23442974 for ; Mon, 30 Mar 2020 12:13:40 +0300 (MSK) From: Chris Sosnin Date: Mon, 30 Mar 2020 12:13:32 +0300 Message-Id: <4ba94f315e947aebf7a723b6e24a6162d8306c98.1585559306.git.k.sosnin@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 1/4] box: replace session_settings modules with a single array List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org, korablev@tarantool.org, tarantool-patches@dev.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__BEGIN = SESSION_SETTING__END, + * ... + * SESSION_SETTING__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)