* [Tarantool-patches] [PATCH 1/4] box: replace session_settings modules with a single array
2020-03-30 9:13 [Tarantool-patches] [PATCH 0/4] session settings fixes Chris Sosnin
@ 2020-03-30 9:13 ` Chris Sosnin
2020-04-03 13:32 ` Nikita Pettik
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 2/4] box: add binary search for _session_settings space Chris Sosnin
` (5 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Chris Sosnin @ 2020-03-30 9:13 UTC (permalink / raw)
To: v.shpilevoy, korablev, tarantool-patches
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)
^ permalink raw reply [flat|nested] 26+ messages in thread* [Tarantool-patches] [PATCH 2/4] box: add binary search for _session_settings space
2020-03-30 9:13 [Tarantool-patches] [PATCH 0/4] session settings fixes Chris Sosnin
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 1/4] box: replace session_settings modules with a single array Chris Sosnin
@ 2020-03-30 9:13 ` Chris Sosnin
2020-04-03 14:00 ` Nikita Pettik
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 3/4] box: provide a user friendly frontend for accessing session settings Chris Sosnin
` (4 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Chris Sosnin @ 2020-03-30 9:13 UTC (permalink / raw)
To: v.shpilevoy, korablev, tarantool-patches
As it is mentioned in implementation, it is important
that _session_settings options are sorted by name, so
there is no need in linear lookup and we can replace it
with binary search.
Closes #4712
---
src/box/session_settings.c | 97 +++++++++++++++++--
...1-access-settings-from-any-frontend.result | 37 ++++---
...access-settings-from-any-frontend.test.lua | 28 ++++--
3 files changed, 133 insertions(+), 29 deletions(-)
diff --git a/src/box/session_settings.c b/src/box/session_settings.c
index 2ecf71f2d..5e4a50427 100644
--- a/src/box/session_settings.c
+++ b/src/box/session_settings.c
@@ -81,6 +81,8 @@ struct session_settings_iterator {
bool is_eq;
/** True if the iterator should include equal keys. */
bool is_including;
+ /** True if the iterator is pointing to an existing setting */
+ bool is_set;
};
static void
@@ -92,6 +94,72 @@ session_settings_iterator_free(struct iterator *ptr)
free(it);
}
+static int
+session_settings_set_forward(int *sid, const char *key, bool is_eq,
+ bool is_including)
+{
+ int low = 0, high = SESSION_SETTING_COUNT - 1;
+ if (key == NULL)
+ return 0;
+ while (low <= high) {
+ int index = (high + low) / 2;
+ const char *name = session_setting_strs[index];
+ int cmp = strcmp(name, key);
+ if (cmp == 0) {
+ if (is_including) {
+ *sid = index;
+ return 0;
+ }
+ *sid = ++index;
+ return index < SESSION_SETTING_COUNT ? 0 : -1;
+ }
+ if (cmp < 0)
+ low = index + 1;
+ else
+ high = index - 1;
+ }
+ if (is_eq) {
+ *sid = SESSION_SETTING_COUNT;
+ return -1;
+ }
+ assert(low > high);
+ *sid = low;
+ return low < SESSION_SETTING_COUNT ? 0 : -1;
+}
+
+static int
+session_settings_set_reverse(int *sid, const char *key, bool is_eq,
+ bool is_including)
+{
+ int low = 0, high = SESSION_SETTING_COUNT - 1;
+ if (key == NULL)
+ return 0;
+ while (low <= high) {
+ int index = (high + low) / 2;
+ const char *name = session_setting_strs[index];
+ int cmp = strcmp(name, key);
+ if (cmp == 0) {
+ if (is_including) {
+ *sid = index;
+ return 0;
+ }
+ *sid = --index;
+ return index >= 0 ? 0 : -1;
+ }
+ if (cmp < 0)
+ low = index + 1;
+ else
+ high = index - 1;
+ }
+ if (is_eq) {
+ *sid = SESSION_SETTING_COUNT;
+ return -1;
+ }
+ assert(low > high);
+ *sid = high;
+ return high >= 0 ? 0 : -1;
+}
+
static int
session_settings_next(int *sid, const char *key, bool is_eq, bool is_including)
{
@@ -145,9 +213,15 @@ session_settings_iterator_next(struct iterator *iterator, struct tuple **result)
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;
- if (session_settings_next(&sid, key, is_eq, is_including) == 0)
- is_found = true;
+ bool is_found;
+ if (!it->is_set) {
+ it->is_set = true;
+ is_found = session_settings_set_forward(&sid, key, is_eq,
+ is_including) == 0;
+ } else {
+ is_found = session_settings_next(&sid, key, is_eq,
+ is_including) == 0;
+ }
it->setting_id = sid + 1;
if (!is_found) {
*result = NULL;
@@ -167,9 +241,15 @@ session_settings_iterator_prev(struct iterator *iterator, struct tuple **result)
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;
- if (session_settings_prev(&sid, key, is_eq, is_including) == 0)
- is_found = true;
+ bool is_found;
+ if (!it->is_set) {
+ it->is_set = true;
+ is_found = session_settings_set_reverse(&sid, key, is_eq,
+ is_including) == 0;
+ } else {
+ is_found = session_settings_prev(&sid, key, is_eq,
+ is_including) == 0;
+ }
it->setting_id = sid - 1;
if (!is_found) {
*result = NULL;
@@ -221,6 +301,7 @@ session_settings_index_create_iterator(struct index *base,
it->is_eq = type == ITER_EQ || type == ITER_REQ;
it->is_including = it->is_eq || type == ITER_GE || type == ITER_ALL ||
type == ITER_LE;
+ it->is_set = false;
it->format = index->format;
if (!iterator_type_is_reverse(type)) {
it->base.next = session_settings_iterator_next;
@@ -244,7 +325,7 @@ session_settings_index_get(struct index *base, const char *key,
key = mp_decode_str(&key, &len);
key = tt_cstr(key, len);
int sid = 0;
- if (session_settings_next(&sid, key, true, true) != 0) {
+ if (session_settings_set_forward(&sid, key, true, true) != 0) {
*result = NULL;
return 0;
}
@@ -345,7 +426,7 @@ session_settings_space_execute_update(struct space *space, struct txn *txn,
}
key = mp_decode_str(&key, &key_len);
key = tt_cstr(key, key_len);
- if (session_settings_next(&sid, key, true, true) != 0) {
+ if (session_settings_set_forward(&sid, key, true, true) != 0) {
*result = NULL;
return 0;
}
diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result
index ed340d053..bae77192e 100644
--- a/test/box/gh-4511-access-settings-from-any-frontend.result
+++ b/test/box/gh-4511-access-settings-from-any-frontend.result
@@ -82,6 +82,12 @@ function check_sorting(ss, ts, key)
for _, it in pairs(iterators_list) do
local view_space = ss:select({key}, {iterator = it})
local test_space = ts:select({key}, {iterator = it})
+ if #view_space ~= #test_space then
+ return {
+ err = 'bad sorting', type = it, exp = test_space,
+ got = view_space
+ }
+ end
for key, value in pairs(view_space) do
if test_space[key].name ~= value.name then
return {
@@ -111,32 +117,37 @@ check_sorting(s, t, 'sql_d')
check_sorting(s, t, 'sql_v')
| ---
| ...
-check_sorting(s, t, 'sql_defer_foreign_keys')
+check_sorting(s, t, 'z')
| ---
| ...
-
-t:drop()
+check_sorting(s, t, 'sql_parser_debuf')
+ | ---
+ | ...
+check_sorting(s, t, 'sql_parser_debuh')
| ---
| ...
--- Check get() method of session_settings space.
-s:get({'sql_defer_foreign_keys'})
+name = nil
| ---
- | - ['sql_defer_foreign_keys', false]
| ...
-s:get({'sql_recursive_triggers'})
+err = nil
| ---
- | - ['sql_recursive_triggers', true]
| ...
-s:get({'sql_reverse_unordered_selects'})
+for _, tuple in t:pairs() do \
+ name = tuple.name \
+ err = check_sorting(s, t, name) \
+ if err then \
+ break \
+ end \
+end
| ---
- | - ['sql_reverse_unordered_selects', false]
| ...
-s:get({'sql_default_engine'})
+err and {err, name}
| ---
- | - ['sql_default_engine', 'memtx']
+ | - null
| ...
-s:get({'abcd'})
+
+t:drop()
| ---
| ...
diff --git a/test/box/gh-4511-access-settings-from-any-frontend.test.lua b/test/box/gh-4511-access-settings-from-any-frontend.test.lua
index 366874998..b243be15e 100644
--- a/test/box/gh-4511-access-settings-from-any-frontend.test.lua
+++ b/test/box/gh-4511-access-settings-from-any-frontend.test.lua
@@ -36,6 +36,12 @@ function check_sorting(ss, ts, key)
for _, it in pairs(iterators_list) do
local view_space = ss:select({key}, {iterator = it})
local test_space = ts:select({key}, {iterator = it})
+ if #view_space ~= #test_space then
+ return {
+ err = 'bad sorting', type = it, exp = test_space,
+ got = view_space
+ }
+ end
for key, value in pairs(view_space) do
if test_space[key].name ~= value.name then
return {
@@ -52,17 +58,23 @@ check_sorting(s, t)
check_sorting(s, t, 'abcde')
check_sorting(s, t, 'sql_d')
check_sorting(s, t, 'sql_v')
-check_sorting(s, t, 'sql_defer_foreign_keys')
+check_sorting(s, t, 'z')
+check_sorting(s, t, 'sql_parser_debuf')
+check_sorting(s, t, 'sql_parser_debuh')
+
+name = nil
+err = nil
+for _, tuple in t:pairs() do \
+ name = tuple.name \
+ err = check_sorting(s, t, name) \
+ if err then \
+ break \
+ end \
+end
+err and {err, name}
t:drop()
--- Check get() method of session_settings space.
-s:get({'sql_defer_foreign_keys'})
-s:get({'sql_recursive_triggers'})
-s:get({'sql_reverse_unordered_selects'})
-s:get({'sql_default_engine'})
-s:get({'abcd'})
-
-- Check pairs() method of session_settings space.
t = {}
for key, value in s:pairs() do table.insert(t, {key, value}) end
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 2/4] box: add binary search for _session_settings space
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 2/4] box: add binary search for _session_settings space Chris Sosnin
@ 2020-04-03 14:00 ` Nikita Pettik
2020-04-13 13:40 ` Kirill Yukhin
0 siblings, 1 reply; 26+ messages in thread
From: Nikita Pettik @ 2020-04-03 14:00 UTC (permalink / raw)
To: Chris Sosnin; +Cc: tarantool-patches, v.shpilevoy
On 30 Mar 12:13, Chris Sosnin wrote:
> As it is mentioned in implementation, it is important
> that _session_settings options are sorted by name, so
> there is no need in linear lookup and we can replace it
> with binary search.
>
> Closes #4712
> ---
Still I see no need for this patch. It is unclear which improvements
it introduces except for code complication.
> diff --git a/src/box/session_settings.c b/src/box/session_settings.c
> index 2ecf71f2d..5e4a50427 100644
> --- a/src/box/session_settings.c
> +++ b/src/box/session_settings.c
> @@ -81,6 +81,8 @@ struct session_settings_iterator {
> bool is_eq;
> /** True if the iterator should include equal keys. */
> bool is_including;
> + /** True if the iterator is pointing to an existing setting */
> + bool is_set;
> };
>
> static void
> @@ -92,6 +94,72 @@ session_settings_iterator_free(struct iterator *ptr)
> free(it);
> }
>
> +static int
> +session_settings_set_forward(int *sid, const char *key, bool is_eq,
> + bool is_including)
> +{
> + int low = 0, high = SESSION_SETTING_COUNT - 1;
> + if (key == NULL)
> + return 0;
> + while (low <= high) {
> + int index = (high + low) / 2;
> + const char *name = session_setting_strs[index];
> + int cmp = strcmp(name, key);
> + if (cmp == 0) {
> + if (is_including) {
> + *sid = index;
> + return 0;
> + }
> + *sid = ++index;
> + return index < SESSION_SETTING_COUNT ? 0 : -1;
> + }
> + if (cmp < 0)
> + low = index + 1;
> + else
> + high = index - 1;
> + }
> + if (is_eq) {
> + *sid = SESSION_SETTING_COUNT;
> + return -1;
> + }
> + assert(low > high);
> + *sid = low;
> + return low < SESSION_SETTING_COUNT ? 0 : -1;
> +}
> +
> +static int
> +session_settings_set_reverse(int *sid, const char *key, bool is_eq,
> + bool is_including)
> +{
> + int low = 0, high = SESSION_SETTING_COUNT - 1;
> + if (key == NULL)
> + return 0;
> + while (low <= high) {
> + int index = (high + low) / 2;
> + const char *name = session_setting_strs[index];
> + int cmp = strcmp(name, key);
> + if (cmp == 0) {
> + if (is_including) {
> + *sid = index;
> + return 0;
> + }
> + *sid = --index;
> + return index >= 0 ? 0 : -1;
> + }
> + if (cmp < 0)
> + low = index + 1;
> + else
> + high = index - 1;
> + }
> + if (is_eq) {
> + *sid = SESSION_SETTING_COUNT;
> + return -1;
> + }
> + assert(low > high);
> + *sid = high;
> + return high >= 0 ? 0 : -1;
> +}
> +
> static int
> session_settings_next(int *sid, const char *key, bool is_eq, bool is_including)
> {
> @@ -145,9 +213,15 @@ session_settings_iterator_next(struct iterator *iterator, struct tuple **result)
> 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;
> - if (session_settings_next(&sid, key, is_eq, is_including) == 0)
> - is_found = true;
> + bool is_found;
> + if (!it->is_set) {
> + it->is_set = true;
> + is_found = session_settings_set_forward(&sid, key, is_eq,
> + is_including) == 0;
> + } else {
> + is_found = session_settings_next(&sid, key, is_eq,
> + is_including) == 0;
> + }
> it->setting_id = sid + 1;
> if (!is_found) {
> *result = NULL;
> @@ -167,9 +241,15 @@ session_settings_iterator_prev(struct iterator *iterator, struct tuple **result)
> 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;
> - if (session_settings_prev(&sid, key, is_eq, is_including) == 0)
> - is_found = true;
> + bool is_found;
> + if (!it->is_set) {
> + it->is_set = true;
> + is_found = session_settings_set_reverse(&sid, key, is_eq,
> + is_including) == 0;
> + } else {
> + is_found = session_settings_prev(&sid, key, is_eq,
> + is_including) == 0;
> + }
> it->setting_id = sid - 1;
> if (!is_found) {
> *result = NULL;
> @@ -221,6 +301,7 @@ session_settings_index_create_iterator(struct index *base,
> it->is_eq = type == ITER_EQ || type == ITER_REQ;
> it->is_including = it->is_eq || type == ITER_GE || type == ITER_ALL ||
> type == ITER_LE;
> + it->is_set = false;
> it->format = index->format;
> if (!iterator_type_is_reverse(type)) {
> it->base.next = session_settings_iterator_next;
> @@ -244,7 +325,7 @@ session_settings_index_get(struct index *base, const char *key,
> key = mp_decode_str(&key, &len);
> key = tt_cstr(key, len);
> int sid = 0;
> - if (session_settings_next(&sid, key, true, true) != 0) {
> + if (session_settings_set_forward(&sid, key, true, true) != 0) {
> *result = NULL;
> return 0;
> }
> @@ -345,7 +426,7 @@ session_settings_space_execute_update(struct space *space, struct txn *txn,
> }
> key = mp_decode_str(&key, &key_len);
> key = tt_cstr(key, key_len);
> - if (session_settings_next(&sid, key, true, true) != 0) {
> + if (session_settings_set_forward(&sid, key, true, true) != 0) {
> *result = NULL;
> return 0;
> }
> diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result
> index ed340d053..bae77192e 100644
> --- a/test/box/gh-4511-access-settings-from-any-frontend.result
> +++ b/test/box/gh-4511-access-settings-from-any-frontend.result
> @@ -82,6 +82,12 @@ function check_sorting(ss, ts, key)
> for _, it in pairs(iterators_list) do
> local view_space = ss:select({key}, {iterator = it})
> local test_space = ts:select({key}, {iterator = it})
> + if #view_space ~= #test_space then
> + return {
> + err = 'bad sorting', type = it, exp = test_space,
> + got = view_space
> + }
> + end
> for key, value in pairs(view_space) do
> if test_space[key].name ~= value.name then
> return {
> @@ -111,32 +117,37 @@ check_sorting(s, t, 'sql_d')
> check_sorting(s, t, 'sql_v')
> | ---
> | ...
> -check_sorting(s, t, 'sql_defer_foreign_keys')
> +check_sorting(s, t, 'z')
> | ---
> | ...
> -
> -t:drop()
> +check_sorting(s, t, 'sql_parser_debuf')
> + | ---
> + | ...
> +check_sorting(s, t, 'sql_parser_debuh')
> | ---
> | ...
>
> --- Check get() method of session_settings space.
> -s:get({'sql_defer_foreign_keys'})
> +name = nil
> | ---
> - | - ['sql_defer_foreign_keys', false]
> | ...
> -s:get({'sql_recursive_triggers'})
> +err = nil
> | ---
> - | - ['sql_recursive_triggers', true]
> | ...
> -s:get({'sql_reverse_unordered_selects'})
> +for _, tuple in t:pairs() do \
> + name = tuple.name \
> + err = check_sorting(s, t, name) \
> + if err then \
> + break \
> + end \
> +end
> | ---
> - | - ['sql_reverse_unordered_selects', false]
> | ...
> -s:get({'sql_default_engine'})
> +err and {err, name}
> | ---
> - | - ['sql_default_engine', 'memtx']
> + | - null
> | ...
> -s:get({'abcd'})
> +
> +t:drop()
> | ---
> | ...
>
> diff --git a/test/box/gh-4511-access-settings-from-any-frontend.test.lua b/test/box/gh-4511-access-settings-from-any-frontend.test.lua
> index 366874998..b243be15e 100644
> --- a/test/box/gh-4511-access-settings-from-any-frontend.test.lua
> +++ b/test/box/gh-4511-access-settings-from-any-frontend.test.lua
> @@ -36,6 +36,12 @@ function check_sorting(ss, ts, key)
> for _, it in pairs(iterators_list) do
> local view_space = ss:select({key}, {iterator = it})
> local test_space = ts:select({key}, {iterator = it})
> + if #view_space ~= #test_space then
> + return {
> + err = 'bad sorting', type = it, exp = test_space,
> + got = view_space
> + }
> + end
> for key, value in pairs(view_space) do
> if test_space[key].name ~= value.name then
> return {
> @@ -52,17 +58,23 @@ check_sorting(s, t)
> check_sorting(s, t, 'abcde')
> check_sorting(s, t, 'sql_d')
> check_sorting(s, t, 'sql_v')
> -check_sorting(s, t, 'sql_defer_foreign_keys')
> +check_sorting(s, t, 'z')
> +check_sorting(s, t, 'sql_parser_debuf')
> +check_sorting(s, t, 'sql_parser_debuh')
> +
> +name = nil
> +err = nil
> +for _, tuple in t:pairs() do \
> + name = tuple.name \
> + err = check_sorting(s, t, name) \
> + if err then \
> + break \
> + end \
> +end
> +err and {err, name}
>
> t:drop()
>
> --- Check get() method of session_settings space.
> -s:get({'sql_defer_foreign_keys'})
> -s:get({'sql_recursive_triggers'})
> -s:get({'sql_reverse_unordered_selects'})
> -s:get({'sql_default_engine'})
> -s:get({'abcd'})
> -
> -- Check pairs() method of session_settings space.
> t = {}
> for key, value in s:pairs() do table.insert(t, {key, value}) end
> --
> 2.21.1 (Apple Git-122.3)
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 2/4] box: add binary search for _session_settings space
2020-04-03 14:00 ` Nikita Pettik
@ 2020-04-13 13:40 ` Kirill Yukhin
0 siblings, 0 replies; 26+ messages in thread
From: Kirill Yukhin @ 2020-04-13 13:40 UTC (permalink / raw)
To: Nikita Pettik; +Cc: tarantool-patches, v.shpilevoy
Hello,
On 03 Apr 14:00, Nikita Pettik wrote:
> On 30 Mar 12:13, Chris Sosnin wrote:
> > As it is mentioned in implementation, it is important
> > that _session_settings options are sorted by name, so
> > there is no need in linear lookup and we can replace it
> > with binary search.
> >
> > Closes #4712
> > ---
>
> Still I see no need for this patch. It is unclear which improvements
> it introduces except for code complication.
I agree, let's drop it.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 3/4] box: provide a user friendly frontend for accessing session settings
2020-03-30 9:13 [Tarantool-patches] [PATCH 0/4] session settings fixes Chris Sosnin
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 1/4] box: replace session_settings modules with a single array Chris Sosnin
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 2/4] box: add binary search for _session_settings space Chris Sosnin
@ 2020-03-30 9:13 ` Chris Sosnin
2020-04-03 14:47 ` Nikita Pettik
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 4/4] sql: " Chris Sosnin
` (3 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Chris Sosnin @ 2020-03-30 9:13 UTC (permalink / raw)
To: v.shpilevoy, korablev, tarantool-patches
- space_object:update() is hard to use for configuring session settings,
so we provide box.session.settings table, which can be used in a much more
native way.
- Prior to this patch sql settings were not accessible before box.cfg()
call, even though these flags can be set right after session creation.
Part of #4711
---
src/box/lua/session.c | 111 ++++++++++++++++++
src/box/session.cc | 1 +
src/box/session.h | 2 +
src/box/session_settings.c | 16 ++-
src/box/session_settings.h | 3 +
src/box/sql.c | 5 -
...rontend.result => session_settings.result} | 61 ++++++++++
...end.test.lua => session_settings.test.lua} | 20 ++++
8 files changed, 211 insertions(+), 8 deletions(-)
rename test/box/{gh-4511-access-settings-from-any-frontend.result => session_settings.result} (86%)
rename test/box/{gh-4511-access-settings-from-any-frontend.test.lua => session_settings.test.lua} (86%)
diff --git a/src/box/lua/session.c b/src/box/lua/session.c
index c6a600f6f..103bf22fd 100644
--- a/src/box/lua/session.c
+++ b/src/box/lua/session.c
@@ -42,6 +42,8 @@
#include "box/user.h"
#include "box/schema.h"
#include "box/port.h"
+#include "box/session_settings.h"
+#include "tt_static.h"
static const char *sessionlib_name = "box.session";
@@ -411,6 +413,114 @@ lbox_session_on_access_denied(struct lua_State *L)
lbox_push_on_access_denied_event, NULL);
}
+static int
+lbox_session_setting_get_by_id(struct lua_State *L, int sid)
+{
+ assert(sid >= 0 && sid < SESSION_SETTING_COUNT);
+ const char *mp_pair, *mp_pair_end;
+ session_settings[sid].get(sid, &mp_pair, &mp_pair_end);
+ uint32_t len;
+ mp_decode_array(&mp_pair);
+ mp_decode_str(&mp_pair, &len);
+ enum field_type field_type = session_settings[sid].field_type;
+ if (field_type == FIELD_TYPE_BOOLEAN) {
+ bool value = mp_decode_bool(&mp_pair);
+ lua_pushboolean(L, value);
+ } else {
+ assert(field_type == FIELD_TYPE_STRING);
+ const char *str = mp_decode_str(&mp_pair, &len);
+ lua_pushlstring(L, str, len);
+ }
+ return 1;
+}
+
+static int
+lbox_session_setting_get(struct lua_State *L)
+{
+ assert(lua_gettop(L) == 2);
+ const char *setting_name = lua_tostring(L, -1);
+ int sid = session_setting_find(setting_name);
+ if (sid < 0) {
+ diag_set(ClientError, ER_PROC_LUA, tt_sprintf("Session "\
+ "setting %s doesn't exist", setting_name));
+ return luaT_error(L);
+ }
+ return lbox_session_setting_get_by_id(L, sid);
+}
+
+static int
+lbox_session_setting_set(struct lua_State *L)
+{
+ assert(lua_gettop(L) == 3);
+ int arg_type = lua_type(L, -1);
+ const char *setting_name = lua_tostring(L, -2);
+ int sid = session_setting_find(setting_name);
+ if (sid < 0) {
+ diag_set(ClientError, ER_PROC_LUA, tt_sprintf("Session "\
+ "setting %s doesn't exist", setting_name));
+ return luaT_error(L);
+ }
+ struct session_setting *setting = &session_settings[sid];
+ switch (arg_type) {
+ case LUA_TBOOLEAN: {
+ bool value = lua_toboolean(L, -1);
+ size_t size = mp_sizeof_bool(value);
+ char *mp_value = (char *) static_alloc(size);
+ mp_encode_bool(mp_value, value);
+ if (setting->set(sid, mp_value) != 0)
+ return luaT_error(L);
+ break;
+ }
+ case LUA_TSTRING: {
+ const char *str = lua_tostring(L, -1);
+ size_t len = strlen(str);
+ uint32_t size = mp_sizeof_str(len);
+ char *mp_value = (char *) static_alloc(size);
+ if (mp_value == NULL) {
+ diag_set(OutOfMemory, size, "static_alloc",
+ "mp_value");
+ return luaT_error(L);
+ }
+ mp_encode_str(mp_value, str, len);
+ if (setting->set(sid, mp_value) != 0)
+ return luaT_error(L);
+ break;
+ }
+ default:
+ diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE,
+ session_setting_strs[sid],
+ field_type_strs[setting->field_type]);
+ return luaT_error(L);
+ }
+ return 0;
+}
+
+static int
+lbox_session_settings_serialize(struct lua_State *L)
+{
+ lua_newtable(L);
+ for (int id = 0; id < SESSION_SETTING_COUNT; ++id) {
+ lbox_session_setting_get_by_id(L, id);
+ lua_setfield(L, -2, session_setting_strs[id]);
+ }
+ return 1;
+}
+
+static void
+lbox_session_settings_init(struct lua_State *L)
+{
+ lua_newtable(L);
+ lua_createtable(L, 0, 3);
+ lua_pushcfunction(L, lbox_session_settings_serialize);
+ lua_setfield(L, -2, "__serialize");
+ lua_pushcfunction(L, lbox_session_setting_get);
+ lua_setfield(L, -2, "__index");
+ lua_pushcfunction(L, lbox_session_setting_set);
+ lua_setfield(L, -2, "__newindex");
+ lua_setmetatable(L, -2);
+ lua_setfield(L, -2, "settings");
+}
+
void
session_storage_cleanup(int sid)
{
@@ -478,5 +588,6 @@ box_lua_session_init(struct lua_State *L)
{NULL, NULL}
};
luaL_register_module(L, sessionlib_name, sessionlib);
+ lbox_session_settings_init(L);
lua_pop(L, 1);
}
diff --git a/src/box/session.cc b/src/box/session.cc
index 881318252..b557eed62 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -283,6 +283,7 @@ session_init()
panic("out of memory");
mempool_create(&session_pool, &cord()->slabc, sizeof(struct session));
credentials_create(&admin_credentials, admin_user);
+ sql_session_settings_init();
}
void
diff --git a/src/box/session.h b/src/box/session.h
index 6dfc7cba5..1c47b8986 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -41,6 +41,8 @@
extern "C" {
#endif /* defined(__cplusplus) */
+extern void sql_session_settings_init();
+
struct port;
struct session_vtab;
diff --git a/src/box/session_settings.c b/src/box/session_settings.c
index 5e4a50427..15ab65a97 100644
--- a/src/box/session_settings.c
+++ b/src/box/session_settings.c
@@ -324,8 +324,8 @@ session_settings_index_get(struct index *base, const char *key,
uint32_t len;
key = mp_decode_str(&key, &len);
key = tt_cstr(key, len);
- int sid = 0;
- if (session_settings_set_forward(&sid, key, true, true) != 0) {
+ int sid = session_setting_find(key);
+ if (sid < 0) {
*result = NULL;
return 0;
}
@@ -426,7 +426,8 @@ session_settings_space_execute_update(struct space *space, struct txn *txn,
}
key = mp_decode_str(&key, &key_len);
key = tt_cstr(key, key_len);
- if (session_settings_set_forward(&sid, key, true, true) != 0) {
+ sid = session_setting_find(key);
+ if (sid < 0) {
*result = NULL;
return 0;
}
@@ -520,3 +521,12 @@ const struct space_vtab session_settings_space_vtab = {
/* .prepare_alter = */ generic_space_prepare_alter,
/* .invalidate = */ generic_space_invalidate,
};
+
+int
+session_setting_find(const char *name) {
+ int sid;
+ if (session_settings_set_forward(&sid, name, true, true) == 0)
+ return sid;
+ else
+ return -1;
+}
diff --git a/src/box/session_settings.h b/src/box/session_settings.h
index de24e3c6f..e2adc5289 100644
--- a/src/box/session_settings.h
+++ b/src/box/session_settings.h
@@ -84,3 +84,6 @@ struct session_setting {
extern struct session_setting session_settings[SESSION_SETTING_COUNT];
extern const char *session_setting_strs[SESSION_SETTING_COUNT];
+
+int
+session_setting_find(const char *name);
diff --git a/src/box/sql.c b/src/box/sql.c
index 1256df856..ba98ce5df 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -64,14 +64,9 @@ static const uint32_t default_sql_flags = SQL_EnableTrigger
| SQL_AutoIndex
| SQL_RecTriggers;
-extern void
-sql_session_settings_init();
-
void
sql_init()
{
- sql_session_settings_init();
-
default_flags |= default_sql_flags;
current_session()->sql_flags |= default_sql_flags;
diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/session_settings.result
similarity index 86%
rename from test/box/gh-4511-access-settings-from-any-frontend.result
rename to test/box/session_settings.result
index bae77192e..b32a0becb 100644
--- a/test/box/gh-4511-access-settings-from-any-frontend.result
+++ b/test/box/session_settings.result
@@ -298,3 +298,64 @@ s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
| ---
| - error: Session setting sql_defer_foreign_keys expected a value of type boolean
| ...
+
+-- gh-4711: Provide a user-friendly frontend for accessing session settings.
+settings = box.session.settings
+ | ---
+ | ...
+assert(settings ~= nil)
+ | ---
+ | - true
+ | ...
+
+s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
+ | ---
+ | - ['sql_default_engine', 'vinyl']
+ | ...
+settings.sql_default_engine
+ | ---
+ | - vinyl
+ | ...
+settings.sql_default_engine = 'memtx'
+ | ---
+ | ...
+s:get('sql_default_engine').value
+ | ---
+ | - memtx
+ | ...
+settings.sql_defer_foreign_keys = true
+ | ---
+ | ...
+s:get('sql_defer_foreign_keys').value
+ | ---
+ | - true
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 2, false}})
+ | ---
+ | - ['sql_defer_foreign_keys', false]
+ | ...
+settings.sql_defer_foreign_keys
+ | ---
+ | - false
+ | ...
+
+settings.sql_default_engine = true
+ | ---
+ | - error: Session setting sql_default_engine expected a value of type string
+ | ...
+settings.sql_defer_foreign_keys = 'false'
+ | ---
+ | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
+ | ...
+settings.sql_parser_debug = 'string'
+ | ---
+ | - error: Session setting sql_parser_debug expected a value of type boolean
+ | ...
+
+str = string.rep('a', 20 * 1024)
+ | ---
+ | ...
+box.session.settings.sql_default_engine = str
+ | ---
+ | - error: Failed to allocate 20483 bytes in static_alloc for mp_value
+ | ...
diff --git a/test/box/gh-4511-access-settings-from-any-frontend.test.lua b/test/box/session_settings.test.lua
similarity index 86%
rename from test/box/gh-4511-access-settings-from-any-frontend.test.lua
rename to test/box/session_settings.test.lua
index b243be15e..440bef7ce 100644
--- a/test/box/gh-4511-access-settings-from-any-frontend.test.lua
+++ b/test/box/session_settings.test.lua
@@ -118,3 +118,23 @@ s:update('sql_defer_foreign_keys', {{'=', 'some text', true}})
s:update('sql_defer_foreign_keys', {{'=', 'value', 1}})
s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}})
s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
+
+-- gh-4711: Provide a user-friendly frontend for accessing session settings.
+settings = box.session.settings
+assert(settings ~= nil)
+
+s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
+settings.sql_default_engine
+settings.sql_default_engine = 'memtx'
+s:get('sql_default_engine').value
+settings.sql_defer_foreign_keys = true
+s:get('sql_defer_foreign_keys').value
+s:update('sql_defer_foreign_keys', {{'=', 2, false}})
+settings.sql_defer_foreign_keys
+
+settings.sql_default_engine = true
+settings.sql_defer_foreign_keys = 'false'
+settings.sql_parser_debug = 'string'
+
+str = string.rep('a', 20 * 1024)
+box.session.settings.sql_default_engine = str
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 26+ messages in thread* [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings
2020-03-30 9:13 [Tarantool-patches] [PATCH 0/4] session settings fixes Chris Sosnin
` (2 preceding siblings ...)
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 3/4] box: provide a user friendly frontend for accessing session settings Chris Sosnin
@ 2020-03-30 9:13 ` Chris Sosnin
2020-04-03 15:19 ` Nikita Pettik
2020-04-04 21:56 ` Vladislav Shpilevoy
2020-04-02 9:14 ` [Tarantool-patches] [PATCH 0/4] session settings fixes Timur Safin
` (2 subsequent siblings)
6 siblings, 2 replies; 26+ messages in thread
From: Chris Sosnin @ 2020-03-30 9:13 UTC (permalink / raw)
To: v.shpilevoy, korablev, tarantool-patches
Currently if a user wants to change session setting with SQL, one has
to execute UPDATE query like:
[[UPDATE "_session_settings" SET "value" = true WHERE "name" = 'name']]
However, direct access to system spaces isn't considered to be a good practice.
To avoid that and a bit simplify user's life, we introduce SQL shortcut command
SETTING SET.
Closes #4711
@TarantoolBot document
Title: API for accessing _session_settings space.
There are two ways of updating values of session settings:
via Lua and SQL.
Lua:
box.session.settings is a table, which is always accessible
to user. The syntax is the following:
`box.session.settings.<setting_name> = <new_value>`.
Example of usage:
```
tarantool> box.session.settings.sql_default_engine
---
- memtx
...
tarantool> box.session.settings.sql_default_engine = 'vinyl'
---
...
```
The table itself represents the (unordered) result of select
from _session_settings space.
SQL:
Instead of typing long UPDATE query one can use the SETTING SET command:
`box.execute([[SETTING SET "<setting_name>" = <new_value>]])`.
Note, that this query is case sensitive so the name must be quoted.
Also, SETTING SET doesn't prove any implicit casts, so <new_value> must
be of the type corresponding to the setting being updated.
Example:
```
tarantool> box.execute([[setting set "sql_default_engine" = 'memtx']])
---
- row_count: 1
...
tarantool> box.execute([[setting set "sql_defer_foreign_keys" = true]])
---
- row_count: 1
...
```
---
| 1 +
src/box/sql/build.c | 25 +++++++++++++++
src/box/sql/parse.y | 5 +++
src/box/sql/sqlInt.h | 11 +++++++
src/box/sql/vdbe.c | 50 ++++++++++++++++++++++++++++++
test/box/session_settings.result | 49 +++++++++++++++++++++++++++++
test/box/session_settings.test.lua | 13 ++++++++
7 files changed, 154 insertions(+)
--git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
index 006285622..887853529 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -159,6 +159,7 @@ static Keyword aKeywordTable[] = {
{ "SCALAR", "TK_SCALAR", true },
{ "SELECT", "TK_SELECT", true },
{ "SET", "TK_SET", true },
+ { "SETTING", "TK_SETTING", false },
{ "SIMPLE", "TK_SIMPLE", true },
{ "START", "TK_START", true },
{ "STRING", "TK_STRING_KW", true },
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index a00da31f9..cdcf8b6d8 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3481,3 +3481,28 @@ sql_session_settings_init()
setting->set = sql_session_setting_set;
}
}
+
+void
+sql_setting_set(struct Parse *parse_context, struct Token *name,
+ struct Expr *expr)
+{
+ struct Vdbe *vdbe = sqlGetVdbe(parse_context);
+ if (vdbe == NULL)
+ goto abort;
+ sqlVdbeCountChanges(vdbe);
+ char *key = sql_name_from_token(parse_context->db, name);
+ if (key == NULL)
+ goto abort;
+ int index = session_setting_find(key);
+ if (index >= 0) {
+ int target = ++parse_context->nMem;
+ sqlExprCode(parse_context, expr, target);
+ sqlVdbeAddOp2(vdbe, OP_SetSetting, index, target);
+ return;
+ }
+ diag_set(ClientError, ER_SQL_PARSER_GENERIC,
+ tt_sprintf("Session setting %s doesn't exist", key));
+abort:
+ parse_context->is_aborted = true;
+ return;
+}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 1a0e89703..56977b003 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1541,6 +1541,11 @@ cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y). {
sql_drop_index(pParse);
}
+///////////////////////////// The SETTING SET command ////////////////////////
+cmd ::= SETTING SET nm(X) EQ expr(Y). {
+ sql_setting_set(pParse,&X,Y.pExpr);
+}
+
///////////////////////////// The PRAGMA command /////////////////////////////
//
cmd ::= PRAGMA nm(X). {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 1579cc92e..a700dd9c9 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4513,4 +4513,15 @@ int
sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
uint32_t *fieldno);
+/**
+ * Create VDBE instructions to set the new value of the session setting.
+ *
+ * @param parse_context Parsing context.
+ * @param name Name of the session setting.
+ * @param value New value of the session setting.
+ */
+void
+sql_setting_set(struct Parse *parse_context, struct Token *name,
+ struct Expr *value);
+
#endif /* sqlINT_H */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index e8a029a8a..d6b753a51 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -55,6 +55,7 @@
#include "box/schema.h"
#include "box/space.h"
#include "box/sequence.h"
+#include "box/session_settings.h"
/*
* Invoke this macro on memory cells just prior to changing the
@@ -5252,6 +5253,55 @@ case OP_IncMaxid: {
break;
}
+/* Opcode: SetSetting P1 P2 * * *
+ *
+ * Set the new value of the session setting. P1 is the id of the
+ * setting in the session_settings array, P2 is the register
+ * holding a value.
+ */
+case OP_SetSetting: {
+ int sid = pOp->p1;
+ pIn1 = &aMem[pOp->p2];
+ assert(sid >= 0 && sid < SESSION_SETTING_COUNT);
+ struct session_setting *setting = &session_settings[sid];
+ switch (setting->field_type) {
+ case FIELD_TYPE_BOOLEAN: {
+ if ((pIn1->flags & MEM_Bool) == 0)
+ goto invalid_type;
+ bool value = pIn1->u.b;
+ size_t size = mp_sizeof_bool(value);
+ char *mp_value = (char *) static_alloc(size);
+ mp_encode_bool(mp_value, value);
+ if (setting->set(sid, mp_value) != 0)
+ goto abort_due_to_error;
+ break;
+ }
+ case FIELD_TYPE_STRING: {
+ if ((pIn1->flags & MEM_Str) == 0)
+ goto invalid_type;
+ const char *str = pIn1->z;
+ uint32_t size = mp_sizeof_str(pIn1->n);
+ char *mp_value = (char *) static_alloc(size);
+ if (mp_value == NULL) {
+ diag_set(OutOfMemory, size, "static_alloc", "mp_value");
+ goto abort_due_to_error;
+ }
+ mp_encode_str(mp_value, str, pIn1->n);
+ if (setting->set(sid, mp_value) != 0)
+ goto abort_due_to_error;
+ break;
+ }
+ default:
+ invalid_type:
+ diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE,
+ session_setting_strs[sid],
+ field_type_strs[setting->field_type]);
+ goto abort_due_to_error;
+ }
+ p->nChange++;
+ break;
+}
+
/* Opcode: Noop * * * * *
*
* Do nothing. This instruction is often useful as a jump
diff --git a/test/box/session_settings.result b/test/box/session_settings.result
index b32a0becb..28c79379b 100644
--- a/test/box/session_settings.result
+++ b/test/box/session_settings.result
@@ -339,6 +339,39 @@ settings.sql_defer_foreign_keys
| - false
| ...
+box.execute([[setting set "sql_default_engine" = 'vinyl']])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_default_engine').value
+ | ---
+ | - vinyl
+ | ...
+box.execute([[setting set "sql_default_engine" = 'memtx']])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_default_engine').value
+ | ---
+ | - memtx
+ | ...
+box.execute([[setting set "sql_defer_foreign_keys" = true]])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_defer_foreign_keys').value
+ | ---
+ | - true
+ | ...
+box.execute([[setting set "sql_defer_foreign_keys" = ?]], {false})
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_defer_foreign_keys').value
+ | ---
+ | - false
+ | ...
+
settings.sql_default_engine = true
| ---
| - error: Session setting sql_default_engine expected a value of type string
@@ -359,3 +392,19 @@ box.session.settings.sql_default_engine = str
| ---
| - error: Failed to allocate 20483 bytes in static_alloc for mp_value
| ...
+
+box.execute([[setting set "sql_def_engine" = true]])
+ | ---
+ | - null
+ | - Session setting sql_def_engine doesn't exist
+ | ...
+box.execute([[setting set "sql_default_engine" = true]])
+ | ---
+ | - null
+ | - Session setting sql_default_engine expected a value of type string
+ | ...
+box.execute([[setting set "sql_defer_foreign_keys" = 'true']])
+ | ---
+ | - null
+ | - Session setting sql_defer_foreign_keys expected a value of type boolean
+ | ...
diff --git a/test/box/session_settings.test.lua b/test/box/session_settings.test.lua
index 440bef7ce..c477139f9 100644
--- a/test/box/session_settings.test.lua
+++ b/test/box/session_settings.test.lua
@@ -132,9 +132,22 @@ s:get('sql_defer_foreign_keys').value
s:update('sql_defer_foreign_keys', {{'=', 2, false}})
settings.sql_defer_foreign_keys
+box.execute([[setting set "sql_default_engine" = 'vinyl']])
+s:get('sql_default_engine').value
+box.execute([[setting set "sql_default_engine" = 'memtx']])
+s:get('sql_default_engine').value
+box.execute([[setting set "sql_defer_foreign_keys" = true]])
+s:get('sql_defer_foreign_keys').value
+box.execute([[setting set "sql_defer_foreign_keys" = ?]], {false})
+s:get('sql_defer_foreign_keys').value
+
settings.sql_default_engine = true
settings.sql_defer_foreign_keys = 'false'
settings.sql_parser_debug = 'string'
str = string.rep('a', 20 * 1024)
box.session.settings.sql_default_engine = str
+
+box.execute([[setting set "sql_def_engine" = true]])
+box.execute([[setting set "sql_default_engine" = true]])
+box.execute([[setting set "sql_defer_foreign_keys" = 'true']])
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 4/4] sql: " Chris Sosnin
@ 2020-04-03 15:19 ` Nikita Pettik
2020-04-04 21:56 ` Vladislav Shpilevoy
1 sibling, 0 replies; 26+ messages in thread
From: Nikita Pettik @ 2020-04-03 15:19 UTC (permalink / raw)
To: Chris Sosnin; +Cc: tarantool-patches, v.shpilevoy
On 30 Mar 12:13, Chris Sosnin wrote:
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index a00da31f9..cdcf8b6d8 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -3481,3 +3481,28 @@ sql_session_settings_init()
> setting->set = sql_session_setting_set;
> }
> }
> +
> +void
> +sql_setting_set(struct Parse *parse_context, struct Token *name,
> + struct Expr *expr)
> +{
> + struct Vdbe *vdbe = sqlGetVdbe(parse_context);
> + if (vdbe == NULL)
> + goto abort;
> + sqlVdbeCountChanges(vdbe);
> + char *key = sql_name_from_token(parse_context->db, name);
> + if (key == NULL)
> + goto abort;
> + int index = session_setting_find(key);
> + if (index >= 0) {
> + int target = ++parse_context->nMem;
> + sqlExprCode(parse_context, expr, target);
> + sqlVdbeAddOp2(vdbe, OP_SetSetting, index, target);
> + return;
> + }
> + diag_set(ClientError, ER_SQL_PARSER_GENERIC,
> + tt_sprintf("Session setting %s doesn't exist", key));
Mb it is worth moving this check to runtime? I.e. pass to OP_SetSetting
name of setting to be set. The less appeals to system spaces and
cache look ups are made, the better parsing process is organized.
The rest is ok.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 4/4] sql: " Chris Sosnin
2020-04-03 15:19 ` Nikita Pettik
@ 2020-04-04 21:56 ` Vladislav Shpilevoy
2020-04-10 15:40 ` Chris Sosnin
1 sibling, 1 reply; 26+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-04 21:56 UTC (permalink / raw)
To: Chris Sosnin, korablev, tarantool-patches
Hi! Thanks for the patch!
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index a00da31f9..cdcf8b6d8 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -3481,3 +3481,28 @@ sql_session_settings_init()
> setting->set = sql_session_setting_set;
> }
> }
> +
> +void
> +sql_setting_set(struct Parse *parse_context, struct Token *name,
> + struct Expr *expr)
> +{
> + struct Vdbe *vdbe = sqlGetVdbe(parse_context);
> + if (vdbe == NULL)
> + goto abort;
> + sqlVdbeCountChanges(vdbe);
> + char *key = sql_name_from_token(parse_context->db, name);
> + if (key == NULL)
> + goto abort;
> + int index = session_setting_find(key);
I agree with Nikita. Better make this call part of OP_SetSetting.
> + if (index >= 0) {
> + int target = ++parse_context->nMem;
> + sqlExprCode(parse_context, expr, target);
> + sqlVdbeAddOp2(vdbe, OP_SetSetting, index, target);
> + return;
> + }
> + diag_set(ClientError, ER_SQL_PARSER_GENERIC,
> + tt_sprintf("Session setting %s doesn't exist", key));
> +abort:
> + parse_context->is_aborted = true;
> + return;
> +}
Talking of the syntax, I don't have a strong opinion here
regarding any particular option. I only want the new statement
be short, and not involving _session_settings manipulations.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings
2020-04-04 21:56 ` Vladislav Shpilevoy
@ 2020-04-10 15:40 ` Chris Sosnin
2020-04-11 17:18 ` Vladislav Shpilevoy
2020-04-13 7:50 ` Timur Safin
0 siblings, 2 replies; 26+ messages in thread
From: Chris Sosnin @ 2020-04-10 15:40 UTC (permalink / raw)
To: v.shpilevoy; +Cc: tarantool-patches
> > diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> > index a00da31f9..cdcf8b6d8 100644
> > --- a/src/box/sql/build.c
> > +++ b/src/box/sql/build.c
> > @@ -3481,3 +3481,28 @@ sql_session_settings_init()
> > setting->set = sql_session_setting_set;
> > }
> > }
> > +
> > +void
> > +sql_setting_set(struct Parse *parse_context, struct Token *name,
> > + struct Expr *expr)
> > +{
> > + struct Vdbe *vdbe = sqlGetVdbe(parse_context);
> > + if (vdbe == NULL)
> > + goto abort;
> > + sqlVdbeCountChanges(vdbe);
> > + char *key = sql_name_from_token(parse_context->db, name);
> > + if (key == NULL)
> > + goto abort;
> > + int index = session_setting_find(key);
>
> I agree with Nikita. Better make this call part of OP_SetSetting.
I agree, fixed:
+
+void
+sql_setting_set(struct Parse *parse_context, struct Token *name,
+ struct Expr *expr)
+{
+ struct Vdbe *vdbe = sqlGetVdbe(parse_context);
+ if (vdbe == NULL)
+ goto abort;
+ sqlVdbeCountChanges(vdbe);
+ char *key = sql_name_from_token(parse_context->db, name);
+ if (key == NULL)
+ goto abort;
+ int target = ++parse_context->nMem;
+ sqlExprCode(parse_context, expr, target);
+ sqlVdbeAddOp4(vdbe, OP_SetSession, target, 0, 0, key, P4_DYNAMIC);
+ return;
+abort:
+ parse_context->is_aborted = true;
+ return;
+}
+/* Opcode: SetSession P1 * * P4 *
+ *
+ * Set new value of the session setting. P4 is the name of the
+ * setting being updated, P1 is the register holding a value.
+ */
+case OP_SetSession: {
+ assert(pOp->p4type == P4_DYNAMIC);
+ const char *setting_name = pOp->p4.z;
+ int sid = session_setting_find(setting_name);
+ if (sid < 0) {
+ diag_set(ClientError, ER_NO_SUCH_SESSION_SETTING, setting_name);
+ goto abort_due_to_error;
+ }
+ pIn1 = &aMem[pOp->p1];
...
> Talking of the syntax, I don't have a strong opinion here
> regarding any particular option. I only want the new statement
> be short, and not involving _session_settings manipulations.
I changed it to the following (the main problem with reserved word persists):
+///////////////////////////// The SET SESSION command ////////////////////////
+//
+cmd ::= SET SESSION nm(X) EQ term(Y). {
+ sql_setting_set(pParse,&X,Y.pExpr);
+}
+
I also changed the 3rd commit (Lua frontend) in the series to introduce new error
code (it's present in the diff above):
+ /*213 */_(ER_NO_SUCH_SESSION_SETTING, "Session setting %s doesn't exist") \
as long as searching is moved to vdbe.c and raising parsing error is not correct.
You can see the whole patch below:
===============================================================================
Currently if a user wants to change session setting with SQL, one has
to execute UPDATE query like:
[[UPDATE "_session_settings" SET "value" = true WHERE "name" = 'name']]
However, direct access to system spaces isn't considered to be a good practice.
To avoid that and a bit simplify user's life, we introduce SQL shortcut command
SET SESSION.
Closes #4711
@TarantoolBot document
Title: API for accessing _session_settings space.
There are two ways of updating values of session settings:
via Lua and SQL.
Lua:
box.session.settings is a table, which is always accessible
to user. The syntax is the following:
`box.session.settings.<setting_name> = <new_value>`.
Example of usage:
```
tarantool> box.session.settings.sql_default_engine
---
- memtx
...
tarantool> box.session.settings.sql_default_engine = 'vinyl'
---
...
```
The table itself represents the (unordered) result of select
from _session_settings space.
SQL:
Instead of typing long UPDATE query one can use the SET SESSION command:
`box.execute([[SET SESSION "<setting_name>" = <new_value>]])`.
Note, that this query is case sensitive so the name must be quoted.
Also, SET SESSION doesn't provide any implicit casts, so <new_value> must
be of the type corresponding to the setting being updated.
Example:
```
tarantool> box.execute([[set session "sql_default_engine" = 'memtx']])
---
- row_count: 1
...
tarantool> box.execute([[set session "sql_defer_foreign_keys" = true]])
---
- row_count: 1
...
```
---
| 1 +
src/box/sql/build.c | 20 +++++++++++
src/box/sql/parse.y | 6 ++++
src/box/sql/sqlInt.h | 11 ++++++
src/box/sql/vdbe.c | 54 ++++++++++++++++++++++++++++++
test/box/session_settings.result | 49 +++++++++++++++++++++++++++
test/box/session_settings.test.lua | 13 +++++++
7 files changed, 154 insertions(+)
--git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
index 006285622..dd42c8f5f 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -158,6 +158,7 @@ static Keyword aKeywordTable[] = {
{ "SAVEPOINT", "TK_SAVEPOINT", true },
{ "SCALAR", "TK_SCALAR", true },
{ "SELECT", "TK_SELECT", true },
+ { "SESSION", "TK_SESSION", false },
{ "SET", "TK_SET", true },
{ "SIMPLE", "TK_SIMPLE", true },
{ "START", "TK_START", true },
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index a00da31f9..aecba4177 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3481,3 +3481,23 @@ sql_session_settings_init()
setting->set = sql_session_setting_set;
}
}
+
+void
+sql_setting_set(struct Parse *parse_context, struct Token *name,
+ struct Expr *expr)
+{
+ struct Vdbe *vdbe = sqlGetVdbe(parse_context);
+ if (vdbe == NULL)
+ goto abort;
+ sqlVdbeCountChanges(vdbe);
+ char *key = sql_name_from_token(parse_context->db, name);
+ if (key == NULL)
+ goto abort;
+ int target = ++parse_context->nMem;
+ sqlExprCode(parse_context, expr, target);
+ sqlVdbeAddOp4(vdbe, OP_SetSession, target, 0, 0, key, P4_DYNAMIC);
+ return;
+abort:
+ parse_context->is_aborted = true;
+ return;
+}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 1a0e89703..995875566 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1541,6 +1541,12 @@ cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y). {
sql_drop_index(pParse);
}
+///////////////////////////// The SET SESSION command ////////////////////////
+//
+cmd ::= SET SESSION nm(X) EQ term(Y). {
+ sql_setting_set(pParse,&X,Y.pExpr);
+}
+
///////////////////////////// The PRAGMA command /////////////////////////////
//
cmd ::= PRAGMA nm(X). {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 2d6bad424..e45a7671d 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4517,4 +4517,15 @@ int
sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
uint32_t *fieldno);
+/**
+ * Create VDBE instructions to set the new value of the session setting.
+ *
+ * @param parse_context Parsing context.
+ * @param name Name of the session setting.
+ * @param value New value of the session setting.
+ */
+void
+sql_setting_set(struct Parse *parse_context, struct Token *name,
+ struct Expr *value);
+
#endif /* sqlINT_H */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index e8a029a8a..c3256c15a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -55,6 +55,7 @@
#include "box/schema.h"
#include "box/space.h"
#include "box/sequence.h"
+#include "box/session_settings.h"
/*
* Invoke this macro on memory cells just prior to changing the
@@ -5252,6 +5253,59 @@ case OP_IncMaxid: {
break;
}
+/* Opcode: SetSession P1 * * P4 *
+ *
+ * Set new value of the session setting. P4 is the name of the
+ * setting being updated, P1 is the register holding a value.
+ */
+case OP_SetSession: {
+ assert(pOp->p4type == P4_DYNAMIC);
+ const char *setting_name = pOp->p4.z;
+ int sid = session_setting_find(setting_name);
+ if (sid < 0) {
+ diag_set(ClientError, ER_NO_SUCH_SESSION_SETTING, setting_name);
+ goto abort_due_to_error;
+ }
+ pIn1 = &aMem[pOp->p1];
+ struct session_setting *setting = &session_settings[sid];
+ switch (setting->field_type) {
+ case FIELD_TYPE_BOOLEAN: {
+ if ((pIn1->flags & MEM_Bool) == 0)
+ goto invalid_type;
+ bool value = pIn1->u.b;
+ size_t size = mp_sizeof_bool(value);
+ char *mp_value = (char *) static_alloc(size);
+ mp_encode_bool(mp_value, value);
+ if (setting->set(sid, mp_value) != 0)
+ goto abort_due_to_error;
+ break;
+ }
+ case FIELD_TYPE_STRING: {
+ if ((pIn1->flags & MEM_Str) == 0)
+ goto invalid_type;
+ const char *str = pIn1->z;
+ uint32_t size = mp_sizeof_str(pIn1->n);
+ char *mp_value = (char *) static_alloc(size);
+ if (mp_value == NULL) {
+ diag_set(OutOfMemory, size, "static_alloc", "mp_value");
+ goto abort_due_to_error;
+ }
+ mp_encode_str(mp_value, str, pIn1->n);
+ if (setting->set(sid, mp_value) != 0)
+ goto abort_due_to_error;
+ break;
+ }
+ default:
+ invalid_type:
+ diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE,
+ session_setting_strs[sid],
+ field_type_strs[setting->field_type]);
+ goto abort_due_to_error;
+ }
+ p->nChange++;
+ break;
+}
+
/* Opcode: Noop * * * * *
*
* Do nothing. This instruction is often useful as a jump
diff --git a/test/box/session_settings.result b/test/box/session_settings.result
index b32a0becb..b82bdda94 100644
--- a/test/box/session_settings.result
+++ b/test/box/session_settings.result
@@ -339,6 +339,39 @@ settings.sql_defer_foreign_keys
| - false
| ...
+box.execute([[set session "sql_default_engine" = 'vinyl']])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_default_engine').value
+ | ---
+ | - vinyl
+ | ...
+box.execute([[set session "sql_default_engine" = 'memtx']])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_default_engine').value
+ | ---
+ | - memtx
+ | ...
+box.execute([[set session "sql_defer_foreign_keys" = true]])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_defer_foreign_keys').value
+ | ---
+ | - true
+ | ...
+box.execute([[set session "sql_defer_foreign_keys" = false]])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_defer_foreign_keys').value
+ | ---
+ | - false
+ | ...
+
settings.sql_default_engine = true
| ---
| - error: Session setting sql_default_engine expected a value of type string
@@ -359,3 +392,19 @@ box.session.settings.sql_default_engine = str
| ---
| - error: Failed to allocate 20483 bytes in static_alloc for mp_value
| ...
+
+box.execute([[set session "sql_def_engine" = true]])
+ | ---
+ | - null
+ | - Session setting sql_def_engine doesn't exist
+ | ...
+box.execute([[set session "sql_default_engine" = true]])
+ | ---
+ | - null
+ | - Session setting sql_default_engine expected a value of type string
+ | ...
+box.execute([[set session "sql_defer_foreign_keys" = 'true']])
+ | ---
+ | - null
+ | - Session setting sql_defer_foreign_keys expected a value of type boolean
+ | ...
diff --git a/test/box/session_settings.test.lua b/test/box/session_settings.test.lua
index 440bef7ce..d4aac0286 100644
--- a/test/box/session_settings.test.lua
+++ b/test/box/session_settings.test.lua
@@ -132,9 +132,22 @@ s:get('sql_defer_foreign_keys').value
s:update('sql_defer_foreign_keys', {{'=', 2, false}})
settings.sql_defer_foreign_keys
+box.execute([[set session "sql_default_engine" = 'vinyl']])
+s:get('sql_default_engine').value
+box.execute([[set session "sql_default_engine" = 'memtx']])
+s:get('sql_default_engine').value
+box.execute([[set session "sql_defer_foreign_keys" = true]])
+s:get('sql_defer_foreign_keys').value
+box.execute([[set session "sql_defer_foreign_keys" = false]])
+s:get('sql_defer_foreign_keys').value
+
settings.sql_default_engine = true
settings.sql_defer_foreign_keys = 'false'
settings.sql_parser_debug = 'string'
str = string.rep('a', 20 * 1024)
box.session.settings.sql_default_engine = str
+
+box.execute([[set session "sql_def_engine" = true]])
+box.execute([[set session "sql_default_engine" = true]])
+box.execute([[set session "sql_defer_foreign_keys" = 'true']])
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings
2020-04-10 15:40 ` Chris Sosnin
@ 2020-04-11 17:18 ` Vladislav Shpilevoy
2020-04-13 7:50 ` Timur Safin
1 sibling, 0 replies; 26+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-11 17:18 UTC (permalink / raw)
To: Chris Sosnin; +Cc: tarantool-patches
Hi! Thanks for the patchset!
I force pushed the style fix below.
LGTM now.
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 995875566..380fb83d1 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1544,7 +1544,7 @@ cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y). {
///////////////////////////// The SET SESSION command ////////////////////////
//
cmd ::= SET SESSION nm(X) EQ term(Y). {
- sql_setting_set(pParse,&X,Y.pExpr);
+ sql_setting_set(pParse, &X, Y.pExpr);
}
///////////////////////////// The PRAGMA command /////////////////////////////
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings
2020-04-10 15:40 ` Chris Sosnin
2020-04-11 17:18 ` Vladislav Shpilevoy
@ 2020-04-13 7:50 ` Timur Safin
1 sibling, 0 replies; 26+ messages in thread
From: Timur Safin @ 2020-04-13 7:50 UTC (permalink / raw)
To: 'Chris Sosnin', v.shpilevoy; +Cc: tarantool-patches
:
: I changed it to the following (the main problem with reserved word
: persists):
:
: +///////////////////////////// The SET SESSION command
: ////////////////////////
: +//
: +cmd ::= SET SESSION nm(X) EQ term(Y). {
: + sql_setting_set(pParse,&X,Y.pExpr);
: +}
: +
:
..
: ==========================================================================
: =====
:
: Currently if a user wants to change session setting with SQL, one has
: to execute UPDATE query like:
: [[UPDATE "_session_settings" SET "value" = true WHERE "name" = 'name']]
: However, direct access to system spaces isn't considered to be a good
: practice.
: To avoid that and a bit simplify user's life, we introduce SQL shortcut
: command
: SET SESSION.
:
: Closes #4711
:
: @TarantoolBot document
: Title: API for accessing _session_settings space.
: There are two ways of updating values of session settings:
: via Lua and SQL.
:
...
: SQL:
: Instead of typing long UPDATE query one can use the SET SESSION command:
: `box.execute([[SET SESSION "<setting_name>" = <new_value>]])`.
: Note, that this query is case sensitive so the name must be quoted.
: Also, SET SESSION doesn't provide any implicit casts, so <new_value> must
: be of the type corresponding to the setting being updated.
:
: Example:
: ```
: tarantool> box.execute([[set session "sql_default_engine" = 'memtx']])
: ---
: - row_count: 1
: ...
:
: tarantool> box.execute([[set session "sql_defer_foreign_keys" = true]])
: ---
: - row_count: 1
: ...
:
Like it!
(The least intrusive way possible, but still is short for user to write)
Timur
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/4] session settings fixes
2020-03-30 9:13 [Tarantool-patches] [PATCH 0/4] session settings fixes Chris Sosnin
` (3 preceding siblings ...)
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 4/4] sql: " Chris Sosnin
@ 2020-04-02 9:14 ` Timur Safin
2020-04-02 10:18 ` Chris Sosnin
2020-04-03 12:47 ` Nikita Pettik
2020-04-03 13:09 ` Nikita Pettik
2020-04-13 14:18 ` Kirill Yukhin
6 siblings, 2 replies; 26+ messages in thread
From: Timur Safin @ 2020-04-02 9:14 UTC (permalink / raw)
To: 'Chris Sosnin',
v.shpilevoy, korablev, tarantool-patches, Kirill Yukhin
I'm late here on this party, but I very much dislike proposed syntax of
SETTING SET configuration_name = value;
It feels not SQL-ish, and I see no much value introducing new statement
while there is still SET statement available.
Why we could not simply use something like:
SET box.session.settings.name = value;
?
Thanks,
Timur
: -----Original Message-----
: From: Tarantool-patches <tarantool-patches-bounces@dev.tarantool.org> On
: Behalf Of Chris Sosnin
: Sent: Monday, March 30, 2020 12:14 PM
: To: v.shpilevoy@tarantool.org; korablev@tarantool.org; tarantool-
: patches@dev.tarantool.org
: Subject: [Tarantool-patches] [PATCH 0/4] session settings fixes
:
: issue #1:https://github.com/tarantool/tarantool/issues/4711
: issue #2:https://github.com/tarantool/tarantool/issues/4712
: branch:https://github.com/tarantool/tarantool/tree/ksosnin/gh-4712-
: session-settings-v2
:
: Chris Sosnin (4):
: box: replace session_settings modules with a single array
: box: add binary search for _session_settings space
: box: provide a user friendly frontend for accessing session settings
: sql: provide a user friendly frontend for accessing session settings
:
: extra/mkkeywordhash.c | 1 +
: src/box/lua/session.c | 111 +++++++++
: src/box/session.cc | 1 +
: src/box/session.h | 2 +
: src/box/session_settings.c | 214 +++++++++++-------
: src/box/session_settings.h | 53 +++--
: src/box/sql.c | 5 -
: src/box/sql/build.c | 104 ++++-----
: src/box/sql/parse.y | 5 +
: src/box/sql/sqlInt.h | 11 +
: src/box/sql/vdbe.c | 50 ++++
: ...rontend.result => session_settings.result} | 147 ++++++++++--
: ...end.test.lua => session_settings.test.lua} | 61 ++++-
: 13 files changed, 589 insertions(+), 176 deletions(-)
: rename test/box/{gh-4511-access-settings-from-any-frontend.result =>
: session_settings.result} (71%)
: rename test/box/{gh-4511-access-settings-from-any-frontend.test.lua =>
: session_settings.test.lua} (64%)
:
: --
: 2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 0/4] session settings fixes
2020-04-02 9:14 ` [Tarantool-patches] [PATCH 0/4] session settings fixes Timur Safin
@ 2020-04-02 10:18 ` Chris Sosnin
2020-04-03 12:47 ` Nikita Pettik
1 sibling, 0 replies; 26+ messages in thread
From: Chris Sosnin @ 2020-04-02 10:18 UTC (permalink / raw)
To: Timur Safin; +Cc: tarantool-patches
> On 2 Apr 2020, at 12:14, Timur Safin <tsafin@tarantool.org> wrote:
>
> I'm late here on this party, but I very much dislike proposed syntax of
>
> SETTING SET configuration_name = value;
>
> It feels not SQL-ish, and I see no much value introducing new statement
> while there is still SET statement available.
>
> Why we could not simply use something like:
> SET box.session.settings.name = value;
> ?
>
> Thanks,
> Timur
It was SET <setting_name> = <new_value> in the first version.
However, we agreed on this syntax being too generic for configuring
session settings, so I added SETTING keyword.
>
>
> : -----Original Message-----
> : From: Tarantool-patches <tarantool-patches-bounces@dev.tarantool.org> On
> : Behalf Of Chris Sosnin
> : Sent: Monday, March 30, 2020 12:14 PM
> : To: v.shpilevoy@tarantool.org; korablev@tarantool.org; tarantool-
> : patches@dev.tarantool.org
> : Subject: [Tarantool-patches] [PATCH 0/4] session settings fixes
> :
> : issue #1:https://github.com/tarantool/tarantool/issues/4711
> : issue #2:https://github.com/tarantool/tarantool/issues/4712
> : branch:https://github.com/tarantool/tarantool/tree/ksosnin/gh-4712-
> : session-settings-v2
> :
> : Chris Sosnin (4):
> : box: replace session_settings modules with a single array
> : box: add binary search for _session_settings space
> : box: provide a user friendly frontend for accessing session settings
> : sql: provide a user friendly frontend for accessing session settings
> :
> : extra/mkkeywordhash.c | 1 +
> : src/box/lua/session.c | 111 +++++++++
> : src/box/session.cc | 1 +
> : src/box/session.h | 2 +
> : src/box/session_settings.c | 214 +++++++++++-------
> : src/box/session_settings.h | 53 +++--
> : src/box/sql.c | 5 -
> : src/box/sql/build.c | 104 ++++-----
> : src/box/sql/parse.y | 5 +
> : src/box/sql/sqlInt.h | 11 +
> : src/box/sql/vdbe.c | 50 ++++
> : ...rontend.result => session_settings.result} | 147 ++++++++++--
> : ...end.test.lua => session_settings.test.lua} | 61 ++++-
> : 13 files changed, 589 insertions(+), 176 deletions(-)
> : rename test/box/{gh-4511-access-settings-from-any-frontend.result =>
> : session_settings.result} (71%)
> : rename test/box/{gh-4511-access-settings-from-any-frontend.test.lua =>
> : session_settings.test.lua} (64%)
> :
> : --
> : 2.21.1 (Apple Git-122.3)
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 0/4] session settings fixes
2020-04-02 9:14 ` [Tarantool-patches] [PATCH 0/4] session settings fixes Timur Safin
2020-04-02 10:18 ` Chris Sosnin
@ 2020-04-03 12:47 ` Nikita Pettik
1 sibling, 0 replies; 26+ messages in thread
From: Nikita Pettik @ 2020-04-03 12:47 UTC (permalink / raw)
To: Timur Safin; +Cc: tarantool-patches, v.shpilevoy
On 02 Apr 12:14, Timur Safin wrote:
> I'm late here on this party, but I very much dislike proposed syntax of
Patch is not merged, so it's still okay to suggest changes.
> SETTING SET configuration_name = value;
>
> It feels not SQL-ish, and I see no much value introducing new statement
> while there is still SET statement available.
SET is not available as a separated statements; it's only a part
of UPDATE statements syntax. Rolling it back (SETTING SET -> SET)
is not a big deal - only one line of parse.y requires patching.
As I already said, I'm okay with all variations, whether it is
SET, SET SETTING or SETTING SET.
> Why we could not simply use something like:
> SET box.session.settings.name = value;
box.session.settings -- too long prefix. Originally, SET was
suggested as a shortcat for normal UPDATE "_session_settings" ...
SQL statement. Since it is shortcat, let's keep it short.
> ?
>
> Thanks,
> Timur
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/4] session settings fixes
2020-03-30 9:13 [Tarantool-patches] [PATCH 0/4] session settings fixes Chris Sosnin
` (4 preceding siblings ...)
2020-04-02 9:14 ` [Tarantool-patches] [PATCH 0/4] session settings fixes Timur Safin
@ 2020-04-03 13:09 ` Nikita Pettik
2020-04-03 14:02 ` Chris Sosnin
2020-04-13 14:18 ` Kirill Yukhin
6 siblings, 1 reply; 26+ messages in thread
From: Nikita Pettik @ 2020-04-03 13:09 UTC (permalink / raw)
To: Chris Sosnin; +Cc: tarantool-patches, v.shpilevoy
On 30 Mar 12:13, Chris Sosnin wrote:
> issue #1:https://github.com/tarantool/tarantool/issues/4711
> issue #2:https://github.com/tarantool/tarantool/issues/4712
> branch:https://github.com/tarantool/tarantool/tree/ksosnin/gh-4712-session-settings-v2
Nit: while sending next version of patch-set please specify it
explicitly with --subject-prefix='PATCH v2' and attach changelog
between versions. For example: https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/015103.html
> Chris Sosnin (4):
> box: replace session_settings modules with a single array
> box: add binary search for _session_settings space
> box: provide a user friendly frontend for accessing session settings
> sql: provide a user friendly frontend for accessing session settings
>
> extra/mkkeywordhash.c | 1 +
> src/box/lua/session.c | 111 +++++++++
> src/box/session.cc | 1 +
> src/box/session.h | 2 +
> src/box/session_settings.c | 214 +++++++++++-------
> src/box/session_settings.h | 53 +++--
> src/box/sql.c | 5 -
> src/box/sql/build.c | 104 ++++-----
> src/box/sql/parse.y | 5 +
> src/box/sql/sqlInt.h | 11 +
> src/box/sql/vdbe.c | 50 ++++
> ...rontend.result => session_settings.result} | 147 ++++++++++--
> ...end.test.lua => session_settings.test.lua} | 61 ++++-
> 13 files changed, 589 insertions(+), 176 deletions(-)
> rename test/box/{gh-4511-access-settings-from-any-frontend.result => session_settings.result} (71%)
> rename test/box/{gh-4511-access-settings-from-any-frontend.test.lua => session_settings.test.lua} (64%)
>
> --
> 2.21.1 (Apple Git-122.3)
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Tarantool-patches] [PATCH 0/4] session settings fixes
2020-04-03 13:09 ` Nikita Pettik
@ 2020-04-03 14:02 ` Chris Sosnin
0 siblings, 0 replies; 26+ messages in thread
From: Chris Sosnin @ 2020-04-03 14:02 UTC (permalink / raw)
To: Nikita Pettik; +Cc: tarantool-patches
It was v2, Vlad asked me to resend the whole patchset for
convenience. Won’t forget to do this next time.
> On 3 Apr 2020, at 16:09, Nikita Pettik <korablev@tarantool.org> wrote:
>
> On 30 Mar 12:13, Chris Sosnin wrote:
>> issue #1:https://github.com/tarantool/tarantool/issues/4711
>> issue #2:https://github.com/tarantool/tarantool/issues/4712
>> branch:https://github.com/tarantool/tarantool/tree/ksosnin/gh-4712-session-settings-v2
>
> Nit: while sending next version of patch-set please specify it
> explicitly with --subject-prefix='PATCH v2' and attach changelog
> between versions. For example: https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/015103.html
>
>> Chris Sosnin (4):
>> box: replace session_settings modules with a single array
>> box: add binary search for _session_settings space
>> box: provide a user friendly frontend for accessing session settings
>> sql: provide a user friendly frontend for accessing session settings
>>
>> extra/mkkeywordhash.c | 1 +
>> src/box/lua/session.c | 111 +++++++++
>> src/box/session.cc | 1 +
>> src/box/session.h | 2 +
>> src/box/session_settings.c | 214 +++++++++++-------
>> src/box/session_settings.h | 53 +++--
>> src/box/sql.c | 5 -
>> src/box/sql/build.c | 104 ++++-----
>> src/box/sql/parse.y | 5 +
>> src/box/sql/sqlInt.h | 11 +
>> src/box/sql/vdbe.c | 50 ++++
>> ...rontend.result => session_settings.result} | 147 ++++++++++--
>> ...end.test.lua => session_settings.test.lua} | 61 ++++-
>> 13 files changed, 589 insertions(+), 176 deletions(-)
>> rename test/box/{gh-4511-access-settings-from-any-frontend.result => session_settings.result} (71%)
>> rename test/box/{gh-4511-access-settings-from-any-frontend.test.lua => session_settings.test.lua} (64%)
>>
>> --
>> 2.21.1 (Apple Git-122.3)
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/4] session settings fixes
2020-03-30 9:13 [Tarantool-patches] [PATCH 0/4] session settings fixes Chris Sosnin
` (5 preceding siblings ...)
2020-04-03 13:09 ` Nikita Pettik
@ 2020-04-13 14:18 ` Kirill Yukhin
6 siblings, 0 replies; 26+ messages in thread
From: Kirill Yukhin @ 2020-04-13 14:18 UTC (permalink / raw)
To: Chris Sosnin; +Cc: v.shpilevoy, tarantool-patches
Hello,
On 30 Mar 12:13, Chris Sosnin wrote:
> issue #1:https://github.com/tarantool/tarantool/issues/4711
> issue #2:https://github.com/tarantool/tarantool/issues/4712
> branch:https://github.com/tarantool/tarantool/tree/ksosnin/gh-4712-session-settings-v2
>
> Chris Sosnin (4):
> box: replace session_settings modules with a single array
> box: add binary search for _session_settings space
> box: provide a user friendly frontend for accessing session settings
> sql: provide a user friendly frontend for accessing session settings
>
> extra/mkkeywordhash.c | 1 +
> src/box/lua/session.c | 111 +++++++++
> src/box/session.cc | 1 +
> src/box/session.h | 2 +
> src/box/session_settings.c | 214 +++++++++++-------
> src/box/session_settings.h | 53 +++--
> src/box/sql.c | 5 -
> src/box/sql/build.c | 104 ++++-----
> src/box/sql/parse.y | 5 +
> src/box/sql/sqlInt.h | 11 +
> src/box/sql/vdbe.c | 50 ++++
> ...rontend.result => session_settings.result} | 147 ++++++++++--
> ...end.test.lua => session_settings.test.lua} | 61 ++++-
> 13 files changed, 589 insertions(+), 176 deletions(-)
> rename test/box/{gh-4511-access-settings-from-any-frontend.result => session_settings.result} (71%)
> rename test/box/{gh-4511-access-settings-from-any-frontend.test.lua => session_settings.test.lua} (64%)
I've checked 1, 3 and 4 patches into master.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 26+ messages in thread