From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 A157246970E for ; Thu, 26 Dec 2019 21:01:18 +0300 (MSK) Date: Thu, 26 Dec 2019 21:01:16 +0300 From: Mergen Imeev Message-ID: <20191226180116.GB5318@tarantool.org> References: <2e14f6aca89173a785a6e2ee5c58b1eb211612ab.1576743850.git.imeevma@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH 2/3] box: introduce _session_settings system space List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Thank you for review and fixes! I haven't found any issues with them. New patch below. On Sat, Dec 21, 2019 at 06:59:12PM +0100, Vladislav Shpilevoy wrote: > Thanks for the patch! > > I've pushed my review fixes on top of this commit. See it below > and on the branch. If you agree, then squash. Otherwise lets > discuss. There are lots of fixes, so review carefully. In my > commit you can find inlined explanations about some changes. > > Also see 3 comments below inlined into your commit. > > During the review I also thought more about how to implement setting > modules - in a single big namespace of all session settings, each > with individual session settings, or like I proposed and you've done > here, with modules approach. > > Looks like I was wrong, and the only extendible option is to have a > single namespace, sorry. This is because not all settings can be split > into submodules and not all of them will have the same prefix. For > example, setting 'read_only'. I can't find a good prefix for it. > Additionally, some SQL settings may migrate down to box in future, such > as foreign key- and trigger- related. And they also likely not to have > one prefix. > > Nonetheless, I've decided that we can move settings to one namespace > later, as a separate simple ticket. Here we will finish public API. > > We can add individual settings in the current solution too, but it will > look like a module with a single setting. > > Also we need to optimize search. Currently it is fullscan what is > not acceptable when we have a sorted array. But also, a subject for a > separate issue. > > On 19/12/2019 09:32, imeevma@tarantool.org wrote: > > This patch creates _session_settings system space. This space is > > used to view and change session settings. There are no settings at > > the moment, some will be added in the next patch. > > > > Part of #4511 > > --- > > src/box/CMakeLists.txt | 1 + > > src/box/bootstrap.snap | Bin 5921 -> 5975 bytes > > src/box/lua/space.cc | 2 + > > src/box/lua/upgrade.lua | 15 + > > src/box/schema_def.h | 8 + > > src/box/session_settings.c | 409 +++++++++++++++++++++ > > src/box/session_settings.h | 61 +++ > > src/box/virtual_engine.c | 52 ++- > > test/app-tap/tarantoolctl.test.lua | 4 +- > > test/box-py/bootstrap.result | 3 + > > test/box/access_sysview.result | 6 +- > > test/box/alter.result | 5 +- > > ...h-4511-access-settings-from-any-frontend.result | 108 +++++- > > ...4511-access-settings-from-any-frontend.test.lua | 37 ++ > > test/wal_off/alter.result | 2 +- > > 15 files changed, 697 insertions(+), 16 deletions(-) > > create mode 100644 src/box/session_settings.c > > create mode 100644 src/box/session_settings.h > > > > diff --git a/src/box/session_settings.c b/src/box/session_settings.c > > new file mode 100644 > > index 0000000..b5b5db8 > > --- /dev/null > > +++ b/src/box/session_settings.c > > @@ -0,0 +1,409 @@ > > + > > +static int > > +session_settings_iterator_next(struct iterator *iterator, struct tuple **result) > > +{ > > + struct session_settings_iterator *it = > > + (struct session_settings_iterator *)iterator; > > + if (it->module_id >= SESSION_SETTING_MODULE_max || it->module_id < 0) { > > + *result = NULL; > > + return 0; > > + } > > + > > + struct tuple *ret = NULL; > > + int i = it->module_id; > > + int j = it->current_id; > > + enum iterator_type type = it->iterator_type; > > + struct tuple_format *format = it->format; > > + struct session_settings_modules *module; > > + const char *key = it->key; > > + > > + if (!iterator_type_is_reverse(type)) { > > 1. That condition never changes after iteration creation. You could > reuse virtuality of iterator.next to calculate that condition only > once. For that I've split that function in two: next and prev > separately. > > > + for (; i < SESSION_SETTING_MODULE_max; ++i) { > > + module = &session_settings_modules[i]; > > + if (module->get(format, j, key, type, &j, &ret) != 0) > > + return -1; > > + if (ret != NULL) > > + break; > > + j = 0; > > + } > > + *result = ret; > > + if (i == SESSION_SETTING_MODULE_max) { > > + it->module_id = SESSION_SETTING_MODULE_max; > > + it->current_id = 0; > > + return 0; > > + } > > + ++j; > > + if (j >= (int)module->size) { > > + ++i; > > + j = 0; > > + } > > + it->module_id = i; > > + it->current_id = j; > > + return 0; > > + } > > + > > + for (; i >= 0; --i) { > > + module = &session_settings_modules[i]; > > + if (module->get(format, j, key, type, &j, &ret) != 0) > > + return -1; > > + if (ret != NULL) > > + break; > > + if (i > 0) > > + j = session_settings_modules[i - 1].size - 1; > > + } > > + *result = ret; > > + if (i < 0) { > > + it->module_id = -1; > > + it->current_id = 0; > > + return 0; > > + } > > + --j; > > + if (j < 0) { > > + --i; > > + if (i >= 0) > > + j = module[i].size - 1; > > + } > > + it->module_id = i; > > + it->current_id = j; > > + return 0; > > +} > > diff --git a/src/box/session_settings.h b/src/box/session_settings.h > > new file mode 100644 > > index 0000000..e7ecbdb > > --- /dev/null > > +++ b/src/box/session_settings.h > > @@ -0,0 +1,61 @@ > > +#if defined(__cplusplus) > > +extern "C" { > > +#endif /* defined(__cplusplus) */ > > + > > +enum session_setting_module { > > + SESSION_SETTING_MODULE_max > > 2. Usually we do vice versa - lowercase enum names, > and uppercase _MAX. That makes possible to use things > such as STR2ENUM. > > > +}; > > + > > +struct tuple; > > +struct tuple_format; > > + > > +struct session_settings_modules { > > + enum session_setting_module id; > > + uint32_t size; > > + int (*get)(struct tuple_format *format, int id, const char *key, > > + enum iterator_type type, int *end_id, struct tuple **result); > > + int (*set)(struct tuple_format *format, int id, const char *value, > > + struct tuple **result); > > 3. The thing I mostly didn't like here is that you force submodule > to deal with tuple format, to create tuples, to do search by key. > That is a space's and index's task, not of SQL or other submodule. > Moreover, that logic would be duplicated in each submodule. > > I simplified this API so now the session_settings space internals do > all the search, comparisons, tuple creation, etc. And submodules only > provide setting name list, and API to get/set a setting by ID. > > > +}; > > + > > +extern struct session_settings_modules session_settings_modules[]; > > +extern const struct space_vtab session_settings_space_vtab; > > + > > My commit with explanations: > > ================================================================== > > commit 38d1392d20ee90f4bf5418e2402dd5ec589cfb1d > Author: Vladislav Shpilevoy > Date: Sat Dec 21 18:04:14 2019 +0100 > > Review fixes 2/3 > > diff --git a/src/box/session_settings.c b/src/box/session_settings.c > index b5b5db81e..dd1874b33 100644 > --- a/src/box/session_settings.c > +++ b/src/box/session_settings.c > @@ -31,35 +31,49 @@ > #include "session_settings.h" > #include "xrow_update.h" > #include "virtual_engine.h" > +#include "column_mask.h" > #include "session.h" > #include "schema.h" > #include "tuple.h" > #include "xrow.h" > #include "sql.h" > > -struct session_settings_modules > - session_settings_modules[SESSION_SETTING_MODULE_max] = {}; > +struct session_setting_module > + session_setting_modules[session_setting_type_MAX] = {}; > > struct session_settings_index { > /** Base index. Must be the first member. */ > struct index base; > - /** Format to create tuples on the fly. */ > + /** > + * Format of the tuples iterators of this index return. It > + * is stored here so as not to lookup space each time to > + * get a format and create an iterator. > + */ > struct tuple_format *format; > }; > > struct session_settings_iterator { > /** Base iterator. Must be the first member. */ > struct iterator base; > - /** Format of the tuples this iterator returns. */ > + /** > + * Format of the tuples this iterator returns. It is > + * stored here so as not to lookup space each time to get > + * a format for selected tuples. > + */ > struct tuple_format *format; > - /** ID of current module in global list of the modules. */ > + /** > + * ID of the current session settings module in the global > + * list of the modules. > + */ > int module_id; > /** ID of the setting in current module. */ > - int current_id; > + int setting_id; > /** Decoded key. */ > char *key; > - /** Type of iterator. */ > - enum iterator_type iterator_type; > + /** True if the iterator returns only equal keys. */ > + bool is_eq; > + /** True if the iterator should include equal keys. */ > + bool is_including; > ================================================================== > > As you can see, I removed iterator type in favor of a couple of > flags. I've found it easier to use these flags in the code than > compare the type with all the known types each time. > > ================================================================== > }; > > static void > @@ -72,72 +86,114 @@ session_settings_iterator_free(struct iterator *ptr) > } > > static int > -session_settings_iterator_next(struct iterator *iterator, struct tuple **result) > +session_settings_next_in_module(const struct session_setting_module *module, > + int *sid, const char *key, bool is_eq, > + bool is_including) > ================================================================== > > I've moved search by name to the session_settings space. Now > submodules shouldn't care about it. And this is reused by all > modules (even though now there is only one) > > ================================================================== > { > - struct session_settings_iterator *it = > - (struct session_settings_iterator *)iterator; > - if (it->module_id >= SESSION_SETTING_MODULE_max || it->module_id < 0) { > - *result = NULL; > + int i = *sid; > + int count = module->setting_count; > + if (i >= 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); > + if ((cmp == 0 && is_including) || > + (cmp > 0 && !is_eq)) { > + *sid = i; > + return 0; > + } > } > + *sid = count; > + return -1; > +} > > - struct tuple *ret = NULL; > - int i = it->module_id; > - int j = it->current_id; > - enum iterator_type type = it->iterator_type; > - struct tuple_format *format = it->format; > - struct session_settings_modules *module; > - const char *key = it->key; > - > - if (!iterator_type_is_reverse(type)) { > - for (; i < SESSION_SETTING_MODULE_max; ++i) { > - module = &session_settings_modules[i]; > - if (module->get(format, j, key, type, &j, &ret) != 0) > - return -1; > - if (ret != NULL) > - break; > - j = 0; > - } > - *result = ret; > - if (i == SESSION_SETTING_MODULE_max) { > - it->module_id = SESSION_SETTING_MODULE_max; > - it->current_id = 0; > +static int > +session_settings_prev_in_module(const struct session_setting_module *module, > + 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 ((cmp == 0 && is_including) || > + (cmp < 0 && !is_eq)) { > + *sid = i; > return 0; > } > - ++j; > - if (j >= (int)module->size) { > - ++i; > - j = 0; > + } > + *sid = -1; > + return -1; > +} > + > +static int > +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; > + 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 = i; > - it->current_id = j; > + } > + it->module_id = mid; > + 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); > + *result = box_tuple_new(it->format, mp_pair, mp_pair_end); > ================================================================== > > Note, I use box_tuple_new(), because iterator API assumes that a > tuple is referenced somewhere, and that box can just bless a tuple > to the userland. Simple tuple_new() does not work here, because it > won't be deleted if anything will go wrong after its creation and > before tuple_bless() in box. > > ================================================================== > + return *result != NULL ? 0 : -1; > +} > > - for (; i >= 0; --i) { > - module = &session_settings_modules[i]; > - if (module->get(format, j, key, type, &j, &ret) != 0) > - return -1; > - if (ret != NULL) > +static int > +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; > + 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; > - if (i > 0) > - j = session_settings_modules[i - 1].size - 1; > + } > } > - *result = ret; > - if (i < 0) { > - it->module_id = -1; > - it->current_id = 0; > + it->module_id = mid; > + it->setting_id = sid - 1; > + if (!is_found) { > + *result = NULL; > return 0; > } > - --j; > - if (j < 0) { > - --i; > - if (i >= 0) > - j = module[i].size - 1; > - } > - it->module_id = i; > - it->current_id = j; > - return 0; > + const char *mp_pair, *mp_pair_end; > + module->get(sid, &mp_pair, &mp_pair_end); > + *result = box_tuple_new(it->format, mp_pair, mp_pair_end); > + return *result != NULL ? 0 : -1; > } > > static void > @@ -175,15 +231,23 @@ session_settings_index_create_iterator(struct index *base, > return NULL; > } > iterator_create(&it->base, base); > - it->base.next = session_settings_iterator_next; > it->base.free = session_settings_iterator_free; > it->key = decoded_key; > - it->iterator_type = type; > + 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->format = index->format; > - it->module_id = iterator_type_is_reverse(type) ? > - SESSION_SETTING_MODULE_max - 1 : 0; > - it->current_id = iterator_type_is_reverse(type) ? > - session_settings_modules[it->module_id].size - 1 : 0; > + 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; > + } > return (struct iterator *)it; > } > > @@ -192,23 +256,28 @@ session_settings_index_get(struct index *base, const char *key, > uint32_t part_count, struct tuple **result) > { > struct session_settings_index *index = > - (struct session_settings_index *)base; > + (struct session_settings_index *) base; > assert(part_count == 1); > - (void)part_count; > + (void) part_count; > uint32_t len; > - const char *tmp = mp_decode_str(&key, &len); > - const char *decoded_key = tt_cstr(tmp, len); > - struct tuple *ret = NULL; > - struct tuple_format *format = index->format; > - for (int i = 0; i < SESSION_SETTING_MODULE_max; ++i) { > - if (session_settings_modules[i].get(format, 0, decoded_key, > - ITER_EQ, NULL, &ret) != 0) > - return -1; > - if (ret != NULL) > - break; > + 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; > } > - *result = ret; > + *result = NULL; > return 0; > +found:; > + const char *mp_pair; > + const char *mp_pair_end; > + module->get(sid, &mp_pair, &mp_pair_end); > + *result = box_tuple_new(index->format, mp_pair, mp_pair_end); > + return *result != NULL ? 0 : -1; > } > > static const struct index_vtab session_settings_index_vtab = { > @@ -281,70 +350,64 @@ session_settings_space_execute_update(struct space *space, struct txn *txn, > struct tuple **result) > { > (void)txn; > - const char *data = request->key; > - uint32_t key_len = mp_decode_array(&data); > + struct tuple_format *format = space->format; > + const char *old_data, *old_data_end, *new_data; > + struct region *region = &fiber()->gc; > + size_t used = region_used(region); > + int rc = -1, sid = 0; > + struct index_def *pk_def = space->index[0]->def; > + uint64_t column_mask; > + > + const char *new_key, *key = request->key; > + uint32_t new_size, new_key_len, key_len = mp_decode_array(&key); > if (key_len == 0) { > diag_set(ClientError, ER_EXACT_MATCH, 1, 0); > return -1; > } > - if (key_len > 1 || mp_typeof(*data) != MP_STR) { > + if (key_len > 1 || mp_typeof(*key) != MP_STR) { > diag_set(ClientError, ER_KEY_PART_TYPE, 0, "string"); > return -1; > } > - uint32_t len; > - const char *tmp = mp_decode_str(&data, &len); > - const char *decoded_key = tt_cstr(tmp, len); > - > - int id; > - struct tuple *old_tuple = NULL; > - struct tuple_format *format = space->format; > - struct session_settings_modules *module = NULL; > - for (int i = 0; i < SESSION_SETTING_MODULE_max; ++i) { > - module = &session_settings_modules[i]; > - if (module->get(format, 0, decoded_key, ITER_EQ, &id, > - &old_tuple) != 0) > - return -1; > - if (old_tuple != NULL) > - break; > + 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 (old_tuple == NULL) > - return 0; > - > - uint32_t new_size = 0, bsize; > - const char *old_data = tuple_data_range(old_tuple, &bsize); > - const char *new_data = > - xrow_update_execute(request->tuple,request->tuple_end, > - old_data, old_data + bsize, format->dict, > - &new_size, request->index_base, NULL); > + *result = NULL; > + return 0; > +found: > + module->get(sid, &old_data, &old_data_end); > + new_data = xrow_update_execute(request->tuple, request->tuple_end, > + old_data, old_data_end, format->dict, > + &new_size, request->index_base, > + &column_mask); > if (new_data == NULL) > - return -1; > + goto finish; > + *result = box_tuple_new(format, new_data, new_data + new_size); > + if (*result == NULL) > + goto finish; > > - uint32_t array_len = mp_decode_array(&new_data); > - if (array_len != 2) > - if (format->exact_field_count != array_len) { > - diag_set(ClientError, ER_EXACT_FIELD_COUNT, > - array_len, format->exact_field_count); > - return -1; > - } > - > - if (mp_typeof(*new_data) != MP_STR) { > - diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, > - space_index(space, 0)->def->name, space_name(space)); > - return -1; > - } > - > - const char *value = new_data; > - mp_next(&value); > - mp_decode_array(&old_data); > - uint32_t size_a = mp_sizeof_str(len); > - uint32_t size_b = value - new_data; > - if (size_a != size_b || memcmp(old_data, new_data, size_a) != 0) { > - diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, > - space_index(space, 0)->def->name, space_name(space)); > - return -1; > + mp_decode_array(&new_data); > + new_key = mp_decode_str(&new_data, &new_key_len); > + if (!key_update_can_be_skipped(pk_def->key_def->column_mask, > + column_mask)) { > + if (key_len != new_key_len || > + memcmp(key, new_key, key_len) != 0) { > + diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, > + pk_def->name, space_name(space)); > + goto finish; > + } > } > - > - return module->set(format, id, value, result); > + if (module->set(sid, new_data) != 0) > + goto finish; > + rc = 0; > +finish: > + region_truncate(region, used); > + return rc; > } > > static int > diff --git a/src/box/session_settings.h b/src/box/session_settings.h > index e7ecbdb08..7415e0eea 100644 > --- a/src/box/session_settings.h > +++ b/src/box/session_settings.h > @@ -29,33 +29,41 @@ > * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > * SUCH DAMAGE. > */ > -#include > -#include > -#include "iterator_type.h" > > -#if defined(__cplusplus) > -extern "C" { > -#endif /* defined(__cplusplus) */ > - > -enum session_setting_module { > - SESSION_SETTING_MODULE_max > +/** > + * 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. > + * > + * The types should be ordered in alphabetical order, because the > + * type list is used by setting iterators. > + */ > +enum session_setting_type { > + session_setting_type_MAX, > }; > > -struct tuple; > -struct tuple_format; > - > -struct session_settings_modules { > - enum session_setting_module id; > - uint32_t size; > - int (*get)(struct tuple_format *format, int id, const char *key, > - enum iterator_type type, int *end_id, struct tuple **result); > - int (*set)(struct tuple_format *format, int id, const char *value, > - struct tuple **result); > +struct session_setting_module { > + /** > + * An array of setting names. All of them should have the > + * same prefix. > + */ > + const char **settings; > + /** Count of settings. */ > + int setting_count; > + /** > + * Get a MessagePack encoded pair [name, value] for a > + * setting having index @a id. Index is from the settings > + * array. > + */ > + void (*get)(int id, const char **mp_pair, const char **mp_pair_end); > + /** > + * Set value of a setting by a given @a id from a > + * MessagePack encoded buffer. Note, it is not a pair, but > + * just value. > + */ > + int (*set)(int id, const char *mp_value); > }; > > -extern struct session_settings_modules session_settings_modules[]; > -extern const struct space_vtab session_settings_space_vtab; > - > -#if defined(__cplusplus) > -} /* extern "C" */ > -#endif /* defined(__plusplus) */ > +extern struct session_setting_module session_setting_modules[]; > diff --git a/src/box/virtual_engine.c b/src/box/virtual_engine.c > index 5ec8015c7..36eec8323 100644 > --- a/src/box/virtual_engine.c > +++ b/src/box/virtual_engine.c > @@ -28,10 +28,12 @@ > * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > * SUCH DAMAGE. > */ > -#include "session_settings.h" > #include "virtual_engine.h" > +#include "tuple.h" > #include "schema.h" > > +extern const struct space_vtab session_settings_space_vtab; > + > static void > virtual_engine_shutdown(struct engine *engine) > { > >From 7efcc8500ae9c58ad12ee7eadc8833be70a590c1 Mon Sep 17 00:00:00 2001 From: Mergen Imeev Date: Sat, 30 Nov 2019 12:59:45 +0300 Subject: [PATCH] box: introduce _session_settings system space This patch creates _session_settings system space. This space is used to view and change session settings. There are no settings at the moment, some will be added in the next patch. Part of #4511 diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt index d79d52c..b57e90b 100644 --- a/src/box/CMakeLists.txt +++ b/src/box/CMakeLists.txt @@ -80,6 +80,7 @@ add_library(box STATIC sysview.c blackhole.c virtual_engine.c + session_settings.c vinyl.c vy_stmt.c vy_mem.c diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap index 7ff3aa5..c573eec 100644 Binary files a/src/box/bootstrap.snap and b/src/box/bootstrap.snap differ diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc index f6e96f0..01b58af 100644 --- a/src/box/lua/space.cc +++ b/src/box/lua/space.cc @@ -653,6 +653,8 @@ box_lua_space_init(struct lua_State *L) lua_setfield(L, -2, "SPACE_SEQUENCE_ID"); lua_pushnumber(L, BOX_FUNC_INDEX_ID); lua_setfield(L, -2, "FUNC_INDEX_ID"); + lua_pushnumber(L, BOX_SESSION_SETTINGS_ID); + lua_setfield(L, -2, "SESSION_SETTINGS_ID"); lua_pushnumber(L, BOX_SYSTEM_ID_MIN); lua_setfield(L, -2, "SYSTEM_ID_MIN"); lua_pushnumber(L, BOX_SYSTEM_ID_MAX); diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua index 07f1e03..4dfd571 100644 --- a/src/box/lua/upgrade.lua +++ b/src/box/lua/upgrade.lua @@ -951,8 +951,23 @@ local function drop_func_collation() _func.index.name:alter({parts = {{'name', 'string'}}}) end +local function create_session_settings_space() + local _space = box.space[box.schema.SPACE_ID] + local _index = box.space[box.schema.INDEX_ID] + local format = {} + format[1] = {name='name', type='string'} + format[2] = {name='value', type='any'} + log.info("create space _session_settings") + _space:insert{box.schema.SESSION_SETTINGS_ID, ADMIN, '_session_settings', + 'virtual', 2, {temporary = true}, format} + log.info("create index _session_settings:primary") + _index:insert{box.schema.SESSION_SETTINGS_ID, 0, 'primary', 'tree', + {unique = true}, {{0, 'string'}}} +end + local function upgrade_to_2_3_1() drop_func_collation() + create_session_settings_space() end -------------------------------------------------------------------------------- diff --git a/src/box/schema_def.h b/src/box/schema_def.h index ba870ff..f86cd42 100644 --- a/src/box/schema_def.h +++ b/src/box/schema_def.h @@ -114,6 +114,8 @@ enum { BOX_CK_CONSTRAINT_ID = 364, /** Space id of _func_index. */ BOX_FUNC_INDEX_ID = 372, + /** Space id of _session_settings. */ + BOX_SESSION_SETTINGS_ID = 380, /** End of the reserved range of system spaces. */ BOX_SYSTEM_ID_MAX = 511, BOX_ID_NIL = 2147483647 @@ -277,6 +279,12 @@ enum { BOX_FUNC_INDEX_FUNCTION_ID = 2, }; +/** _session_settings fields. */ +enum { + BOX_SESSION_SETTINGS_FIELD_NAME = 0, + BOX_SESSION_SETTINGS_FIELD_VALUE = 1, +}; + /* * Different objects which can be subject to access * control. diff --git a/src/box/session_settings.c b/src/box/session_settings.c new file mode 100644 index 0000000..dd1874b --- /dev/null +++ b/src/box/session_settings.c @@ -0,0 +1,472 @@ +/* + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ +#include "session_settings.h" +#include "xrow_update.h" +#include "virtual_engine.h" +#include "column_mask.h" +#include "session.h" +#include "schema.h" +#include "tuple.h" +#include "xrow.h" +#include "sql.h" + +struct session_setting_module + session_setting_modules[session_setting_type_MAX] = {}; + +struct session_settings_index { + /** Base index. Must be the first member. */ + struct index base; + /** + * Format of the tuples iterators of this index return. It + * is stored here so as not to lookup space each time to + * get a format and create an iterator. + */ + struct tuple_format *format; +}; + +struct session_settings_iterator { + /** Base iterator. Must be the first member. */ + struct iterator base; + /** + * Format of the tuples this iterator returns. It is + * stored here so as not to lookup space each time to get + * 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. */ + int setting_id; + /** Decoded key. */ + char *key; + /** True if the iterator returns only equal keys. */ + bool is_eq; + /** True if the iterator should include equal keys. */ + bool is_including; +}; + +static void +session_settings_iterator_free(struct iterator *ptr) +{ + struct session_settings_iterator *it = + (struct session_settings_iterator *)ptr; + free(it->key); + free(it); +} + +static int +session_settings_next_in_module(const struct session_setting_module *module, + int *sid, const char *key, bool is_eq, + bool is_including) +{ + int i = *sid; + int count = module->setting_count; + if (i >= 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); + if ((cmp == 0 && is_including) || + (cmp > 0 && !is_eq)) { + *sid = i; + return 0; + } + } + *sid = 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) +{ + 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 ((cmp == 0 && is_including) || + (cmp < 0 && !is_eq)) { + *sid = i; + return 0; + } + } + *sid = -1; + return -1; +} + +static int +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; + 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; + 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); + *result = box_tuple_new(it->format, mp_pair, mp_pair_end); + return *result != NULL ? 0 : -1; +} + +static int +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; + 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; + 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); + *result = box_tuple_new(it->format, mp_pair, mp_pair_end); + return *result != NULL ? 0 : -1; +} + +static void +session_settings_index_destroy(struct index *index) +{ + free(index); +} + +static struct iterator * +session_settings_index_create_iterator(struct index *base, + enum iterator_type type, const char *key, + uint32_t part_count) +{ + struct session_settings_index *index = + (struct session_settings_index *)base; + char *decoded_key = NULL; + if (part_count > 0) { + assert(part_count == 1); + assert(mp_typeof(*key) == MP_STR); + uint32_t len; + const char *name = mp_decode_str(&key, &len); + decoded_key = (char *)malloc(len + 1); + if (decoded_key == NULL) { + diag_set(OutOfMemory, len + 1, "malloc", "decoded_key"); + return NULL; + } + memcpy(decoded_key, name, len); + decoded_key[len] = '\0'; + } + struct session_settings_iterator *it = + (struct session_settings_iterator *)malloc(sizeof(*it)); + if (it == NULL) { + diag_set(OutOfMemory, sizeof(*it), "malloc", "it"); + free(decoded_key); + return NULL; + } + iterator_create(&it->base, base); + it->base.free = session_settings_iterator_free; + it->key = decoded_key; + 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->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; + } + return (struct iterator *)it; +} + +static int +session_settings_index_get(struct index *base, const char *key, + uint32_t part_count, struct tuple **result) +{ + struct session_settings_index *index = + (struct session_settings_index *) base; + assert(part_count == 1); + (void) part_count; + 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; + } + *result = NULL; + return 0; +found:; + const char *mp_pair; + const char *mp_pair_end; + module->get(sid, &mp_pair, &mp_pair_end); + *result = box_tuple_new(index->format, mp_pair, mp_pair_end); + return *result != NULL ? 0 : -1; +} + +static const struct index_vtab session_settings_index_vtab = { + /* .destroy = */ session_settings_index_destroy, + /* .commit_create = */ generic_index_commit_create, + /* .abort_create = */ generic_index_abort_create, + /* .commit_modify = */ generic_index_commit_modify, + /* .commit_drop = */ generic_index_commit_drop, + /* .update_def = */ generic_index_update_def, + /* .depends_on_pk = */ generic_index_depends_on_pk, + /* .def_change_requires_rebuild = */ + generic_index_def_change_requires_rebuild, + /* .size = */ generic_index_size, + /* .bsize = */ generic_index_bsize, + /* .min = */ generic_index_min, + /* .max = */ generic_index_max, + /* .random = */ generic_index_random, + /* .count = */ generic_index_count, + /* .get = */ session_settings_index_get, + /* .replace = */ generic_index_replace, + /* .create_iterator = */ session_settings_index_create_iterator, + /* .create_snapshot_iterator = */ + generic_index_create_snapshot_iterator, + /* .stat = */ generic_index_stat, + /* .compact = */ generic_index_compact, + /* .reset_stat = */ generic_index_reset_stat, + /* .begin_build = */ generic_index_begin_build, + /* .reserve = */ generic_index_reserve, + /* .build_next = */ generic_index_build_next, + /* .end_build = */ generic_index_end_build, +}; + +static void +session_settings_space_destroy(struct space *space) +{ + free(space); +} + +static int +session_settings_space_execute_replace(struct space *space, struct txn *txn, + struct request *request, + struct tuple **result) +{ + (void)space; + (void)txn; + (void)request; + (void)result; + diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space", + "replace()"); + return -1; +} + +static int +session_settings_space_execute_delete(struct space *space, struct txn *txn, + struct request *request, + struct tuple **result) +{ + (void)space; + (void)txn; + (void)request; + (void)result; + diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space", + "delete()"); + return -1; +} + +static int +session_settings_space_execute_update(struct space *space, struct txn *txn, + struct request *request, + struct tuple **result) +{ + (void)txn; + struct tuple_format *format = space->format; + const char *old_data, *old_data_end, *new_data; + struct region *region = &fiber()->gc; + size_t used = region_used(region); + int rc = -1, sid = 0; + struct index_def *pk_def = space->index[0]->def; + uint64_t column_mask; + + const char *new_key, *key = request->key; + uint32_t new_size, new_key_len, key_len = mp_decode_array(&key); + if (key_len == 0) { + diag_set(ClientError, ER_EXACT_MATCH, 1, 0); + return -1; + } + if (key_len > 1 || mp_typeof(*key) != MP_STR) { + diag_set(ClientError, ER_KEY_PART_TYPE, 0, "string"); + return -1; + } + 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; + } + *result = NULL; + return 0; +found: + module->get(sid, &old_data, &old_data_end); + new_data = xrow_update_execute(request->tuple, request->tuple_end, + old_data, old_data_end, format->dict, + &new_size, request->index_base, + &column_mask); + if (new_data == NULL) + goto finish; + *result = box_tuple_new(format, new_data, new_data + new_size); + if (*result == NULL) + goto finish; + + mp_decode_array(&new_data); + new_key = mp_decode_str(&new_data, &new_key_len); + if (!key_update_can_be_skipped(pk_def->key_def->column_mask, + column_mask)) { + if (key_len != new_key_len || + memcmp(key, new_key, key_len) != 0) { + diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, + pk_def->name, space_name(space)); + goto finish; + } + } + if (module->set(sid, new_data) != 0) + goto finish; + rc = 0; +finish: + region_truncate(region, used); + return rc; +} + +static int +session_settings_space_execute_upsert(struct space *space, struct txn *txn, + struct request *request) +{ + (void)space; + (void)txn; + (void)request; + diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space", + "upsert()"); + return -1; +} + +static struct index * +session_settings_space_create_index(struct space *space, struct index_def *def) +{ + assert(space->def->id == BOX_SESSION_SETTINGS_ID); + if (def->iid != 0) { + diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space", + "create_index()"); + return NULL; + } + + struct session_settings_index *index = + (struct session_settings_index *)calloc(1, sizeof(*index)); + if (index == NULL) { + diag_set(OutOfMemory, sizeof(*index), "calloc", "index"); + return NULL; + } + if (index_create(&index->base, space->engine, + &session_settings_index_vtab, def) != 0) { + free(index); + return NULL; + } + + index->format = space->format; + return &index->base; +} + +const struct space_vtab session_settings_space_vtab = { + /* .destroy = */ session_settings_space_destroy, + /* .bsize = */ generic_space_bsize, + /* .execute_replace = */ session_settings_space_execute_replace, + /* .execute_delete = */ session_settings_space_execute_delete, + /* .execute_update = */ session_settings_space_execute_update, + /* .execute_upsert = */ session_settings_space_execute_upsert, + /* .ephemeral_replace = */ generic_space_ephemeral_replace, + /* .ephemeral_delete = */ generic_space_ephemeral_delete, + /* .ephemeral_rowid_next = */ generic_space_ephemeral_rowid_next, + /* .init_system_space = */ generic_init_system_space, + /* .init_ephemeral_space = */ generic_init_ephemeral_space, + /* .check_index_def = */ generic_space_check_index_def, + /* .create_index = */ session_settings_space_create_index, + /* .add_primary_key = */ generic_space_add_primary_key, + /* .drop_primary_key = */ generic_space_drop_primary_key, + /* .check_format = */ generic_space_check_format, + /* .build_index = */ generic_space_build_index, + /* .swap_index = */ generic_space_swap_index, + /* .prepare_alter = */ generic_space_prepare_alter, + /* .invalidate = */ generic_space_invalidate, +}; diff --git a/src/box/session_settings.h b/src/box/session_settings.h new file mode 100644 index 0000000..7415e0e --- /dev/null +++ b/src/box/session_settings.h @@ -0,0 +1,69 @@ +#pragma once +/* + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/** + * 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. + * + * The types should be ordered in alphabetical order, because the + * type list is used by setting iterators. + */ +enum session_setting_type { + session_setting_type_MAX, +}; + +struct session_setting_module { + /** + * An array of setting names. All of them should have the + * same prefix. + */ + const char **settings; + /** Count of settings. */ + int setting_count; + /** + * Get a MessagePack encoded pair [name, value] for a + * setting having index @a id. Index is from the settings + * array. + */ + void (*get)(int id, const char **mp_pair, const char **mp_pair_end); + /** + * Set value of a setting by a given @a id from a + * MessagePack encoded buffer. Note, it is not a pair, but + * just value. + */ + int (*set)(int id, const char *mp_value); +}; + +extern struct session_setting_module session_setting_modules[]; diff --git a/src/box/virtual_engine.c b/src/box/virtual_engine.c index 9a59a3f..36eec83 100644 --- a/src/box/virtual_engine.c +++ b/src/box/virtual_engine.c @@ -29,8 +29,11 @@ * SUCH DAMAGE. */ #include "virtual_engine.h" +#include "tuple.h" #include "schema.h" +extern const struct space_vtab session_settings_space_vtab; + static void virtual_engine_shutdown(struct engine *engine) { @@ -41,13 +44,50 @@ static struct space * virtual_engine_create_space(struct engine *engine, struct space_def *def, struct rlist *key_list) { - (void)engine; - (void)def; - (void)key_list; - /* There are currently no spaces with this engine. */ - diag_set(ClientError, ER_UNSUPPORTED, "Tarantool", - "spaces with this engine."); - return NULL; + /* + * At the moment the only space that have this engine is + * _session_sessings. + */ + if (def->id != BOX_SESSION_SETTINGS_ID) { + diag_set(ClientError, ER_UNSUPPORTED, "Tarantool", + "non-system space with this engine."); + return NULL; + } + const struct space_vtab *space_vtab = &session_settings_space_vtab; + + struct space *space = (struct space *)calloc(1, sizeof(*space)); + if (space == NULL) { + diag_set(OutOfMemory, sizeof(*space), "calloc", "space"); + return NULL; + } + int key_count = 0; + struct key_def **keys = index_def_to_key_def(key_list, &key_count); + if (keys == NULL) { + free(space); + return NULL; + } + struct tuple_format *format = + tuple_format_new(&tuple_format_runtime->vtab, NULL, keys, + key_count, def->fields, def->field_count, + def->exact_field_count, def->dict, + def->opts.is_temporary, + def->opts.is_ephemeral); + if (format == NULL) { + free(space); + return NULL; + } + tuple_format_ref(format); + int rc = space_create(space, engine, space_vtab, def, key_list, format); + /* + * Format is now referenced by the space if space has beed + * created. + */ + tuple_format_unref(format); + if (rc != 0) { + free(space); + return NULL; + } + return space; } static const struct engine_vtab virtual_engine_vtab = { diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua index 7a07860..4d70595 100755 --- a/test/app-tap/tarantoolctl.test.lua +++ b/test/app-tap/tarantoolctl.test.lua @@ -415,8 +415,8 @@ do check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1", "\n", 3) check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1 --replica 2", "\n", 3) check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 2", "\n", 0) - check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 24) - check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 52) + check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 25) + check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 53) end) end) diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result index 938a763..f2ad75e 100644 --- a/test/box-py/bootstrap.result +++ b/test/box-py/bootstrap.result @@ -96,6 +96,8 @@ box.space._space:select{} 'type': 'boolean'}]] - [372, 1, '_func_index', 'memtx', 0, {}, [{'name': 'space_id', 'type': 'unsigned'}, {'name': 'index_id', 'type': 'unsigned'}, {'name': 'func_id', 'type': 'unsigned'}]] + - [380, 1, '_session_settings', 'virtual', 2, {'temporary': true}, [{'name': 'name', + 'type': 'string'}, {'name': 'value', 'type': 'any'}]] ... box.space._index:select{} --- @@ -153,6 +155,7 @@ box.space._index:select{} - [364, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'string']]] - [372, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]] - [372, 1, 'fid', 'tree', {'unique': false}, [[2, 'unsigned']]] + - [380, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]] ... box.space._user:select{} --- diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result index 1f33dec..799d19f 100644 --- a/test/box/access_sysview.result +++ b/test/box/access_sysview.result @@ -246,11 +246,11 @@ box.session.su('guest') ... #box.space._vspace:select{} --- -- 25 +- 26 ... #box.space._vindex:select{} --- -- 53 +- 54 ... #box.space._vuser:select{} --- @@ -282,7 +282,7 @@ box.session.su('guest') ... #box.space._vindex:select{} --- -- 53 +- 54 ... #box.space._vuser:select{} --- diff --git a/test/box/alter.result b/test/box/alter.result index 9a2f991..f150faa 100644 --- a/test/box/alter.result +++ b/test/box/alter.result @@ -92,7 +92,7 @@ space = box.space[t[1]] ... space.id --- -- 373 +- 381 ... space.field_count --- @@ -137,7 +137,7 @@ space_deleted ... space:replace{0} --- -- error: Space '373' does not exist +- error: Space '381' does not exist ... _index:insert{_space.id, 0, 'primary', 'tree', {unique=true}, {{0, 'unsigned'}}} --- @@ -220,6 +220,7 @@ _index:select{} - [364, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'string']]] - [372, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]] - [372, 1, 'fid', 'tree', {'unique': false}, [[2, 'unsigned']]] + - [380, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]] ... -- modify indexes of a system space _index:delete{_index.id, 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 9874616..75d53cf 100644 --- a/test/box/gh-4511-access-settings-from-any-frontend.result +++ b/test/box/gh-4511-access-settings-from-any-frontend.result @@ -6,5 +6,111 @@ test_run = require('test_run').new() -- User cannot create spaces with this engine. s = box.schema.space.create('test', {engine = 'virtual'}) | --- - | - error: Tarantool does not support spaces with this engine. + | - error: Tarantool does not support non-system space with this engine. + | ... + +-- Check _session_settings space. +s = box.space._session_settings + | --- + | ... +s:format() + | --- + | - [{'name': 'name', 'type': 'string'}, {'name': 'value', 'type': 'any'}] + | ... + +-- Make sure that we cannot drop space. +s:drop() + | --- + | - error: Can't drop the primary key in a system space, space '_session_settings' + | ... + +-- +-- Make sure, that session_settings space doesn't support +-- create_index(), insert(), replace() and delete() methods. +-- +s:create_index('a') + | --- + | - error: Session_settings space does not support create_index() + | ... +s:insert({'a', 1}) + | --- + | - error: Session_settings space does not support replace() + | ... +s:delete({'b'}) + | --- + | - error: Session_settings space does not support delete() + | ... +s:replace({'sql_defer_foreign_keys', true}) + | --- + | - error: Session_settings space does not support replace() + | ... + +-- Check get() and select(). They should return nothing for now. +s:get({'a'}) + | --- + | ... +s:select() + | --- + | - [] + | ... +s:select({}, {iterator='EQ'}) + | --- + | - [] + | ... +s:select({}, {iterator='ALL'}) + | --- + | - [] + | ... +s:select({}, {iterator='GE'}) + | --- + | - [] + | ... +s:select({}, {iterator='GT'}) + | --- + | - [] + | ... +s:select({}, {iterator='REQ'}) + | --- + | - [] + | ... +s:select({}, {iterator='LE'}) + | --- + | - [] + | ... +s:select({}, {iterator='LT'}) + | --- + | - [] + | ... +s:select({'a'}, {iterator='EQ'}) + | --- + | - [] + | ... +s:select({'a'}, {iterator='ALL'}) + | --- + | - [] + | ... +s:select({'a'}, {iterator='GE'}) + | --- + | - [] + | ... +s:select({'a'}, {iterator='GT'}) + | --- + | - [] + | ... +s:select({'a'}, {iterator='REQ'}) + | --- + | - [] + | ... +s:select({'a'}, {iterator='LE'}) + | --- + | - [] + | ... +s:select({'a'}, {iterator='LT'}) + | --- + | - [] + | ... + +-- Currently there is nothing to update, but update() should work. +s:update('some_option', {{'=', 'value', true}}) + | --- | ... 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 611caef..3304454 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 @@ -2,3 +2,40 @@ test_run = require('test_run').new() -- User cannot create spaces with this engine. s = box.schema.space.create('test', {engine = 'virtual'}) + +-- Check _session_settings space. +s = box.space._session_settings +s:format() + +-- Make sure that we cannot drop space. +s:drop() + +-- +-- Make sure, that session_settings space doesn't support +-- create_index(), insert(), replace() and delete() methods. +-- +s:create_index('a') +s:insert({'a', 1}) +s:delete({'b'}) +s:replace({'sql_defer_foreign_keys', true}) + +-- Check get() and select(). They should return nothing for now. +s:get({'a'}) +s:select() +s:select({}, {iterator='EQ'}) +s:select({}, {iterator='ALL'}) +s:select({}, {iterator='GE'}) +s:select({}, {iterator='GT'}) +s:select({}, {iterator='REQ'}) +s:select({}, {iterator='LE'}) +s:select({}, {iterator='LT'}) +s:select({'a'}, {iterator='EQ'}) +s:select({'a'}, {iterator='ALL'}) +s:select({'a'}, {iterator='GE'}) +s:select({'a'}, {iterator='GT'}) +s:select({'a'}, {iterator='REQ'}) +s:select({'a'}, {iterator='LE'}) +s:select({'a'}, {iterator='LT'}) + +-- Currently there is nothing to update, but update() should work. +s:update('some_option', {{'=', 'value', true}}) diff --git a/test/wal_off/alter.result b/test/wal_off/alter.result index 62cb11d..97f7e6f 100644 --- a/test/wal_off/alter.result +++ b/test/wal_off/alter.result @@ -28,7 +28,7 @@ end; ... #spaces; --- -- 65502 +- 65501 ... -- cleanup for k, v in pairs(spaces) do