[Tarantool-patches] [PATCH 3/3] box: add SQL settings to _session_settings

Mergen Imeev imeevma at tarantool.org
Thu Dec 26 21:07:18 MSK 2019


Thank you for review and fixes! I haven't found any issues
with them. New patch below.

Also, I added new session setting: sql_full_metadata.


On Sat, Dec 21, 2019 at 06:59:10PM +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. In my commit you can find inlined explanations about some
> changes.
> 
> ================================================================================
> 
> commit 0cb8d1f6a337a1eb912ada031a6393bc22616e2e
> Author: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
> Date:   Sat Dec 21 18:08:06 2019 +0100
> 
>     Review fixes 3/3
> 
> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index c660b1c70..11894fccc 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -258,6 +258,7 @@ struct errcode_record {
>  	/*203 */_(ER_BOOTSTRAP_READONLY,	"Trying to bootstrap a local read-only instance as master") \
>  	/*204 */_(ER_SQL_FUNC_WRONG_RET_COUNT,	"SQL expects exactly one argument returned from %s, got %d")\
>  	/*205 */_(ER_FUNC_INVALID_RETURN_TYPE,	"Function '%s' returned value of invalid type: expected %s got %s") \
> +	/*206 */_(ER_SESSION_SETTING_INVALID_VALUE,	"Session setting %s expected a value of type %s") \
> ================================================================================
> 
> I've decided that ER_FIELD_TYPE, used before, does not
> really fit here. Because it produces a not user-friendly
> message, and because formally speaking in the space format
> we said that type of the field is 'any'. So it should have
> accepted everything. Via that new error I've tried to
> explain to a user the problem a bit deeper.
> 
> ================================================================================
>  
>  /*
>   * !IMPORTANT! Please follow instructions at start of the file
> diff --git a/src/box/session_settings.h b/src/box/session_settings.h
> index 8166f17f8..25490a7e3 100644
> --- a/src/box/session_settings.h
> +++ b/src/box/session_settings.h
> @@ -30,9 +30,19 @@
>   * SUCH DAMAGE.
>   */
>  
> -enum session_setting_module {
> -	SESSION_SETTING_MODULE_SQL,
> -	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_SQL,
> +	session_setting_type_MAX,
>  };
>  
>  struct session_setting_module {
> diff --git a/src/box/sql.c b/src/box/sql.c
> index f1df55571..cc826177b 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -64,9 +64,14 @@ static const uint32_t default_sql_flags = SQL_ShortColNames
>  					  | SQL_AutoIndex
>  					  | SQL_RecTriggers;
>  
> +extern void
> +sql_session_settings_init();
> +
>  void
>  sql_init()
>  {
> +	sql_session_settings_init();
> +
>  	default_flags |= default_sql_flags;
>  
>  	current_session()->sql_flags |= default_sql_flags;
> diff --git a/src/box/sql.h b/src/box/sql.h
> index 3f86568a6..0fa52fc0b 100644
> --- a/src/box/sql.h
> +++ b/src/box/sql.h
> @@ -33,7 +33,6 @@
>  
>  #include <stdbool.h>
>  #include <stdint.h>
> -#include "iterator_type.h"
>  
>  #if defined(__cplusplus)
>  extern "C" {
> @@ -71,7 +70,6 @@ struct Select;
>  struct Table;
>  struct sql_trigger;
>  struct space_def;
> -struct tuple_format;
>  
>  /**
>   * Perform parsing of provided expression. This is done by
> @@ -422,51 +420,6 @@ void
>  vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref,
>  			     struct tuple *tuple);
>  
> -
> -/**
> - * Return number of SQL session settings.
> - *
> - * @retval Number of SQL session settings.
> - */
> -int
> -sql_session_settings_count();
> -
> -/**
> - * Matches given key with SQL settings names and returns tuple
> - * that contains name and value of the setting.
> - *
> - * @param format The format of result tuple.
> - * @param id The ID to start search setting from.
> - * @param key The key to find the setting.
> - * @param type Type of iterator to match key with settings name.
> - * @param end_id The ID of returned setting.
> - * @param result Result tuple.
> - *
> - * @retval 0 on success.
> - * @retval -1 on error.
> - */
> -int
> -sql_session_setting_get(struct tuple_format *format, int id, const char *key,
> -			enum iterator_type type, int *end_id,
> -			struct tuple **result);
> -
> -/**
> - * Set new value to SQL session setting. Value given in MsgPack
> - * format. Returns tuple that contains name and new value of the
> - * setting.
> - *
> - * @param format Format of result tuple.
> - * @param id ID of SQL setting to set new value.
> - * @param value MsgPack contains value to set.
> - * @param result Result tuple.
> - *
> - * @retval 0 on success.
> - * @retval -1 on error.
> - */
> -int
> -sql_session_setting_set(struct tuple_format *format, int id, const char *value,
> -			struct tuple **result);
> -
>  #if defined(__cplusplus)
>  } /* extern "C" { */
>  #endif
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 995a32e8d..a6e3752fc 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -46,7 +46,6 @@
>  #include <ctype.h>
>  #include "sqlInt.h"
>  #include "vdbeInt.h"
> -#include "box/tuple.h"
>  #include "tarantoolInt.h"
>  #include "box/box.h"
>  #include "box/ck_constraint.h"
> @@ -57,6 +56,7 @@
>  #include "box/schema.h"
>  #include "box/tuple_format.h"
>  #include "box/coll_id_cache.h"
> +#include "box/session_settings.h"
>  
>  void
>  sql_finish_coding(struct Parse *parse_context)
> @@ -3272,177 +3272,119 @@ enum {
>  	SQL_SESSION_SETTING_VDBE_TRACE,
>  	SQL_SESSION_SETTING_WHERE_TRACE,
>  #endif
> -	SQL_SESSION_SETTING_max,
> +	sql_session_setting_MAX,
>  };
>  
> -int
> -sql_session_settings_count()
> -{
> -	return SQL_SESSION_SETTING_max;
> -}
> +static const char *sql_session_setting_strs[sql_session_setting_MAX] = {
> +	"sql_default_engine",
> +	"sql_defer_foreign_keys",
> +	"sql_full_column_names",
> +#ifndef NDEBUG
> +	"sql_parser_trace",
> +#endif
> +	"sql_recursive_triggers",
> +	"sql_reverse_unordered_selects",
> +#ifndef NDEBUG
> +	"sql_select_trace",
> +	"sql_trace",
> +	"sql_vdbe_addoptrace",
> +	"sql_vdbe_debug",
> +	"sql_vdbe_eqp",
> +	"sql_vdbe_listing",
> +	"sql_vdbe_trace",
> +	"sql_where_trace",
> +#endif
> +};
>  
>  /**
>   * A local structure that allows to establish a connection between
> - * the name of the parameter, its field type and mask, if it have
> - * one.
> + * parameter and its field type and mask, if it has one.
>   */
>  struct sql_option_metadata
>  {
> -	const char *name;
>  	uint32_t field_type;
>  	uint32_t mask;
>  };
>  
>  /**
> - * Variable that contains names of the SQL session options, their
> - * field types and mask if they have one or 0 if don't have.
> + * Variable that contains SQL session option field types and masks
> + * 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_session_opts[] = {
>  	/** SQL_SESSION_SETTING_DEFAULT_ENGINE */
> -	{"sql_default_engine", FIELD_TYPE_STRING, 0},
> +	{FIELD_TYPE_STRING, 0},
>  	/** SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS */
> -	{"sql_defer_foreign_keys", FIELD_TYPE_BOOLEAN, SQL_DeferFKs},
> +	{FIELD_TYPE_BOOLEAN, SQL_DeferFKs},
>  	/** SQL_SESSION_SETTING_FULL_COLUMN_NAMES */
> -	{"sql_full_column_names", FIELD_TYPE_BOOLEAN, SQL_FullColNames},
> +	{FIELD_TYPE_BOOLEAN, SQL_FullColNames},
>  #ifndef NDEBUG
>  	/** SQL_SESSION_SETTING_PARSER_TRACE */
> -	{"sql_parser_trace", FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG},
> +	{FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG},
>  #endif
>  	/** SQL_SESSION_SETTING_RECURSIVE_TRIGGERS */
> -	{"sql_recursive_triggers", FIELD_TYPE_BOOLEAN, SQL_RecTriggers},
> +	{FIELD_TYPE_BOOLEAN, SQL_RecTriggers},
>  	/** SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS */
> -	{"sql_reverse_unordered_selects", FIELD_TYPE_BOOLEAN, SQL_ReverseOrder},
> +	{FIELD_TYPE_BOOLEAN, SQL_ReverseOrder},
>  #ifndef NDEBUG
>  	/** SQL_SESSION_SETTING_SELECT_TRACE */
> -	{"sql_select_trace", FIELD_TYPE_BOOLEAN, SQL_SelectTrace},
> +	{FIELD_TYPE_BOOLEAN, SQL_SelectTrace},
>  	/** SQL_SESSION_SETTING_TRACE */
> -	{"sql_trace", FIELD_TYPE_BOOLEAN, SQL_SqlTrace},
> +	{FIELD_TYPE_BOOLEAN, SQL_SqlTrace},
>  	/** SQL_SESSION_SETTING_VDBE_ADDOPTRACE */
> -	{"sql_vdbe_addoptrace", FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace},
> +	{FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace},
>  	/** SQL_SESSION_SETTING_VDBE_DEBUG */
> -	{"sql_vdbe_debug", FIELD_TYPE_BOOLEAN,
> +	{FIELD_TYPE_BOOLEAN,
>  	 SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace},
>  	/** SQL_SESSION_SETTING_VDBE_EQP */
> -	{"sql_vdbe_eqp", FIELD_TYPE_BOOLEAN, SQL_VdbeEQP},
> +	{FIELD_TYPE_BOOLEAN, SQL_VdbeEQP},
>  	/** SQL_SESSION_SETTING_VDBE_LISTING */
> -	{"sql_vdbe_listing", FIELD_TYPE_BOOLEAN, SQL_VdbeListing},
> +	{FIELD_TYPE_BOOLEAN, SQL_VdbeListing},
>  	/** SQL_SESSION_SETTING_VDBE_TRACE */
> -	{"sql_vdbe_trace", FIELD_TYPE_BOOLEAN, SQL_VdbeTrace},
> +	{FIELD_TYPE_BOOLEAN, SQL_VdbeTrace},
>  	/** SQL_SESSION_SETTING_WHERE_TRACE */
> -	{"sql_where_trace", FIELD_TYPE_BOOLEAN, SQL_WhereTrace},
> +	{FIELD_TYPE_BOOLEAN, SQL_WhereTrace},
>  #endif
>  };
>  
> -/**
> - * Return SQL setting ID that matches the given key and iterator
> - * type. Search begins from next_id.
> - *
> - * @param key Key to match settings name to.
> - * @param next_id ID to start search from.
> - * @param type Type of iterator.
> - *
> - * @retval setting ID.
> - */
> -static int
> -sql_session_setting_id_by_name(const char *key, int next_id,
> -			       enum iterator_type type)
> -{
> -	int id = next_id;
> -	if (key == NULL)
> -		return id;
> -	if (iterator_type_is_reverse(type)) {
> -		for (; id >= 0; --id) {
> -			int compare = strcmp(sql_session_opts[id].name, key);
> -			if (compare == 0 && (type == ITER_REQ ||
> -					     type == ITER_LE))
> -				break;
> -			if (compare < 0 && (type == ITER_LT || type == ITER_LE))
> -				break;
> -		}
> -	} else {
> -		for (; id < SQL_SESSION_SETTING_max; ++id) {
> -			int compare = strcmp(sql_session_opts[id].name, key);
> -			if (compare == 0 && (type == ITER_EQ ||
> -					     type == ITER_GE ||
> -					     type == ITER_ALL))
> -				break;
> -			if (compare > 0 && (type == ITER_GT ||
> -					    type == ITER_GE ||
> -					    type == ITER_ALL))
> -				break;
> -		}
> -	}
> -	return id;
> -}
> -
> -/**
> - * Return a tuple with the specified format, which contains the
> - * name and value of the SQL setting with the specified ID.
> - *
> - * @param format Format of tuple to return.
> - * @param id ID of tuple to return.
> - * @param result[out] Returned tuple.
> - *
> - * @retval 0 on success.
> - * @retval -1 on error.
> - */
> -static int
> -sql_session_setting_tuple(struct tuple_format *format, int id,
> -			  struct tuple **result)
> +static void
> +sql_session_setting_get(int id, const char **mp_pair, const char **mp_pair_end)
>  {
> -	if (id < 0 || id >= SQL_SESSION_SETTING_max) {
> -		*result = NULL;
> -		return 0;
> -	}
> +	assert(id >= 0 && id < sql_session_setting_MAX);
>  	struct session *session = current_session();
>  	uint32_t flags = session->sql_flags;
> -	uint32_t mask = sql_session_opts[id].mask;
> -	const char *engine = NULL;
> -	/* 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_session_opts[id].name));
> +	struct sql_option_metadata *opt = &sql_session_opts[id];
> +	uint32_t mask = opt->mask;
> +	const char *name = sql_session_setting_strs[id];
> +	size_t name_len = strlen(name);
> +	size_t engine_len;
> +	const char *engine;
> +	size_t size = mp_sizeof_array(2) + mp_sizeof_str(name_len);
>  	/*
>  	 * Currently, SQL session settings are of a boolean or
>  	 * string type.
>  	 */
> -	bool is_bool = sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN;
> +	bool is_bool = opt->field_type == FIELD_TYPE_BOOLEAN;
>  	if (is_bool) {
>  		size += mp_sizeof_bool(true);
>  	} else {
>  		assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE);
>  		engine = sql_storage_engine_strs[session->sql_default_engine];
> -		size += mp_sizeof_str(strlen(engine));
> +		engine_len = strlen(engine);
> +		size += mp_sizeof_str(engine_len);
>  	}
>  
> -	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_session_opts[id].name,
> -			    strlen(sql_session_opts[id].name));
> +	char *pos = static_alloc(size);
> +	assert(pos != NULL);
> +	char *pos_end = mp_encode_array(pos, 2);
> +	pos_end = mp_encode_str(pos_end, name, name_len);
>  	if (is_bool)
> -		pos = mp_encode_bool(pos, (flags & mask) == mask);
> +		pos_end = mp_encode_bool(pos_end, (flags & mask) == mask);
>  	else
> -		pos = mp_encode_str(pos, engine, strlen(engine));
> -	struct tuple *tuple = tuple_new(format, pos_ret, pos_ret + size);
> -	if (tuple == NULL)
> -		return -1;
> -	*result = tuple;
> -	return 0;
> -}
> -
> -int
> -sql_session_setting_get(struct tuple_format *format, int id, const char *key,
> -			enum iterator_type type, int *end_id,
> -			struct tuple **result)
> -{
> -	int new_id = sql_session_setting_id_by_name(key, id, type);
> -	if (end_id != NULL)
> -		*end_id = new_id;
> -	return sql_session_setting_tuple(format, new_id, result);
> +		pos_end = mp_encode_str(pos_end, engine, engine_len);
> +	*mp_pair = pos;
> +	*mp_pair_end = pos_end;
>  }
>  
>  static int
> @@ -3481,37 +3423,40 @@ sql_set_string_option(int id, const char *value)
>  	return 0;
>  }
>  
> -bool
> -is_value_type_correct(char mp_type, enum field_type field_type)
> -{
> -	if (mp_typeof(mp_type) == MP_BOOL && field_type == FIELD_TYPE_BOOLEAN)
> -		return true;
> -	if (mp_typeof(mp_type) == MP_STR && field_type == FIELD_TYPE_STRING)
> -		return true;
> -	return false;
> +static int
> +sql_session_setting_set(int id, const char *mp_value)
> +{
> +	assert(id >= 0 && id < sql_session_setting_MAX);
> +	enum mp_type mtype = mp_typeof(*mp_value);
> +	enum field_type stype = sql_session_opts[id].field_type;
> +	uint32_t len;
> +	const char *tmp;
> +	switch(stype) {
> +	case FIELD_TYPE_BOOLEAN:
> +		if (mtype != MP_BOOL)
> +			break;
> +		return sql_set_boolean_option(id, mp_decode_bool(&mp_value));
> +	case FIELD_TYPE_STRING:
> +		if (mtype != MP_STR)
> +			break;
> +		tmp = mp_decode_str(&mp_value, &len);
> +		tmp = tt_cstr(tmp, len);
> +		return sql_set_string_option(id, tmp);
> +	default:
> +		unreachable();
> +	}
> +	diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE,
> +		 sql_session_setting_strs[id], field_type_strs[stype]);
> +	return -1;
>  }
>  
> -int
> -sql_session_setting_set(struct tuple_format *format, int id, const char *value,
> -			struct tuple **result)
> -{
> -	if (id < 0 || id >= SQL_SESSION_SETTING_max)
> -		return 0;
> -	if (!is_value_type_correct(*value, sql_session_opts[id].field_type)) {
> -		diag_set(ClientError, ER_FIELD_TYPE, "2",
> -			 field_type_strs[sql_session_opts[id].field_type]);
> -		return -1;
> -	}
> -	if (sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN) {
> -		if (sql_set_boolean_option(id, mp_decode_bool(&value)) != 0)
> -			return -1;
> -	} else {
> -		assert(sql_session_opts[id].field_type == FIELD_TYPE_STRING);
> -		uint32_t len;
> -		const char *tmp = mp_decode_str(&value, &len);
> -		const char *decoded_value = tt_cstr(tmp, len);
> -		if (sql_set_string_option(id, decoded_value) != 0)
> -			return -1;
> -	}
> -	return sql_session_setting_tuple(format, id, result);
> +void
> +sql_session_settings_init()
> +{
> +	struct session_setting_module *module =
> +		&session_setting_modules[SESSION_SETTING_SQL];
> +	module->settings = sql_session_setting_strs;
> +	module->setting_count = sql_session_setting_MAX;
> +	module->get = sql_session_setting_get;
> +	module->set = sql_session_setting_set;
>  }
> diff --git a/src/box/sql/main.c b/src/box/sql/main.c
> index 826cd1c90..0b20f2132 100644
> --- a/src/box/sql/main.c
> +++ b/src/box/sql/main.c
> @@ -39,7 +39,6 @@
>  #include "vdbeInt.h"
>  #include "version.h"
>  #include "box/session.h"
> -#include "box/session_settings.h"
>  
>  /*
>   * If the following global variable points to a string which is the
> @@ -399,13 +398,6 @@ sql_init_db(sql **out_db)
>  	/* Enable the lookaside-malloc subsystem */
>  	setupLookaside(db, 0, LOOKASIDE_SLOT_SIZE, LOOKASIDE_SLOT_NUMBER);
>  
> -	struct session_settings_modules *sql_settings =
> -		&session_settings_modules[SESSION_SETTING_MODULE_SQL];
> -	sql_settings->id = SESSION_SETTING_MODULE_SQL;
> -	sql_settings->size = sql_session_settings_count();
> -	sql_settings->get = sql_session_setting_get;
> -	sql_settings->set = sql_session_setting_set;
> -
>  	*out_db = db;
>  	return 0;
>  }
> diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result
> index 579a5a18f..e86efedb4 100644
> --- a/test/box/gh-4511-access-settings-from-any-frontend.result
> +++ b/test/box/gh-4511-access-settings-from-any-frontend.result
> @@ -65,16 +65,19 @@ test_run:cmd('setopt delimiter ";"')
>   | - true
>   | ...
>  function check_sorting(ss, ts, key)
> -    local is_right = true
>      local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'}
>      for _, it in pairs(iterators_list) do
>          local view_space = ss:select({key}, {iterator = it})
>          local test_space = ts:select({key}, {iterator = it})
>          for key, value in pairs(view_space) do
> -            is_right = is_right and (test_space[key].name == value.name)
> +            if test_space[key].name ~= value.name then
> +                return {
> +                    err = 'bad sorting', type = it,
> +                    exp = test_space[key].name, got = value.name
> +                }
> +            end
>          end
> ================================================================================
> 
> This change simplifies debug. Because a failed test now prints where exactly
> it has failed.
> 
> ================================================================================
>      end
> -    return is_right
>  end;
>   | ---
>   | ...
> @@ -85,23 +88,18 @@ test_run:cmd('setopt delimiter ""');
>  
>  check_sorting(s, t)
>   | ---
> - | - true
>   | ...
>  check_sorting(s, t, 'abcde')
>   | ---
> - | - true
>   | ...
>  check_sorting(s, t, 'sql_d')
>   | ---
> - | - true
>   | ...
>  check_sorting(s, t, 'sql_v')
>   | ---
> - | - true
>   | ...
>  check_sorting(s, t, 'sql_defer_foreign_keys')
>   | ---
> - | - true
>   | ...
>  
>  t:drop()
> @@ -242,12 +240,12 @@ s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}})
>   | ---
>   | - error: Illegal parameters, field id must be a number or a string
>   | ...
> -s:update('sql_defer_foreign_keys', {{'=', 1, true}})
> +s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}})
>   | ---
>   | - error: Attempt to modify a tuple field which is part of index 'primary' in space
>   |     '_session_settings'
>   | ...
> -s:update('sql_defer_foreign_keys', {{'=', 'name', true}})
> +s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}})
>   | ---
>   | - error: Attempt to modify a tuple field which is part of index 'primary' in space
>   |     '_session_settings'
> @@ -263,13 +261,13 @@ s:update('sql_defer_foreign_keys', {{'=', 'some text', true}})
>  
>  s:update('sql_defer_foreign_keys', {{'=', 'value', 1}})
>   | ---
> - | - error: 'Tuple field 2 type does not match one required by operation: expected boolean'
> + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
>   | ...
>  s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}})
>   | ---
> - | - error: 'Tuple field 2 type does not match one required by operation: expected boolean'
> + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
>   | ...
>  s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
>   | ---
> - | - error: 'Tuple field 2 type does not match one required by operation: expected boolean'
> + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
>   | ...
> 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 0699aec31..9642f681c 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
> @@ -30,16 +30,19 @@ for _,value in s:pairs() do t:insert(value) end
>  
>  test_run:cmd('setopt delimiter ";"')
>  function check_sorting(ss, ts, key)
> -    local is_right = true
>      local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'}
>      for _, it in pairs(iterators_list) do
>          local view_space = ss:select({key}, {iterator = it})
>          local test_space = ts:select({key}, {iterator = it})
>          for key, value in pairs(view_space) do
> -            is_right = is_right and (test_space[key].name == value.name)
> +            if test_space[key].name ~= value.name then
> +                return {
> +                    err = 'bad sorting', type = it,
> +                    exp = test_space[key].name, got = value.name
> +                }
> +            end
>          end
>      end
> -    return is_right
>  end;
>  test_run:cmd('setopt delimiter ""');
>  
> @@ -93,8 +96,8 @@ s:update('sql_defer_foreign_keys', {{1, 'value', true}})
>  s:update('sql_defer_foreign_keys', {{{1}, 'value', true}})
>  
>  s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}})
> -s:update('sql_defer_foreign_keys', {{'=', 1, true}})
> -s:update('sql_defer_foreign_keys', {{'=', 'name', true}})
> +s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}})
> +s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}})
>  s:update('sql_defer_foreign_keys', {{'=', 3, true}})
>  s:update('sql_defer_foreign_keys', {{'=', 'some text', true}})
>  
> diff --git a/test/box/misc.result b/test/box/misc.result
> index d2a20307a..004faaaad 100644
> --- a/test/box/misc.result
> +++ b/test/box/misc.result
> @@ -554,6 +554,7 @@ t;
>    203: box.error.BOOTSTRAP_READONLY
>    204: box.error.SQL_FUNC_WRONG_RET_COUNT
>    205: box.error.FUNC_INVALID_RETURN_TYPE
> +  206: box.error.SESSION_SETTING_INVALID_VALUE
>  ...
>  test_run:cmd("setopt delimiter ''");
>  ---



