From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 478B4452566 for ; Thu, 31 Oct 2019 14:26:39 +0300 (MSK) Date: Thu, 31 Oct 2019 14:26:36 +0300 From: Mergen Imeev Message-ID: <20191031112636.GA18545@tarantool.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH v2 2/5] box: introdice _vsession_settings sysview List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org Thank you for review! My answers and new patch below. On Sat, Oct 26, 2019 at 07:35:27PM +0200, Vladislav Shpilevoy wrote: > Hi! Thanks for the fixes! > > On 25/10/2019 17:45, imeevma@tarantool.org wrote: > > Thank you for review! My answers, one question and new patch > > below. > > > > On 10/19/19 1:08 AM, Vladislav Shpilevoy wrote: > >> Thanks for the patch! > >> > >> See 14 comments below. > >> > >> On 17/10/2019 16:40, imeevma@tarantool.org wrote: > >>> This patch creates the _vsql_settings sysview. This system view > >>> has custom methods get() and create_iterator(). This allows us > >>> to get SQL settings by creating a tuple on the fly without having > >>> to save it anywhere. > >>> > >>> Part of #4511 > >>> > >>> @TarantoolBot document > >> > >> 1. I think, it is worth to create one doc request for the > >> whole feature - both SET command and _vsql_settings space. > >> They are useless without each other. > >> > > Fixed. However, do I need to write a doc-bot request now when the > > sysview purpose has changed? > > Nah, I still think one request for the whole _vsession_settings > feature is enough. You describe what is _vsession_settings, > which options it has already, how to set them, and it should be > sufficient. > > > diff --git a/src/box/session.cc b/src/box/session.cc > > index 59bf226..32dc87a 100644 > > --- a/src/box/session.cc > > +++ b/src/box/session.cc > > Recently I've seen a ticket https://github.com/tarantool/tarantool/issues/4205 > about read-only sessions. That looks like a first candidate on a > non-SQL session local setting. > > That means we probably need to design how to make session > settings iterator more generic. Currently all the settings > iteration code consists of explicit SQL things. > > On the other hand I don't really know whether we should > consider read-only option a session setting. > And do we need to create a more generic session settings > iterator in scope of this patchset? > > Here is my personal opinion how it should be done: > > - We push this patchset with current SQL-centric design of > the settings iteration to stabilize the public API as > soon as possible; > > - Later in scope of 4205 we add session-local option > read-only and reimplement session settings iterator so > as it would not depend on SQL so much. > > > See 6 comments below. > > > @@ -360,3 +363,116 @@ generic_session_sync(struct session *session) > > + > > +struct iterator * > > +session_options_create_iterator(struct index *index, enum iterator_type type, > > + const char *key, uint32_t part_count) > > +{ > > + int max_tuples_to_show = -1; > > + uint32_t sql_option_id; > > + if (part_count > 0) { > > + assert(part_count == 1); > > + uint32_t len; > > + const char *name = mp_decode_str(&key, &len); > > + name = tt_cstr(name, len); > > + sql_option_id = sql_option_id_by_name(name); > > + if (type == ITER_EQ || type == ITER_REQ) > > + max_tuples_to_show = 1; > > + else if (type == ITER_LT) > > + --sql_option_id; > > + else if (type == ITER_GT) > > + ++sql_option_id; > > + if (sql_option_id == sql_option_id_max() && type == ITER_LE) > > + --sql_option_id; > > 1. Strangely GE does not return anything here: > > tarantool> box.space._vsession_settings:select({'sql_'}, {iterator = 'GE'}) > --- > - [] > ... > > LE also behaves strange: > > tarantool> box.space._vsession_settings:select({'sql_d'}, {iterator = 'LE'}) > --- > - - ['sql_where_trace', false] > - ['sql_vdbe_trace', false] > - ['sql_vdbe_listing', false] > - ['sql_vdbe_eqp', false] > - ['sql_vdbe_debug', false] > - ['sql_vdbe_addoptrace', false] > - ['sql_trace', false] > - ['sql_select_trace', false] > - ['sql_reverse_unordered_selects', false] > - ['sql_recursive_triggers', true] > - ['sql_parser_trace', false] > - ['sql_full_column_names', false] > - ['sql_defer_foreign_keys', false] > - ['sql_default_engine', 'memtx'] > - ['sql_compound_select_limit', 30] > ... > > 'sql_where_trace' definitely is not <= 'sql_d'. > > This is why I was thinking that probably our _vsession_settings > iterator API should be very limited. After all, it is a fixed > size space with settings. A user don't need to be able to dance > with it using different iterators. I would allow only EQ, and either > empty key, or a subsystem name like 'sql', or a full key for a > concrete setting. > > Up to you. And probably to Kirill since it is a public API. > I fixed behaviour of iterators. > > + } else { > > + sql_option_id = iterator_type_is_reverse(type) ? > > + sql_option_id_max() - 1 : 0; > > + } > > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > > index 51cd7ce..cb97106 100644 > > --- a/src/box/sql/build.c > > +++ b/src/box/sql/build.c > > @@ -3242,3 +3243,160 @@ sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name, > > +/** > > + * Identifiers of all SQL options that can be viewed. The > > + * identifier of the option is equal to its place in the sorted > > + * list of options, which starts at 0. > > + * > > + * It is IMPORTANT that SQL options are sorted by name. If this is > > + * not the case, the result returned by the _vsession_settings > > + * space iterator will not be sorted properly. > > + */ > > 2. Please, add a test that selects all the options and checks > that they are sorted. It would prevent accidental reorders. > Not sure that we can do this since some of the options appears only in debug build. But I reworked some tests using additional space to check sorting and iterator. > > +enum { > > + SQL_OPTION_COMPOUND_SELECT_LIMIT = 0, > > + SQL_OPTION_DEFAULT_ENGINE, > > + SQL_OPTION_DEFER_FOREIGN_KEYS, > > + SQL_OPTION_FULL_COLUMN_NAMES, > > +#ifndef NDEBUG > > + SQL_OPTION_PARSER_TRACE, > > +#endif > > + SQL_OPTION_RECURSIVE_TRIGGERS, > > + SQL_OPTION_REVERSE_UNORDERED_SELECTS, > > +#ifndef NDEBUG > > + SQL_OPTION_SELECT_TRACE, > > + SQL_OPTION_TRACE, > > + SQL_OPTION_VDBE_ADDOPTRACE, > > + SQL_OPTION_VDBE_DEBUG, > > + SQL_OPTION_VDBE_EQP, > > + SQL_OPTION_VDBE_LISTING, > > + SQL_OPTION_VDBE_TRACE, > > + SQL_OPTION_WHERE_TRACE, > > +#endif > > + SQL_OPTION_max, > > +}; > > + > > +/** > > + * A local structure that allows to establish a connection between > > + * the name of the parameter, its field type and mask, if it have > > + * one. > > + */ > > +struct sql_option_metadata > > +{ > > + const char *name; > > + uint32_t field_type; > > + uint32_t mask; > > 3. What is the option mask? > Previously it was "flag", but since one of the options (sql_debug) have three three of flags to set/unset I changed its name to "mask". > > +}; > > + > > +int > > +sql_option_tuple(struct tuple_format *format, int option_id, > > + struct tuple **result) > > +{ > > + if (option_id < 0 || option_id >= SQL_OPTION_max) { > > + *result = NULL; > > + return 0; > > + } > > + int limit = 0; > > + const char *engine = NULL; > > + struct region *region = &fiber()->gc; > > + struct session *session = current_session(); > > + uint32_t flags = session->sql_flags; > > + uint32_t mask = sql_options[option_id].mask; > > + /* Tuple format contains two fields - name and value. */ > > + uint32_t column_count = format->min_field_count; > > + assert(column_count == 2); > > + size_t size = mp_sizeof_array(column_count) + > > + mp_sizeof_str(strlen(sql_options[option_id].name)); > > + if (sql_options[option_id].field_type == FIELD_TYPE_BOOLEAN) { > > + size += mp_sizeof_bool(true); > > + } else if (option_id == SQL_OPTION_DEFAULT_ENGINE) { > > + engine = sql_storage_engine_strs[session->sql_default_engine]; > > + size += mp_sizeof_str(strlen(engine)); > > + } else { > > + assert(option_id == SQL_OPTION_COMPOUND_SELECT_LIMIT); > > + limit = sql_get()->aLimit[SQL_LIMIT_COMPOUND_SELECT]; > > 4. So the SQL_LIMIT_COMPOUND_SELECT is not session local? Then > why is it in the session settings? > True, I forgot about this when changed sysview. Still, I do not know where should we move this option. > > + size += mp_sizeof_uint(limit); > > + } > > + > > + size_t svp = region_used(region); > > + char *pos_ret = region_alloc(region, size); > > 5. Up to you, but probably static_alloc() would work well here. > Size of option tuples is limited, and static_alloc() returns up > to 12KB that should be more than enough here. We could assert, > that static_alloc() returns not NULL for the given size. > Thanks, fixed. > > + if (pos_ret == NULL) { > > + diag_set(OutOfMemory, size, "region_alloc", "pos_ret"); > > + return -1; > > + } > > + char *pos = mp_encode_array(pos_ret, column_count); > > + pos = mp_encode_str(pos, sql_options[option_id].name, > > + strlen(sql_options[option_id].name)); > > + if (sql_options[option_id].field_type == FIELD_TYPE_BOOLEAN) > > + pos = mp_encode_bool(pos, (flags & mask) == mask); > > + else if (option_id == SQL_OPTION_DEFAULT_ENGINE) > > + pos = mp_encode_str(pos, engine, strlen(engine)); > > + else > > + pos = mp_encode_uint(pos, limit); > > + struct tuple *tuple = tuple_new(format, pos_ret, pos_ret + size); > > + region_truncate(region, svp); > > + if (tuple == NULL) > > + return -1; > > + *result = tuple; > > + return 0; > > +} > > diff --git a/test/box/misc.result b/test/box/misc.result > > index b293051..b41ffa4 100644 > > --- a/test/box/misc.result > > +++ b/test/box/misc.result > > @@ -1330,3 +1330,124 @@ test_run:grep_log('default', 'test log') > > --- > > - test log > > ... > > +-- > > +-- gh-4511: make sure that _vsession_settings sysview works as > > +-- intended. > > +-- > > +v = box.space._vsession_settings > > +--- > > +... > > +option_count = v:count() > > +--- > > +... > > +v:format() > > +--- > > +- [{'name': 'name', 'type': 'string'}, {'name': 'value', 'type': 'any'}] > > +... > > +(#v:select()) == option_count > > +--- > > +- true > > +... > > +(#v:select({}, {iterator = 'ALL'})) == option_count > > +--- > > +- true > > +... > > +(#v:select({}, {iterator = 'REQ'})) == option_count > > +--- > > +- true > > +... > > +(#v:select({}, {iterator = 'EQ'})) == option_count > > +--- > > +- true > > +... > > +(#v:select({}, {iterator = 'GE'})) == option_count > > +--- > > +- true > > +... > > +(#v:select({}, {iterator = 'GT'})) == option_count > > +--- > > +- true > > +... > > +(#v:select({}, {iterator = 'LE'})) == option_count > > +--- > > +- true > > +... > > +(#v:select({}, {iterator = 'LT'})) == option_count > > +--- > > +- true > > +... > > +(#v:select({'abcde'}, {iterator = 'ALL'})) == 0 > > +--- > > +- true > > +... > > +(#v:select({'abcde'}, {iterator = 'REQ'})) == 0 > > +--- > > +- true > > +... > > +(#v:select({'abcde'}, {iterator = 'EQ'})) == 0 > > +--- > > +- true > > +... > > +(#v:select({'abcde'}, {iterator = 'GE'})) == 0 > > +--- > > +- true > > +... > > +(#v:select({'abcde'}, {iterator = 'GT'})) == 0 > > +--- > > +- true > > +... > > +(#v:select({'abcde'}, {iterator = 'LE'})) == option_count > > +--- > > +- true > > +... > > 6. Are you sure? We have no options, whose name is <= 'a'. > At least because all of them start with 'sql_' which > 'a'. > > If you want to keep these iterator types, then probably > the most correct test would fill a normal space with > session settings, and check for each of these keys above, > that the virtual space and the real one return the same. > Thank you for suggestion! I reworked tests using additional space. New patch: >From 61f75f2b4a800bab37ba5195b71c488fe0d145ee Mon Sep 17 00:00:00 2001 From: Mergen Imeev Date: Thu, 24 Oct 2019 15:18:09 +0300 Subject: [PATCH] box: introdice _vsession_settings sysview This patch creates the _vsession_settings sysview. This sysview contains names and values of the session-local options. This sysview creates tuples on the fly using its own get() and create_iterator() methods. This allows us to get a tuple without having to save it anywhere. Currently, only SQL options can be extracted from this sysview, since the only session-local options are SQL options. Part of #4511 diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap index 59f6cc1..6b4b96a 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..9b5df28 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_VSESSION_SETTINGS_ID); + lua_setfield(L, -2, "VSESSION_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 e71b7fb..94e4118 100644 --- a/src/box/lua/upgrade.lua +++ b/src/box/lua/upgrade.lua @@ -943,6 +943,28 @@ local function upgrade_to_2_3_0() end -------------------------------------------------------------------------------- +-- Tarantool 2.3.1 +-------------------------------------------------------------------------------- + +local function create_vsession_settings_sysview() + 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 _vsession_settings") + _space:insert{box.schema.VSESSION_SETTINGS_ID, ADMIN, '_vsession_settings', + 'sysview', 0, setmap({}), format} + log.info("create index _vsession_settings:primary") + _index:insert{box.schema.VSESSION_SETTINGS_ID, 0, 'primary', 'tree', + {unique = true}, {{0, 'string'}}} +end + +local function upgrade_to_2_3_1() + create_vsession_settings_sysview() +end + +-------------------------------------------------------------------------------- local function get_version() local version = box.space._schema:get{'version'} @@ -977,6 +999,7 @@ local function upgrade(options) {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true}, {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true}, {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true}, + {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true}, } for _, handler in ipairs(handlers) do diff --git a/src/box/schema_def.h b/src/box/schema_def.h index ba870ff..b9824cb 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 _vsession_settings. */ + BOX_VSESSION_SETTINGS_ID = 381, /** End of the reserved range of system spaces. */ BOX_SYSTEM_ID_MAX = 511, BOX_ID_NIL = 2147483647 diff --git a/src/box/session.cc b/src/box/session.cc index d69b657..6d301bf 100644 --- a/src/box/session.cc +++ b/src/box/session.cc @@ -36,6 +36,9 @@ #include "user.h" #include "error.h" #include "tt_static.h" +#include "index.h" +#include "schema.h" +#include "sql.h" const char *session_type_strs[] = { "background", @@ -361,3 +364,135 @@ generic_session_sync(struct session *session) (void) session; return 0; } + +int +session_options_get(struct index *index, const char *key, uint32_t part_count, + struct tuple **result) +{ + assert(part_count == 1); + (void)part_count; + struct space *space = space_cache_find(index->def->space_id); + uint32_t len; + const char *name = mp_decode_str(&key, &len); + name = tt_cstr(name, len); + /* + * Currently, the only session local options are SQL + * options. + */ + uint32_t id_max = sql_option_id_max(); + for (uint32_t id = 0; id < id_max; ++id) { + if (sql_option_compare(name, id) == 0) + return sql_option_tuple(space->format, id, result); + } + *result = NULL; + return 0; +} + +/** + * An iterator that iterates over current session options. + */ +struct session_options_iterator { + /** Base iterator. Must be the first member. */ + struct iterator base; + /** Format of the tuples this iterator returns. */ + struct tuple_format *format; + /** Counter, count number of options already shown. */ + uint32_t counter; + /** Decoded key. */ + char *key; + /** Type of iterator. */ + enum iterator_type iterator_type; +}; + +static int +session_options_iterator_next(struct iterator *itr, struct tuple **ret) +{ + struct session_options_iterator *it = + (struct session_options_iterator *)itr; + uint32_t id_max = sql_option_id_max(); + if (it->key == NULL) + return sql_option_tuple(it->format, it->counter++, ret); + for (; it->counter < id_max; ++it->counter) { + int compare = sql_option_compare(it->key, it->counter); + if (compare == 0 && (it->iterator_type == ITER_EQ || + it->iterator_type == ITER_GE || + it->iterator_type == ITER_ALL)) + break; + if (compare > 0 && (it->iterator_type == ITER_GT || + it->iterator_type == ITER_GE || + it->iterator_type == ITER_ALL)) + break; + } + if (it->counter < id_max) + return sql_option_tuple(it->format, it->counter++, ret); + *ret = NULL; + return 0; +} + +static int +session_options_iterator_prev(struct iterator *itr, struct tuple **ret) +{ + struct session_options_iterator *it = + (struct session_options_iterator *)itr; + uint32_t id_max = sql_option_id_max(); + if (it->key == NULL) + return sql_option_tuple(it->format, id_max - 1 - it->counter++, + ret); + for (; it->counter < id_max; ++it->counter) { + uint32_t id = id_max - it->counter - 1; + int compare = sql_option_compare(it->key, id); + if (compare == 0 && (it->iterator_type == ITER_REQ || + it->iterator_type == ITER_LE)) + break; + if (compare < 0 && (it->iterator_type == ITER_LT || + it->iterator_type == ITER_LE)) + break; + } + if (it->counter < id_max) + return sql_option_tuple(it->format, id_max - 1 - it->counter++, + ret); + *ret = NULL; + return 0; +} + +static void +session_options_iterator_free(struct iterator *itr) +{ + struct session_options_iterator *it = + (struct session_options_iterator *)itr; + free(it->key); + free(it); +} + +struct iterator * +session_options_create_iterator(struct index *index, enum iterator_type type, + const char *key, uint32_t part_count) +{ + 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); + name = tt_cstr(name, len); + decoded_key = (char *)malloc(len + 1); + memcpy(decoded_key, name, len + 1); + } + struct space *space = space_cache_find(index->def->space_id); + struct session_options_iterator *it = + (session_options_iterator *)malloc(sizeof(*it)); + if (it == NULL) { + diag_set(OutOfMemory, sizeof(*it), "malloc", "it"); + return NULL; + } + iterator_create(&it->base, index); + it->base.next = iterator_type_is_reverse(type) ? + session_options_iterator_prev : + session_options_iterator_next; + it->base.free = session_options_iterator_free; + it->key = decoded_key; + it->iterator_type = type; + it->format = space->format; + it->counter = 0; + return (struct iterator *)it; +} diff --git a/src/box/session.h b/src/box/session.h index 85a2d94..96d9ad0 100644 --- a/src/box/session.h +++ b/src/box/session.h @@ -36,12 +36,15 @@ #include "fiber.h" #include "user.h" #include "authentication.h" +#include "iterator_type.h" #if defined(__cplusplus) extern "C" { #endif /* defined(__cplusplus) */ struct port; +struct index; +struct tuple; struct session_vtab; void @@ -355,6 +358,16 @@ generic_session_fd(struct session *session); int64_t generic_session_sync(struct session *session); +/** Method get() for _vsession_settings sysview. */ +int +session_options_get(struct index *index, const char *key, uint32_t part_count, + struct tuple **result); + +/** Method create_iterator() for _vsession_settings sysview. */ +struct iterator * +session_options_create_iterator(struct index *index, enum iterator_type type, + const char *key, uint32_t part_count); + #if defined(__cplusplus) } /* extern "C" */ diff --git a/src/box/sql.h b/src/box/sql.h index 0fa52fc..ad85f1e 100644 --- a/src/box/sql.h +++ b/src/box/sql.h @@ -70,6 +70,7 @@ struct Select; struct Table; struct sql_trigger; struct space_def; +struct tuple_format; /** * Perform parsing of provided expression. This is done by @@ -420,6 +421,40 @@ void vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref, struct tuple *tuple); +/** + * Return the next number after the ID of the last SQL option. + * + * @retval Next number after the last SQL option. + */ +uint32_t +sql_option_id_max(); + +/** + * Return an integer less than, equal to, or greater than zero if + * name of option with given ID is found, respectively, to be less + * than, to match, or be greater than key. + * + * @param key Key to compare with option name. + * @param id ID of the option. + * @retval result of comparison. + */ +int +sql_option_compare(const char *key, uint32_t id); + +/** + * Create a tuple that contains the name and value of the SQL + * option. + * + * @param format Format of the tuple. + * @param option_id ID of SQL option. + * @param[out] result New tuple or NULL. + * @retval 0 On success. + * @retval < 0 On error. + */ +int +sql_option_tuple(struct tuple_format *format, int option_id, + struct tuple **result); + #if defined(__cplusplus) } /* extern "C" { */ #endif diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 51cd7ce..dceb092 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -48,6 +48,7 @@ #include "vdbeInt.h" #include "tarantoolInt.h" #include "box/box.h" +#include "box/tuple.h" #include "box/ck_constraint.h" #include "box/fk_constraint.h" #include "box/sequence.h" @@ -3242,3 +3243,151 @@ sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name, *fieldno = i; return 0; } + +/** + * Identifiers of all SQL options that can be viewed. The + * identifier of the option is equal to its place in the sorted + * list of options, which starts at 0. + * + * It is IMPORTANT that SQL options are sorted by name. If this is + * not the case, the result returned by the _vsession_settings + * space iterator will not be sorted properly. + */ +enum { + SQL_OPTION_COMPOUND_SELECT_LIMIT = 0, + SQL_OPTION_DEFAULT_ENGINE, + SQL_OPTION_DEFER_FOREIGN_KEYS, + SQL_OPTION_FULL_COLUMN_NAMES, +#ifndef NDEBUG + SQL_OPTION_PARSER_TRACE, +#endif + SQL_OPTION_RECURSIVE_TRIGGERS, + SQL_OPTION_REVERSE_UNORDERED_SELECTS, +#ifndef NDEBUG + SQL_OPTION_SELECT_TRACE, + SQL_OPTION_TRACE, + SQL_OPTION_VDBE_ADDOPTRACE, + SQL_OPTION_VDBE_DEBUG, + SQL_OPTION_VDBE_EQP, + SQL_OPTION_VDBE_LISTING, + SQL_OPTION_VDBE_TRACE, + SQL_OPTION_WHERE_TRACE, +#endif + SQL_OPTION_max, +}; + +/** + * A local structure that allows to establish a connection between + * the name of the parameter, its field type and mask, if it have + * one. + */ +struct sql_option_metadata +{ + const char *name; + uint32_t field_type; + uint32_t mask; +}; + +/** + * Variable that contains names of the SQL options, their field + * types and mask if they have one or 0 if don't have. + * + * It is IMPORTANT that these options sorted by name. + */ +static struct sql_option_metadata sql_options[] = { + /** SQL_OPTION_COMPOUND_SELECT_LIMIT */ + {"sql_compound_select_limit", FIELD_TYPE_INTEGER, 0}, + /** SQL_OPTION_DEFAULT_ENGINE */ + {"sql_default_engine", FIELD_TYPE_STRING, 0}, + /** SQL_OPTION_DEFER_FOREIGN_KEYS */ + {"sql_defer_foreign_keys", FIELD_TYPE_BOOLEAN, SQL_DeferFKs}, + /** SQL_OPTION_FULL_COLUMN_NAMES */ + {"sql_full_column_names", FIELD_TYPE_BOOLEAN, SQL_FullColNames}, +#ifndef NDEBUG + /** SQL_OPTION_PARSER_TRACE */ + {"sql_parser_trace", FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG}, +#endif + /** SQL_OPTION_RECURSIVE_TRIGGERS */ + {"sql_recursive_triggers", FIELD_TYPE_BOOLEAN, SQL_RecTriggers}, + /** SQL_OPTION_REVERSE_UNORDERED_SELECTS */ + {"sql_reverse_unordered_selects", FIELD_TYPE_BOOLEAN, SQL_ReverseOrder}, +#ifndef NDEBUG + /** SQL_OPTION_SELECT_TRACE */ + {"sql_select_trace", FIELD_TYPE_BOOLEAN, SQL_SelectTrace}, + /** SQL_OPTION_TRACE */ + {"sql_trace", FIELD_TYPE_BOOLEAN, SQL_SqlTrace}, + /** SQL_OPTION_VDBE_ADDOPTRACE */ + {"sql_vdbe_addoptrace", FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace}, + /** SQL_OPTION_VDBE_DEBUG */ + {"sql_vdbe_debug", FIELD_TYPE_BOOLEAN, + SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace}, + /** SQL_OPTION_VDBE_EQP */ + {"sql_vdbe_eqp", FIELD_TYPE_BOOLEAN, SQL_VdbeEQP}, + /** SQL_OPTION_VDBE_LISTING */ + {"sql_vdbe_listing", FIELD_TYPE_BOOLEAN, SQL_VdbeListing}, + /** SQL_OPTION_VDBE_TRACE */ + {"sql_vdbe_trace", FIELD_TYPE_BOOLEAN, SQL_VdbeTrace}, + /** SQL_OPTION_WHERE_TRACE */ + {"sql_where_trace", FIELD_TYPE_BOOLEAN, SQL_WhereTrace}, +#endif +}; + +uint32_t +sql_option_id_max() +{ + return SQL_OPTION_max; +} + +int +sql_option_compare(const char *key, uint32_t id) +{ + assert(id < SQL_OPTION_max); + return strcmp(sql_options[id].name, key); +} + +int +sql_option_tuple(struct tuple_format *format, int option_id, + struct tuple **result) +{ + if (option_id < 0 || option_id >= SQL_OPTION_max) { + *result = NULL; + return 0; + } + int limit = 0; + const char *engine = NULL; + struct session *session = current_session(); + uint32_t flags = session->sql_flags; + uint32_t mask = sql_options[option_id].mask; + /* Tuple format contains two fields - name and value. */ + uint32_t column_count = format->min_field_count; + assert(column_count == 2); + size_t size = mp_sizeof_array(column_count) + + mp_sizeof_str(strlen(sql_options[option_id].name)); + if (sql_options[option_id].field_type == FIELD_TYPE_BOOLEAN) { + size += mp_sizeof_bool(true); + } else if (option_id == SQL_OPTION_DEFAULT_ENGINE) { + engine = sql_storage_engine_strs[session->sql_default_engine]; + size += mp_sizeof_str(strlen(engine)); + } else { + assert(option_id == SQL_OPTION_COMPOUND_SELECT_LIMIT); + limit = sql_get()->aLimit[SQL_LIMIT_COMPOUND_SELECT]; + size += mp_sizeof_uint(limit); + } + + char *pos_ret = static_alloc(size); + assert(pos_ret != NULL); + char *pos = mp_encode_array(pos_ret, column_count); + pos = mp_encode_str(pos, sql_options[option_id].name, + strlen(sql_options[option_id].name)); + if (sql_options[option_id].field_type == FIELD_TYPE_BOOLEAN) + pos = mp_encode_bool(pos, (flags & mask) == mask); + else if (option_id == SQL_OPTION_DEFAULT_ENGINE) + pos = mp_encode_str(pos, engine, strlen(engine)); + else + pos = mp_encode_uint(pos, limit); + struct tuple *tuple = tuple_new(format, pos_ret, pos_ret + size); + if (tuple == NULL) + return -1; + *result = tuple; + return 0; +} diff --git a/src/box/sysview.c b/src/box/sysview.c index 745cf09..83a8ccc 100644 --- a/src/box/sysview.c +++ b/src/box/sysview.c @@ -411,7 +411,7 @@ vsequence_filter(struct space *source, struct tuple *tuple) } static bool -vcollation_filter(struct space *source, struct tuple *tuple) +generic_filter(struct space *source, struct tuple *tuple) { (void) source; (void) tuple; @@ -481,10 +481,17 @@ sysview_space_create_index(struct space *space, struct index_def *def) case BOX_VCOLLATION_ID: source_space_id = BOX_COLLATION_ID; source_index_id = def->iid; - filter = vcollation_filter; + filter = generic_filter; get = index_get; create_iterator = index_create_iterator; break; + case BOX_VSESSION_SETTINGS_ID: + source_space_id = BOX_VSESSION_SETTINGS_ID; + source_index_id = def->iid; + filter = generic_filter; + get = session_options_get; + create_iterator = session_options_create_iterator; + break; default: diag_set(ClientError, ER_MODIFY_INDEX, def->name, space_name(space), @@ -569,9 +576,10 @@ sysview_engine_create_space(struct engine *engine, struct space_def *def, return NULL; } struct tuple_format *format = - tuple_format_new(NULL, NULL, keys, key_count, def->fields, - def->field_count, def->exact_field_count, - def->dict, def->opts.is_temporary, + 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); diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua index f388208..a41fad3 100755 --- a/test/app-tap/tarantoolctl.test.lua +++ b/test/app-tap/tarantoolctl.test.lua @@ -410,8 +410,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 123aa2f..54b43ec 100644 --- a/test/box-py/bootstrap.result +++ b/test/box-py/bootstrap.result @@ -4,7 +4,7 @@ box.internal.bootstrap() box.space._schema:select{} --- - - ['max_id', 511] - - ['version', 2, 3, 0] + - ['version', 2, 3, 1] ... box.space._cluster:select{} --- @@ -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'}]] + - [381, 1, '_vsession_settings', 'sysview', 0, {}, [{'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']]] + - [381, 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 3072b73..5a41526 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 46ce868..2a0eaca 100644 --- a/test/box/alter.result +++ b/test/box/alter.result @@ -92,7 +92,7 @@ space = box.space[t[1]] ... space.id --- -- 373 +- 382 ... space.field_count --- @@ -137,7 +137,7 @@ space_deleted ... space:replace{0} --- -- error: Space '373' does not exist +- error: Space '382' 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']]] + - [381, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]] ... -- modify indexes of a system space _index:delete{_index.id, 0} diff --git a/test/box/misc.result b/test/box/misc.result index b293051..1edd59e 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -1330,3 +1330,174 @@ test_run:grep_log('default', 'test log') --- - test log ... +-- +-- gh-4511: make sure that _vsession_settings sysview works as +-- intended. +-- +v = box.space._vsession_settings +--- +... +option_count = v:count() +--- +... +v:format() +--- +- [{'name': 'name', 'type': 'string'}, {'name': 'value', 'type': 'any'}] +... +(#v:select()) == option_count +--- +- true +... +(#v:select({}, {iterator = 'ALL'})) == option_count +--- +- true +... +(#v:select({}, {iterator = 'REQ'})) == option_count +--- +- true +... +(#v:select({}, {iterator = 'EQ'})) == option_count +--- +- true +... +(#v:select({}, {iterator = 'GE'})) == option_count +--- +- true +... +(#v:select({}, {iterator = 'GT'})) == option_count +--- +- true +... +(#v:select({}, {iterator = 'LE'})) == option_count +--- +- true +... +(#v:select({}, {iterator = 'LT'})) == option_count +--- +- true +... +s = box.schema.space.create('settings', {format = v:format()}) +--- +... +_ = s:create_index('primary') +--- +... +for _,value in v:pairs() do s:insert(value) end +--- +... +(#v:select({'abcde'}, {iterator = 'ALL'})) == (#s:select({'abcde'}, {iterator = 'ALL'})) +--- +- true +... +(#v:select({'abcde'}, {iterator = 'REQ'})) == (#s:select({'abcde'}, {iterator = 'REQ'})) +--- +- true +... +(#v:select({'abcde'}, {iterator = 'EQ'})) == (#s:select({'abcde'}, {iterator = 'EQ'})) +--- +- true +... +(#v:select({'abcde'}, {iterator = 'GE'})) == (#s:select({'abcde'}, {iterator = 'GE'})) +--- +- true +... +(#v:select({'abcde'}, {iterator = 'GT'})) == (#s:select({'abcde'}, {iterator = 'GT'})) +--- +- true +... +(#v:select({'abcde'}, {iterator = 'LE'})) == (#s:select({'abcde'}, {iterator = 'LE'})) +--- +- true +... +(#v:select({'abcde'}, {iterator = 'LT'})) == (#s:select({'abcde'}, {iterator = 'LT'})) +--- +- true +... +v:select({'sql_defer_foreign_keys'}) +--- +- - ['sql_defer_foreign_keys', false] +... +v:select({'sql_recursive_triggers'}) +--- +- - ['sql_recursive_triggers', true] +... +v:select({'sql_reverse_unordered_selects'}) +--- +- - ['sql_reverse_unordered_selects', false] +... +v:select({'sql_compound_select_limit'}) +--- +- - ['sql_compound_select_limit', 30] +... +v:select({'sql_default_engine'}) +--- +- - ['sql_default_engine', 'memtx'] +... +-- Check get() method. +v:get({'sql_defer_foreign_keys'}) +--- +- ['sql_defer_foreign_keys', false] +... +v:get({'sql_recursive_triggers'}) +--- +- ['sql_recursive_triggers', true] +... +v:get({'sql_reverse_unordered_selects'}) +--- +- ['sql_reverse_unordered_selects', false] +... +v:get({'sql_compound_select_limit'}) +--- +- ['sql_compound_select_limit', 30] +... +v:get({'sql_default_engine'}) +--- +- ['sql_default_engine', 'memtx'] +... +-- Make sure that space is read-only. +new_record = v:frommap({name='abs', value=123}) +--- +... +v:insert(new_record) +--- +- error: View '_vsession_settings' is read-only +... +-- Check pairs() method. +t = {} +--- +... +for key, value in box.space._vsession_settings:pairs() do table.insert(t, {key, value}) end +--- +... +#t == option_count +--- +- true +... +-- Check sorting. +in_sysview = v:select() +--- +... +in_space = s:select() +--- +... +is_right = true +--- +... +for key, value in pairs(in_sysview) do is_right = is_right and (in_space[key].name == value.name) end +--- +... +is_right +--- +- true +... +-- Make sure that SQL SELECT works. +box.execute([[SELECT * from "_vsession_settings" WHERE "name" = 'sql_default_engine']]) +--- +- metadata: + - name: name + type: string + - name: value + type: any + rows: + - ['sql_default_engine', 'memtx'] +... diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua index cc223b2..037c866 100644 --- a/test/box/misc.test.lua +++ b/test/box/misc.test.lua @@ -383,3 +383,66 @@ tuple_format = box.internal.new_tuple_format(format) box.cfg{} local ffi = require'ffi' ffi.C._say(ffi.C.S_WARN, nil, 0, nil, "%s", "test log") test_run:grep_log('default', 'test log') + +-- +-- gh-4511: make sure that _vsession_settings sysview works as +-- intended. +-- + +v = box.space._vsession_settings +option_count = v:count() + +v:format() + +(#v:select()) == option_count +(#v:select({}, {iterator = 'ALL'})) == option_count +(#v:select({}, {iterator = 'REQ'})) == option_count +(#v:select({}, {iterator = 'EQ'})) == option_count +(#v:select({}, {iterator = 'GE'})) == option_count +(#v:select({}, {iterator = 'GT'})) == option_count +(#v:select({}, {iterator = 'LE'})) == option_count +(#v:select({}, {iterator = 'LT'})) == option_count + +s = box.schema.space.create('settings', {format = v:format()}) +_ = s:create_index('primary') +for _,value in v:pairs() do s:insert(value) end + +(#v:select({'abcde'}, {iterator = 'ALL'})) == (#s:select({'abcde'}, {iterator = 'ALL'})) +(#v:select({'abcde'}, {iterator = 'REQ'})) == (#s:select({'abcde'}, {iterator = 'REQ'})) +(#v:select({'abcde'}, {iterator = 'EQ'})) == (#s:select({'abcde'}, {iterator = 'EQ'})) +(#v:select({'abcde'}, {iterator = 'GE'})) == (#s:select({'abcde'}, {iterator = 'GE'})) +(#v:select({'abcde'}, {iterator = 'GT'})) == (#s:select({'abcde'}, {iterator = 'GT'})) +(#v:select({'abcde'}, {iterator = 'LE'})) == (#s:select({'abcde'}, {iterator = 'LE'})) +(#v:select({'abcde'}, {iterator = 'LT'})) == (#s:select({'abcde'}, {iterator = 'LT'})) + +v:select({'sql_defer_foreign_keys'}) +v:select({'sql_recursive_triggers'}) +v:select({'sql_reverse_unordered_selects'}) +v:select({'sql_compound_select_limit'}) +v:select({'sql_default_engine'}) + +-- Check get() method. +v:get({'sql_defer_foreign_keys'}) +v:get({'sql_recursive_triggers'}) +v:get({'sql_reverse_unordered_selects'}) +v:get({'sql_compound_select_limit'}) +v:get({'sql_default_engine'}) + +-- Make sure that space is read-only. +new_record = v:frommap({name='abs', value=123}) +v:insert(new_record) + +-- Check pairs() method. +t = {} +for key, value in box.space._vsession_settings:pairs() do table.insert(t, {key, value}) end +#t == option_count + +-- Check sorting. +in_sysview = v:select() +in_space = s:select() +is_right = true +for key, value in pairs(in_sysview) do is_right = is_right and (in_space[key].name == value.name) end +is_right + +-- Make sure that SQL SELECT works. +box.execute([[SELECT * from "_vsession_settings" WHERE "name" = 'sql_default_engine']]) 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