>From 2a3ed37c777c771725f9d29b03483aa372f829d6 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma at gmail.com>
Date: Sat, 30 Nov 2019 12:59:45 +0300
Subject: [PATCH] box: add SQL settings to _session_settings

Part of #4511

@TarantoolBot document
Title: _session_settings system space
The _session_settings system space used to view or change session
settings.

This space uses a new engine. This allows us to create tuples on
the fly when the get() or select() methods are called. This
engine does not support the insert(), replace(), and delete()
methods. The only way to change the setting value is update(),
which can only be used with the "=" operation.

Because space creates a tuple on the fly, it allows us to get a
tuple without saving it anywhere. But this means that every time
we get a tuple from this system space, it is a new tuple, even if
they look the same:

tarantool> s = box.space._session_settings
tarantool> name = 'sql_default_engine'
tarantool> s:get({name}) == s:get({name})
---
- false
...

Currently, this space contains only SQL settings, since the only
session settings are SQL settings.

List of currently available session settings:

sql_default_engine
sql_defer_foreign_keys
sql_full_column_names
sql_full_metadata
sql_recursive_triggers
sql_reverse_unordered_selects

Debug build also have debug settings that could be obtained from
this sysview:

sql_parser_trace
sql_select_trace
sql_trace
sql_vdbe_addoptrace
sql_vdbe_debug
sql_vdbe_eqp
sql_vdbe_listing
sql_vdbe_trace
sql_where_trace

Example of usage:
tarantool> s = box.space._session_settings
-- View session settings values.
tarantool> s:get({'sql_default_engine'})
---
- ['sql_default_engine', 'memtx']
...

tarantool> s:select()
---
- - ['sql_default_engine', 'memtx']
  - ['sql_defer_foreign_keys', false]
  - ['sql_full_column_names', false]
  - ['sql_full_metadata', false]
  - ['sql_recursive_triggers', true]
  - ['sql_reverse_unordered_selects', false]
...

tarantool> s:select('sql_g', {iterator='LE'})
---
- - ['sql_full_column_names', false]
  - ['sql_full_metadata', false]
  - ['sql_defer_foreign_keys', false]
  - ['sql_default_engine', 'memtx']
...

-- Change session setting value.
tarantool> s:update('sql_default_engine', {{'=', 'value', 'vinyl'}})
---
- ['sql_default_engine', 'vinyl']
...

diff --git a/src/box/errcode.h b/src/box/errcode.h
index c660b1c..11894fc 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -258,6 +258,7 @@ struct errcode_record {
 	/*203 */_(ER_BOOTSTRAP_READONLY,	"Trying to bootstrap a local read-only instance as master") \
 	/*204 */_(ER_SQL_FUNC_WRONG_RET_COUNT,	"SQL expects exactly one argument returned from %s, got %d")\
 	/*205 */_(ER_FUNC_INVALID_RETURN_TYPE,	"Function '%s' returned value of invalid type: expected %s got %s") \
+	/*206 */_(ER_SESSION_SETTING_INVALID_VALUE,	"Session setting %s expected a value of type %s") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/session_settings.h b/src/box/session_settings.h
index 7415e0e..25490a7 100644
--- a/src/box/session_settings.h
+++ b/src/box/session_settings.h
@@ -41,6 +41,7 @@
  * type list is used by setting iterators.
  */
 enum session_setting_type {
+	SESSION_SETTING_SQL,
 	session_setting_type_MAX,
 };
 
diff --git a/src/box/sql.c b/src/box/sql.c
index f1df555..cc82617 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -64,9 +64,14 @@ static const uint32_t default_sql_flags = SQL_ShortColNames
 					  | SQL_AutoIndex
 					  | SQL_RecTriggers;
 
+extern void
+sql_session_settings_init();
+
 void
 sql_init()
 {
+	sql_session_settings_init();
+
 	default_flags |= default_sql_flags;
 
 	current_session()->sql_flags |= default_sql_flags;
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index a106dc3..c7c4901 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -57,6 +57,7 @@
 #include "box/tuple_format.h"
 #include "box/coll_id_cache.h"
 #include "box/user.h"
+#include "box/session_settings.h"
 
 void
 sql_finish_coding(struct Parse *parse_context)
@@ -3295,3 +3296,224 @@ sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
 	*fieldno = i;
 	return 0;
 }
+
+/**
+ * Identifiers of all SQL session setings. The identifier of the
+ * option is equal to its place in the sorted list of session
+ * options of current module.
+ *
+ * It is IMPORTANT that these options are sorted by name. If this
+ * is not the case, the result returned by the _session_settings
+ * space iterator will not be sorted properly.
+ */
+enum {
+	SQL_SESSION_SETTING_DEFAULT_ENGINE = 0,
+	SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS,
+	SQL_SESSION_SETTING_FULL_COLUMN_NAMES,
+	SQL_SESSION_SETTING_FULL_METADATA,
+#ifndef NDEBUG
+	SQL_SESSION_SETTING_PARSER_TRACE,
+#endif
+	SQL_SESSION_SETTING_RECURSIVE_TRIGGERS,
+	SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS,
+#ifndef NDEBUG
+	SQL_SESSION_SETTING_SELECT_TRACE,
+	SQL_SESSION_SETTING_TRACE,
+	SQL_SESSION_SETTING_VDBE_ADDOPTRACE,
+	SQL_SESSION_SETTING_VDBE_DEBUG,
+	SQL_SESSION_SETTING_VDBE_EQP,
+	SQL_SESSION_SETTING_VDBE_LISTING,
+	SQL_SESSION_SETTING_VDBE_TRACE,
+	SQL_SESSION_SETTING_WHERE_TRACE,
+#endif
+	sql_session_setting_MAX,
+};
+
+static const char *sql_session_setting_strs[sql_session_setting_MAX] = {
+	"sql_default_engine",
+	"sql_defer_foreign_keys",
+	"sql_full_column_names",
+	"sql_full_metadata",
+#ifndef NDEBUG
+	"sql_parser_trace",
+#endif
+	"sql_recursive_triggers",
+	"sql_reverse_unordered_selects",
+#ifndef NDEBUG
+	"sql_select_trace",
+	"sql_trace",
+	"sql_vdbe_addoptrace",
+	"sql_vdbe_debug",
+	"sql_vdbe_eqp",
+	"sql_vdbe_listing",
+	"sql_vdbe_trace",
+	"sql_where_trace",
+#endif
+};
+
+/**
+ * A local structure that allows to establish a connection between
+ * parameter and its field type and mask, if it has one.
+ */
+struct sql_option_metadata
+{
+	uint32_t field_type;
+	uint32_t mask;
+};
+
+/**
+ * Variable that contains SQL session option field types and masks
+ * 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_session_opts[] = {
+	/** SQL_SESSION_SETTING_DEFAULT_ENGINE */
+	{FIELD_TYPE_STRING, 0},
+	/** SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS */
+	{FIELD_TYPE_BOOLEAN, SQL_DeferFKs},
+	/** SQL_SESSION_SETTING_FULL_COLUMN_NAMES */
+	{FIELD_TYPE_BOOLEAN, SQL_FullColNames},
+	/** SQL_SESSION_SETTING_FULL_METADATA */
+	{FIELD_TYPE_BOOLEAN, SQL_FullMetadata},
+#ifndef NDEBUG
+	/** SQL_SESSION_SETTING_PARSER_TRACE */
+	{FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG},
+#endif
+	/** SQL_SESSION_SETTING_RECURSIVE_TRIGGERS */
+	{FIELD_TYPE_BOOLEAN, SQL_RecTriggers},
+	/** SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS */
+	{FIELD_TYPE_BOOLEAN, SQL_ReverseOrder},
+#ifndef NDEBUG
+	/** SQL_SESSION_SETTING_SELECT_TRACE */
+	{FIELD_TYPE_BOOLEAN, SQL_SelectTrace},
+	/** SQL_SESSION_SETTING_TRACE */
+	{FIELD_TYPE_BOOLEAN, SQL_SqlTrace},
+	/** SQL_SESSION_SETTING_VDBE_ADDOPTRACE */
+	{FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace},
+	/** SQL_SESSION_SETTING_VDBE_DEBUG */
+	{FIELD_TYPE_BOOLEAN,
+	 SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace},
+	/** SQL_SESSION_SETTING_VDBE_EQP */
+	{FIELD_TYPE_BOOLEAN, SQL_VdbeEQP},
+	/** SQL_SESSION_SETTING_VDBE_LISTING */
+	{FIELD_TYPE_BOOLEAN, SQL_VdbeListing},
+	/** SQL_SESSION_SETTING_VDBE_TRACE */
+	{FIELD_TYPE_BOOLEAN, SQL_VdbeTrace},
+	/** SQL_SESSION_SETTING_WHERE_TRACE */
+	{FIELD_TYPE_BOOLEAN, SQL_WhereTrace},
+#endif
+};
+
+static void
+sql_session_setting_get(int id, const char **mp_pair, const char **mp_pair_end)
+{
+	assert(id >= 0 && id < sql_session_setting_MAX);
+	struct session *session = current_session();
+	uint32_t flags = session->sql_flags;
+	struct sql_option_metadata *opt = &sql_session_opts[id];
+	uint32_t mask = opt->mask;
+	const char *name = sql_session_setting_strs[id];
+	size_t name_len = strlen(name);
+	size_t engine_len;
+	const char *engine;
+	size_t size = mp_sizeof_array(2) + mp_sizeof_str(name_len);
+	/*
+	 * Currently, SQL session settings are of a boolean or
+	 * string type.
+	 */
+	bool is_bool = opt->field_type == FIELD_TYPE_BOOLEAN;
+	if (is_bool) {
+		size += mp_sizeof_bool(true);
+	} else {
+		assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE);
+		engine = sql_storage_engine_strs[session->sql_default_engine];
+		engine_len = strlen(engine);
+		size += mp_sizeof_str(engine_len);
+	}
+
+	char *pos = static_alloc(size);
+	assert(pos != NULL);
+	char *pos_end = mp_encode_array(pos, 2);
+	pos_end = mp_encode_str(pos_end, name, name_len);
+	if (is_bool)
+		pos_end = mp_encode_bool(pos_end, (flags & mask) == mask);
+	else
+		pos_end = mp_encode_str(pos_end, engine, engine_len);
+	*mp_pair = pos;
+	*mp_pair_end = pos_end;
+}
+
+static int
+sql_set_boolean_option(int id, bool value)
+{
+	struct session *session = current_session();
+	struct sql_option_metadata *option = &sql_session_opts[id];
+	assert(option->field_type == FIELD_TYPE_BOOLEAN);
+	if (value)
+		session->sql_flags |= option->mask;
+	else
+		session->sql_flags &= ~option->mask;
+#ifndef NDEBUG
+	if (id == SQL_SESSION_SETTING_PARSER_TRACE) {
+		if (value)
+			sqlParserTrace(stdout, "parser: ");
+		else
+			sqlParserTrace(NULL, NULL);
+	}
+#endif
+	return 0;
+}
+
+static int
+sql_set_string_option(int id, const char *value)
+{
+	assert(sql_session_opts[id].field_type = FIELD_TYPE_STRING);
+	assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE);
+	(void)id;
+	enum sql_storage_engine engine = STR2ENUM(sql_storage_engine, value);
+	if (engine == sql_storage_engine_MAX) {
+		diag_set(ClientError, ER_NO_SUCH_ENGINE, value);
+		return -1;
+	}
+	current_session()->sql_default_engine = engine;
+	return 0;
+}
+
+static int
+sql_session_setting_set(int id, const char *mp_value)
+{
+	assert(id >= 0 && id < sql_session_setting_MAX);
+	enum mp_type mtype = mp_typeof(*mp_value);
+	enum field_type stype = sql_session_opts[id].field_type;
+	uint32_t len;
+	const char *tmp;
+	switch(stype) {
+	case FIELD_TYPE_BOOLEAN:
+		if (mtype != MP_BOOL)
+			break;
+		return sql_set_boolean_option(id, mp_decode_bool(&mp_value));
+	case FIELD_TYPE_STRING:
+		if (mtype != MP_STR)
+			break;
+		tmp = mp_decode_str(&mp_value, &len);
+		tmp = tt_cstr(tmp, len);
+		return sql_set_string_option(id, tmp);
+	default:
+		unreachable();
+	}
+	diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE,
+		 sql_session_setting_strs[id], field_type_strs[stype]);
+	return -1;
+}
+
+void
+sql_session_settings_init()
+{
+	struct session_setting_module *module =
+		&session_setting_modules[SESSION_SETTING_SQL];
+	module->settings = sql_session_setting_strs;
+	module->setting_count = sql_session_setting_MAX;
+	module->get = sql_session_setting_get;
+	module->set = sql_session_setting_set;
+}
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 75d53cf..4e68129 100644
--- a/test/box/gh-4511-access-settings-from-any-frontend.result
+++ b/test/box/gh-4511-access-settings-from-any-frontend.result
@@ -45,72 +45,229 @@ 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'})
+--
+-- Check select() method of session_settings space. Should work
+-- the same way as an ordinary space with an index of the type
+-- "TREE".
+--
+t = box.schema.space.create('settings', {format = s:format()})
+ | ---
+ | ...
+_ = t:create_index('primary')
+ | ---
+ | ...
+for _,value in s:pairs() do t:insert(value) end
+ | ---
+ | ...
+
+test_run:cmd('setopt delimiter ";"')
+ | ---
+ | - true
+ | ...
+function check_sorting(ss, ts, key)
+    local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'}
+    for _, it in pairs(iterators_list) do
+        local view_space = ss:select({key}, {iterator = it})
+        local test_space = ts:select({key}, {iterator = it})
+        for key, value in pairs(view_space) do
+            if test_space[key].name ~= value.name then
+                return {
+                    err = 'bad sorting', type = it,
+                    exp = test_space[key].name, got = value.name
+                }
+            end
+        end
+    end
+end;
  | ---
  | ...
-s:select()
+test_run:cmd('setopt delimiter ""');
  | ---
- | - []
+ | - true
  | ...
-s:select({}, {iterator='EQ'})
+
+check_sorting(s, t)
  | ---
- | - []
  | ...
-s:select({}, {iterator='ALL'})
+check_sorting(s, t, 'abcde')
  | ---
- | - []
  | ...
-s:select({}, {iterator='GE'})
+check_sorting(s, t, 'sql_d')
  | ---
- | - []
  | ...
-s:select({}, {iterator='GT'})
+check_sorting(s, t, 'sql_v')
  | ---
- | - []
  | ...
-s:select({}, {iterator='REQ'})
+check_sorting(s, t, 'sql_defer_foreign_keys')
  | ---
- | - []
  | ...
-s:select({}, {iterator='LE'})
+
+t:drop()
  | ---
- | - []
  | ...
-s:select({}, {iterator='LT'})
+
+-- Check get() method of session_settings space.
+s:get({'sql_defer_foreign_keys'})
  | ---
- | - []
+ | - ['sql_defer_foreign_keys', false]
  | ...
-s:select({'a'}, {iterator='EQ'})
+s:get({'sql_recursive_triggers'})
  | ---
- | - []
+ | - ['sql_recursive_triggers', true]
  | ...
-s:select({'a'}, {iterator='ALL'})
+s:get({'sql_reverse_unordered_selects'})
  | ---
- | - []
+ | - ['sql_reverse_unordered_selects', false]
  | ...
-s:select({'a'}, {iterator='GE'})
+s:get({'sql_default_engine'})
  | ---
- | - []
+ | - ['sql_default_engine', 'memtx']
  | ...
-s:select({'a'}, {iterator='GT'})
+s:get({'abcd'})
  | ---
- | - []
  | ...
-s:select({'a'}, {iterator='REQ'})
+
+-- Check pairs() method of session_settings space.
+t = {}
  | ---
- | - []
  | ...
-s:select({'a'}, {iterator='LE'})
+for key, value in s:pairs() do table.insert(t, {key, value}) end
  | ---
- | - []
  | ...
-s:select({'a'}, {iterator='LT'})
+#t == s:count()
  | ---
- | - []
+ | - true
  | ...
 
--- Currently there is nothing to update, but update() should work.
-s:update('some_option', {{'=', 'value', true}})
+-- Check update() method of session_settings space.
+
+-- Correct updates.
+s:update('sql_defer_foreign_keys', {{'=', 'value', true}})
+ | ---
+ | - ['sql_defer_foreign_keys', true]
+ | ...
+s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}})
+ | ---
+ | - ['sql_defer_foreign_keys', false]
+ | ...
+s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
+ | ---
+ | - ['sql_default_engine', 'vinyl']
+ | ...
+s:update('sql_default_engine', {{':', 'value', 1, 5, 'memtx'}})
+ | ---
+ | - ['sql_default_engine', 'memtx']
+ | ...
+s:update('a', {{'=', 2, 1}})
+ | ---
+ | ...
+
+-- Inorrect updates.
+s:update({{'sql_defer_foreign_keys'}}, {{'=', 'value', true}})
+ | ---
+ | - error: 'Supplied key type of part 0 does not match index part type: expected string'
+ | ...
+
+s:update('sql_defer_foreign_keys', {'=', 'value', true})
+ | ---
+ | - error: Illegal parameters, update operation must be an array {op,..}
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 'value', true}, {'=', 2, true}})
+ | ---
+ | - ['sql_defer_foreign_keys', true]
+ | ...
+s:update('sql_defer_foreign_keys', {{}})
+ | ---
+ | - error: Illegal parameters, update operation must be an array {op,..}, got empty
+ |     array
+ | ...
+s:update('sql_defer_foreign_keys', {{'='}})
+ | ---
+ | - error: Unknown UPDATE operation
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 'value'}})
+ | ---
+ | - error: Unknown UPDATE operation
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 'value', true, 1}})
+ | ---
+ | - error: Unknown UPDATE operation
+ | ...
+
+s:update('sql_defer_foreign_keys', {{'+', 'value', 2}})
+ | ---
+ | - error: 'Argument type in operation ''+'' on field ''value'' does not match field
+ |     type: expected a number'
+ | ...
+s:update('sql_defer_foreign_keys', {{'-', 'value', 2}})
+ | ---
+ | - error: 'Argument type in operation ''-'' on field ''value'' does not match field
+ |     type: expected a number'
+ | ...
+s:update('sql_defer_foreign_keys', {{'&', 'value', 2}})
+ | ---
+ | - error: 'Argument type in operation ''&'' on field ''value'' does not match field
+ |     type: expected a positive integer'
+ | ...
+s:update('sql_defer_foreign_keys', {{'|', 'value', 2}})
+ | ---
+ | - error: 'Argument type in operation ''|'' on field ''value'' does not match field
+ |     type: expected a positive integer'
+ | ...
+s:update('sql_defer_foreign_keys', {{'^', 'value', 2}})
+ | ---
+ | - error: 'Argument type in operation ''^'' on field ''value'' does not match field
+ |     type: expected a positive integer'
+ | ...
+s:update('sql_defer_foreign_keys', {{'!', 'value', 2}})
+ | ---
+ | - error: Tuple field count 3 does not match space field count 2
+ | ...
+s:update('sql_defer_foreign_keys', {{'#', 'value', 2}})
+ | ---
+ | - error: Tuple field count 1 does not match space field count 2
+ | ...
+s:update('sql_defer_foreign_keys', {{1, 'value', true}})
+ | ---
+ | - error: Illegal parameters, update operation name must be a string
+ | ...
+s:update('sql_defer_foreign_keys', {{{1}, 'value', true}})
+ | ---
+ | - error: Illegal parameters, update operation name must be a string
+ | ...
+
+s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}})
+ | ---
+ | - error: Illegal parameters, field id must be a number or a string
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}})
+ | ---
+ | - error: Attempt to modify a tuple field which is part of index 'primary' in space
+ |     '_session_settings'
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}})
+ | ---
+ | - error: Attempt to modify a tuple field which is part of index 'primary' in space
+ |     '_session_settings'
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 3, true}})
+ | ---
+ | - error: Tuple field count 3 does not match space field count 2
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 'some text', true}})
+ | ---
+ | - error: Field 'some text' was not found in the tuple
+ | ...
+
+s:update('sql_defer_foreign_keys', {{'=', 'value', 1}})
+ | ---
+ | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}})
+ | ---
+ | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
  | ---
+ | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
  | ...
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 3304454..9642f68 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
@@ -19,23 +19,88 @@ 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}})
+--
+-- Check select() method of session_settings space. Should work
+-- the same way as an ordinary space with an index of the type
+-- "TREE".
+--
+t = box.schema.space.create('settings', {format = s:format()})
+_ = t:create_index('primary')
+for _,value in s:pairs() do t:insert(value) end
+
+test_run:cmd('setopt delimiter ";"')
+function check_sorting(ss, ts, key)
+    local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'}
+    for _, it in pairs(iterators_list) do
+        local view_space = ss:select({key}, {iterator = it})
+        local test_space = ts:select({key}, {iterator = it})
+        for key, value in pairs(view_space) do
+            if test_space[key].name ~= value.name then
+                return {
+                    err = 'bad sorting', type = it,
+                    exp = test_space[key].name, got = value.name
+                }
+            end
+        end
+    end
+end;
+test_run:cmd('setopt delimiter ""');
+
+check_sorting(s, t)
+check_sorting(s, t, 'abcde')
+check_sorting(s, t, 'sql_d')
+check_sorting(s, t, 'sql_v')
+check_sorting(s, t, 'sql_defer_foreign_keys')
+
+t:drop()
+
+-- Check get() method of session_settings space.
+s:get({'sql_defer_foreign_keys'})
+s:get({'sql_recursive_triggers'})
+s:get({'sql_reverse_unordered_selects'})
+s:get({'sql_default_engine'})
+s:get({'abcd'})
+
+-- Check pairs() method of session_settings space.
+t = {}
+for key, value in s:pairs() do table.insert(t, {key, value}) end
+#t == s:count()
+
+-- Check update() method of session_settings space.
+
+-- Correct updates.
+s:update('sql_defer_foreign_keys', {{'=', 'value', true}})
+s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}})
+s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
+s:update('sql_default_engine', {{':', 'value', 1, 5, 'memtx'}})
+s:update('a', {{'=', 2, 1}})
+
+-- Inorrect updates.
+s:update({{'sql_defer_foreign_keys'}}, {{'=', 'value', true}})
+
+s:update('sql_defer_foreign_keys', {'=', 'value', true})
+s:update('sql_defer_foreign_keys', {{'=', 'value', true}, {'=', 2, true}})
+s:update('sql_defer_foreign_keys', {{}})
+s:update('sql_defer_foreign_keys', {{'='}})
+s:update('sql_defer_foreign_keys', {{'=', 'value'}})
+s:update('sql_defer_foreign_keys', {{'=', 'value', true, 1}})
+
+s:update('sql_defer_foreign_keys', {{'+', 'value', 2}})
+s:update('sql_defer_foreign_keys', {{'-', 'value', 2}})
+s:update('sql_defer_foreign_keys', {{'&', 'value', 2}})
+s:update('sql_defer_foreign_keys', {{'|', 'value', 2}})
+s:update('sql_defer_foreign_keys', {{'^', 'value', 2}})
+s:update('sql_defer_foreign_keys', {{'!', 'value', 2}})
+s:update('sql_defer_foreign_keys', {{'#', 'value', 2}})
+s:update('sql_defer_foreign_keys', {{1, 'value', true}})
+s:update('sql_defer_foreign_keys', {{{1}, 'value', true}})
+
+s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}})
+s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}})
+s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}})
+s:update('sql_defer_foreign_keys', {{'=', 3, true}})
+s:update('sql_defer_foreign_keys', {{'=', 'some text', true}})
+
+s:update('sql_defer_foreign_keys', {{'=', 'value', 1}})
+s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}})
+s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
diff --git a/test/box/misc.result b/test/box/misc.result
index d2a2030..004faaa 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -554,6 +554,7 @@ t;
   203: box.error.BOOTSTRAP_READONLY
   204: box.error.SQL_FUNC_WRONG_RET_COUNT
   205: box.error.FUNC_INVALID_RETURN_TYPE
+  206: box.error.SESSION_SETTING_INVALID_VALUE
 ...
 test_run:cmd("setopt delimiter ''");
 ---


More information about the Tarantool-patches mailing list