Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v5 1/1] box: introduce _session_settings system space
@ 2019-12-09 13:23 imeevma
  2019-12-11 23:47 ` Vladislav Shpilevoy
  0 siblings, 1 reply; 2+ messages in thread
From: imeevma @ 2019-12-09 13:23 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

This patch creates the _session_settings space. This space
contains names and values of the session settings. User can use
this system space to view or change 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_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_recursive_triggers', true]
  - ['sql_reverse_unordered_selects', false]
...

tarantool> s:select('sql_g', {iterator='LE'})
---
- - ['sql_full_column_names', 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']
...
---
https://github.com/tarantool/tarantool/issues/4511
https://github.com/tarantool/tarantool/tree/imeevma/gh-4511-new-engine

 src/box/CMakeLists.txt             |   1 +
 src/box/bootstrap.snap             | Bin 5944 -> 5997 bytes
 src/box/box.cc                     |   4 +
 src/box/loophole.c                 | 431 +++++++++++++++++++++++++++++++++++++
 src/box/loophole.h                 |  55 +++++
 src/box/lua/space.cc               |   2 +
 src/box/lua/upgrade.lua            |  23 ++
 src/box/schema_def.h               |   2 +
 src/box/session.cc                 |  87 ++++++++
 src/box/session.h                  |  59 +++++
 src/box/sql.h                      |  47 ++++
 src/box/sql/build.c                | 203 +++++++++++++++++
 test/app-tap/tarantoolctl.test.lua |   4 +-
 test/box-py/bootstrap.result       |   5 +-
 test/box/access_sysview.result     |   6 +-
 test/box/alter.result              |   5 +-
 test/box/loophole.result           | 264 +++++++++++++++++++++++
 test/box/loophole.test.lua         |  97 +++++++++
 test/wal_off/alter.result          |   2 +-
 19 files changed, 1288 insertions(+), 9 deletions(-)
 create mode 100644 src/box/loophole.c
 create mode 100644 src/box/loophole.h
 create mode 100644 test/box/loophole.result
 create mode 100644 test/box/loophole.test.lua

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 5cd5cba8..0f94a5b 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -78,6 +78,7 @@ add_library(box STATIC
     memtx_space.c
     sysview.c
     blackhole.c
+    loophole.c
     vinyl.c
     vy_stmt.c
     vy_mem.c

diff --git a/src/box/box.cc b/src/box/box.cc
index b119c92..caffd34 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -53,6 +53,7 @@
 #include "memtx_engine.h"
 #include "sysview.h"
 #include "blackhole.h"
+#include "loophole.h"
 #include "vinyl.h"
 #include "space.h"
 #include "index.h"
@@ -1693,6 +1694,9 @@ engine_init()
 	struct sysview_engine *sysview = sysview_engine_new_xc();
 	engine_register((struct engine *)sysview);
 
+	struct engine *loophole = loophole_engine_new_xc();
+	engine_register(loophole);
+
 	struct engine *blackhole = blackhole_engine_new_xc();
 	engine_register(blackhole);
 
diff --git a/src/box/loophole.c b/src/box/loophole.c
new file mode 100644
index 0000000..775f5a1
--- /dev/null
+++ b/src/box/loophole.c
@@ -0,0 +1,431 @@
+/*
+ * 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 <COPYRIGHT HOLDER> ``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
+ * <COPYRIGHT HOLDER> 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 "loophole.h"
+#include "session.h"
+#include "schema.h"
+#include "engine.h"
+#include "tuple.h"
+#include "xrow.h"
+
+struct loophole_index {
+	struct index base;
+	struct tuple_format *format;
+};
+
+struct loophole_iterator {
+	/** Base iterator. Must be the first member. */
+	struct iterator base;
+	/** Format of the tuples this iterator returns. */
+	struct tuple_format *format;
+	/** ID of the option in global list of the options. */
+	int current_id;
+	/** Decoded key. */
+	char *key;
+	/** Type of iterator. */
+	enum iterator_type iterator_type;
+};
+
+static void
+loophole_iterator_free(struct iterator *ptr)
+{
+	struct loophole_iterator *it = (struct loophole_iterator *)ptr;
+	free(it->key);
+	free(it);
+}
+
+static int
+loophole_iterator_next(struct iterator *iterator, struct tuple **ret)
+{
+	struct loophole_iterator *it = (struct loophole_iterator *)iterator;
+	it->current_id = session_setting_id_by_name(it->key, it->current_id,
+						    it->iterator_type);
+	int id = it->current_id;
+	it->current_id += iterator_type_is_reverse(it->iterator_type) ? -1 : 1;
+	return session_setting_tuple(it->format, id, ret);
+}
+
+static void
+loophole_index_destroy(struct index *index)
+{
+	free_session_settings();
+	free(index);
+}
+
+static struct iterator *
+loophole_index_create_iterator(struct index *base, enum iterator_type type,
+			       const char *key, uint32_t part_count)
+{
+	struct loophole_index *index = (struct loophole_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 loophole_iterator *it =
+		(struct loophole_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.next = loophole_iterator_next;
+	it->base.free = loophole_iterator_free;
+	it->key = decoded_key;
+	it->iterator_type = type;
+	it->format = index->format;
+	it->current_id = iterator_type_is_reverse(type) ?
+			 session_get_settings_max_id() - 1 : 0;
+	return (struct iterator *)it;
+	return NULL;
+}
+
+static int
+loophole_index_get(struct index *base, const char *key,
+		   uint32_t part_count, struct tuple **result)
+{
+	struct loophole_index *index = (struct loophole_index *)base;
+	(void)base;
+	assert(part_count == 1);
+	(void)part_count;
+	uint32_t len;
+	const char *tmp = mp_decode_str(&key, &len);
+	const char *decoded_key = tt_cstr(tmp, len);
+	int id = session_setting_id_by_name(decoded_key, 0, ITER_EQ);
+	return session_setting_tuple(index->format, id, result);
+}
+
+static const struct index_vtab loophole_index_vtab = {
+	/* .destroy = */ loophole_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 = */ loophole_index_get,
+	/* .replace = */ generic_index_replace,
+	/* .create_iterator = */ loophole_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
+loophole_space_destroy(struct space *space)
+{
+	free(space);
+}
+
+static int
+loophole_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, "Loophole", "replace()");
+	return -1;
+}
+
+static int
+loophole_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, "Loophole", "delete()");
+	return -1;
+}
+
+static int
+loophole_space_execute_update(struct space *space, struct txn *txn,
+			      struct request *request, struct tuple **result)
+{
+	(void)txn;
+	uint32_t len;
+	const char *tmp;
+	const char *data;
+
+	data = request->key;
+	uint32_t key_len = mp_decode_array(&data);
+	if (key_len == 0) {
+		diag_set(ClientError, ER_EXACT_MATCH, 1, 0);
+		return -1;
+	}
+	if (key_len > 1 || mp_typeof(*data) != MP_STR) {
+		diag_set(ClientError, ER_KEY_PART_TYPE, 0, "string");
+		return -1;
+	}
+	tmp = mp_decode_str(&data, &len);
+	const char *decoded_key = tt_cstr(tmp, len);
+	int id = session_setting_id_by_name(decoded_key, 0, ITER_EQ);
+	if (id == session_get_settings_max_id()) {
+		*result = NULL;
+		return 0;
+	}
+
+	data = request->tuple;
+	assert(mp_typeof(*data) == MP_ARRAY);
+	uint32_t update_counter = mp_decode_array(&data);
+	if (mp_typeof(*data) != MP_ARRAY) {
+		diag_set(ClientError, ER_ILLEGAL_PARAMS, "update operation "
+			 "must be an array {op,..}");
+		return -1;
+	}
+	if (update_counter > 1) {
+		diag_set(ClientError, ER_UNSUPPORTED, "Loophole", "update of "
+			 "more than one field");
+		return -1;
+	}
+	if (mp_decode_array(&data) != 3) {
+		diag_set(ClientError, ER_UNKNOWN_UPDATE_OP);
+		return -1;
+	}
+	if (mp_typeof(*data) != MP_STR) {
+		diag_set(ClientError, ER_ILLEGAL_PARAMS, "update operation "
+			 "name must be a string");
+		return -1;
+	}
+	tmp = mp_decode_str(&data, &len);
+	if (tmp[0] != '=' || len != 1) {
+		diag_set(ClientError, ER_UNSUPPORTED, "Loophole", "not '='");
+		return -1;
+	}
+	if (mp_typeof(*data) == MP_UINT) {
+		uint32_t id = mp_decode_uint(&data);
+		if (id - request->index_base == 0) {
+			diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
+				 space_index(space, 0)->def->name,
+				 space_name(space));
+			return -1;
+		}
+		if (id - request->index_base != 1) {
+			diag_set(ClientError, ER_EXACT_FIELD_COUNT, id,
+				 1 + request->index_base);
+			return -1;
+		}
+	} else if (mp_typeof(*data) == MP_STR) {
+		tmp = mp_decode_str(&data, &len);
+		const char *field_name = tt_cstr(tmp, len);
+		if (strcmp(field_name, "name") == 0) {
+			diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
+				 space_index(space, 0)->def->name,
+				 space_name(space));
+			return -1;
+		}
+		if (strcmp(field_name, "value") != 0) {
+			diag_set(ClientError, ER_NO_SUCH_FIELD_NAME,
+				 field_name);
+			return -1;
+		}
+	} else {
+		diag_set(ClientError, ER_ILLEGAL_PARAMS,
+			 "field id must be a number or a string");
+		return -1;
+	}
+	const char *value = data;
+
+	if (session_setting_new_value(id, value) != 0)
+		return -1;
+	return session_setting_tuple(space->format, id, result);
+}
+
+static int
+loophole_space_execute_upsert(struct space *space, struct txn *txn,
+			      struct request *request)
+{
+	(void)space;
+	(void)txn;
+	(void)request;
+	diag_set(ClientError, ER_UNSUPPORTED, "Loophole", "upsert()");
+	return -1;
+}
+
+static struct index *
+loophole_space_create_index(struct space *space, struct index_def *def)
+{
+	if (space->def->id != BOX_SESSION_SETTINGS_ID) {
+		diag_set(ClientError, ER_UNSUPPORTED, "Loophole",
+			 "create_index()");
+		return NULL;
+	}
+	new_session_settings();
+
+	struct loophole_index *index =
+		(struct loophole_index *)calloc(1, sizeof(*index));
+	if (index == NULL) {
+		diag_set(OutOfMemory, sizeof(*index), "calloc", "index");
+		return NULL;
+	}
+	if (index_create(&index->base, space->engine, &loophole_index_vtab,
+			 def) != 0) {
+		free(index);
+		return NULL;
+	}
+
+	index->format = space->format;
+	return &index->base;
+}
+
+static const struct space_vtab loophole_space_vtab = {
+	/* .destroy = */ loophole_space_destroy,
+	/* .bsize = */ generic_space_bsize,
+	/* .execute_replace = */ loophole_space_execute_replace,
+	/* .execute_delete = */ loophole_space_execute_delete,
+	/* .execute_update = */ loophole_space_execute_update,
+	/* .execute_upsert = */ loophole_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 = */ loophole_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,
+};
+
+static void
+loophole_engine_shutdown(struct engine *engine)
+{
+	free(engine);
+}
+
+static struct space *
+loophole_engine_create_space(struct engine *engine, struct space_def *def,
+			     struct rlist *key_list)
+{
+	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);
+	if (space_create(space, engine, &loophole_space_vtab, def, key_list,
+			 format) != 0) {
+		free(space);
+		return NULL;
+	}
+	/* Format is now referenced by the space. */
+	tuple_format_unref(format);
+	return space;
+}
+
+static const struct engine_vtab loophole_engine_vtab = {
+	/* .shutdown = */ loophole_engine_shutdown,
+	/* .create_space = */ loophole_engine_create_space,
+	/* .prepare_join = */ generic_engine_prepare_join,
+	/* .join = */ generic_engine_join,
+	/* .complete_join = */ generic_engine_complete_join,
+	/* .begin = */ generic_engine_begin,
+	/* .begin_statement = */ generic_engine_begin_statement,
+	/* .prepare = */ generic_engine_prepare,
+	/* .commit = */ generic_engine_commit,
+	/* .rollback_statement = */ generic_engine_rollback_statement,
+	/* .rollback = */ generic_engine_rollback,
+	/* .switch_to_ro = */ generic_engine_switch_to_ro,
+	/* .bootstrap = */ generic_engine_bootstrap,
+	/* .begin_initial_recovery = */ generic_engine_begin_initial_recovery,
+	/* .begin_final_recovery = */ generic_engine_begin_final_recovery,
+	/* .end_recovery = */ generic_engine_end_recovery,
+	/* .begin_checkpoint = */ generic_engine_begin_checkpoint,
+	/* .wait_checkpoint = */ generic_engine_wait_checkpoint,
+	/* .commit_checkpoint = */ generic_engine_commit_checkpoint,
+	/* .abort_checkpoint = */ generic_engine_abort_checkpoint,
+	/* .collect_garbage = */ generic_engine_collect_garbage,
+	/* .backup = */ generic_engine_backup,
+	/* .memory_stat = */ generic_engine_memory_stat,
+	/* .reset_stat = */ generic_engine_reset_stat,
+	/* .check_space_def = */ generic_engine_check_space_def,
+};
+
+struct engine *
+loophole_engine_new(void)
+{
+	struct engine *loophole = calloc(1, sizeof(*loophole));
+	if (loophole == NULL) {
+		diag_set(OutOfMemory, sizeof(*loophole), "calloc", "loophole");
+		return NULL;
+	}
+
+	loophole->vtab = &loophole_engine_vtab;
+	loophole->name = "loophole";
+	loophole->flags = ENGINE_BYPASS_TX;
+	return loophole;
+}
diff --git a/src/box/loophole.h b/src/box/loophole.h
new file mode 100644
index 0000000..1b78d33
--- /dev/null
+++ b/src/box/loophole.h
@@ -0,0 +1,55 @@
+#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 <COPYRIGHT HOLDER> ``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
+ * <COPYRIGHT HOLDER> 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 <stddef.h>
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+struct engine *
+loophole_engine_new(void);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+
+#include "diag.h"
+
+static inline struct engine *
+loophole_engine_new_xc(void)
+{
+	struct engine *loophole = loophole_engine_new();
+	if (loophole == NULL)
+		diag_raise();
+	return loophole;
+}
+
+#endif /* defined(__plusplus) */
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 e71b7fb..165a7b9 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_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',
+                  'loophole', 0, {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()
+    create_session_settings_space()
+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..c6612c3 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
diff --git a/src/box/session.cc b/src/box/session.cc
index 461d1cf..d72d917 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -36,6 +36,7 @@
 #include "user.h"
 #include "error.h"
 #include "tt_static.h"
+#include "sql.h"
 
 const char *session_type_strs[] = {
 	"background",
@@ -61,6 +62,8 @@ struct session_vtab session_vtab_registry[] = {
 };
 
 static struct mh_i64ptr_t *session_registry;
+static struct session_setting *session_settings = NULL;
+static int session_settings_max_id = 0;
 
 uint32_t default_flags = 0;
 
@@ -338,3 +341,87 @@ generic_session_sync(struct session *session)
 	(void) session;
 	return 0;
 }
+
+struct session_setting {
+	const char *name;
+	uint32_t id;
+};
+
+int
+new_session_settings()
+{
+	int max_id = sql_session_setting_max_id();
+	int size = max_id * sizeof(*session_settings);
+	session_settings = (struct session_setting *)malloc(size);
+	if (session_settings == NULL) {
+		diag_set(OutOfMemory, size, "malloc", "session_settings");
+		return -1;
+	}
+	session_settings_max_id = max_id;
+	for (int i = 0; i < max_id; ++i)
+		session_settings[i].name = sql_session_setting_name(i);
+	return 0;
+}
+
+void
+free_session_settings()
+{
+	free(session_settings);
+	session_settings = NULL;
+}
+
+int
+session_setting_id_by_name(const char *key, int next_id,
+			   enum iterator_type type)
+{
+	assert(session_settings_max_id == sql_session_setting_max_id());
+	int id = next_id;
+	if (key == NULL)
+		return id;
+	if (iterator_type_is_reverse(type)) {
+		for (; id >= 0; --id) {
+			int compare = strcmp(session_settings[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 < session_settings_max_id; ++id) {
+			int compare = strcmp(session_settings[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;
+}
+
+int
+session_setting_tuple(struct tuple_format *format, int id,
+		      struct tuple **result)
+{
+	if (id > session_settings_max_id) {
+		*result = NULL;
+		return 0;
+	}
+	return sql_session_setting_tuple(format, id, result);
+}
+
+int
+session_setting_new_value(int id, const char *value)
+{
+	return sql_session_setting_new_values(id, value);
+}
+
+int
+session_get_settings_max_id()
+{
+	return session_settings_max_id;
+}
diff --git a/src/box/session.h b/src/box/session.h
index eff3d7a..09c0055 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -36,13 +36,16 @@
 #include "fiber.h"
 #include "user.h"
 #include "authentication.h"
+#include "iterator_type.h"
 
 #if defined(__cplusplus)
 extern "C" {
 #endif /* defined(__cplusplus) */
 
 struct port;
+struct tuple;
 struct session_vtab;
+struct tuple_format;
 
 void
 session_init();
@@ -347,6 +350,62 @@ generic_session_fd(struct session *session);
 int64_t
 generic_session_sync(struct session *session);
 
+int
+new_session_settings();
+
+void
+free_session_settings();
+
+/**
+ * Return 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.
+ */
+int
+session_setting_id_by_name(const char *key, int next_id,
+			   enum iterator_type type);
+
+/**
+ * Return a tuple with the specified format, which contains the
+ * name and value of the setting with the specified ID.
+ *
+ * @param format Format of tuple to return.
+ * @param current_id ID of tuple to return.
+ * @param result[out] Returned tuple.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+session_setting_tuple(struct tuple_format *format, int current_id,
+		      struct tuple **result);
+
+/**
+ * Set new value to session setting. Value given in MsgPack
+ * format.
+ *
+ * @param id ID of setting to set new value.
+ * @param value MsgPack contains value to set.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+session_setting_new_value(int id, const char *value);
+
+/**
+ * Return number after session settings max id.
+ *
+ * @retval A number equal to the max session setting ID plus one.
+ */
+int
+session_get_settings_max_id();
+
 #if defined(__cplusplus)
 } /* extern "C" */
 
diff --git a/src/box/sql.h b/src/box/sql.h
index 0fa52fc..35e0f2b 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,52 @@ void
 vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref,
 			     struct tuple *tuple);
 
+/**
+ * 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 current_id ID of tuple to return.
+ * @param result[out] Returned tuple.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_session_setting_tuple(struct tuple_format *format, int id,
+			  struct tuple **result);
+
+/**
+ * Return number after SQL session settings max id.
+ *
+ * @retval A number that follows SQL session settings max id.
+ */
+int
+sql_session_setting_max_id();
+
+/**
+ * Set new value to SQL session setting. Value given in MsgPack
+ * format.
+ *
+ * @param id ID of SQL setting to set new value.
+ * @param value MsgPack contains value to set.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_session_setting_new_values(int id, const char *value);
+
+/**
+ * Return name of the setting with given ID.
+ *
+ * @param id ID of SQL setting which name is returned.
+ *
+ * @retval SQL setting name.
+ */
+const char *
+sql_session_setting_name(int id);
+
 #if defined(__cplusplus)
 } /* extern "C" { */
 #endif
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 51cd7ce..e7ecc61 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -46,6 +46,7 @@
 #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"
@@ -3242,3 +3243,205 @@ sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
 	*fieldno = i;
 	return 0;
 }
+
+/**
+ * Identifiers of all SQL session options that can be viewed. The
+ * identifier of the option is equal to its place in the sorted
+ * list of session options, which starts at 0.
+ *
+ * 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_OPTION_DEFAULT_ENGINE = 0,
+	SQL_SESSION_OPTION_DEFER_FOREIGN_KEYS,
+	SQL_SESSION_OPTION_FULL_COLUMN_NAMES,
+#ifndef NDEBUG
+	SQL_SESSION_OPTION_PARSER_TRACE,
+#endif
+	SQL_SESSION_OPTION_RECURSIVE_TRIGGERS,
+	SQL_SESSION_OPTION_REVERSE_UNORDERED_SELECTS,
+#ifndef NDEBUG
+	SQL_SESSION_OPTION_SELECT_TRACE,
+	SQL_SESSION_OPTION_TRACE,
+	SQL_SESSION_OPTION_VDBE_ADDOPTRACE,
+	SQL_SESSION_OPTION_VDBE_DEBUG,
+	SQL_SESSION_OPTION_VDBE_EQP,
+	SQL_SESSION_OPTION_VDBE_LISTING,
+	SQL_SESSION_OPTION_VDBE_TRACE,
+	SQL_SESSION_OPTION_WHERE_TRACE,
+#endif
+	SQL_SESSION_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 session 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_session_opts[] = {
+	/** SQL_SESSION_OPTION_DEFAULT_ENGINE */
+	{"sql_default_engine", FIELD_TYPE_STRING, 0},
+	/** SQL_SESSION_OPTION_DEFER_FOREIGN_KEYS */
+	{"sql_defer_foreign_keys", FIELD_TYPE_BOOLEAN, SQL_DeferFKs},
+	/** SQL_SESSION_OPTION_FULL_COLUMN_NAMES */
+	{"sql_full_column_names", FIELD_TYPE_BOOLEAN, SQL_FullColNames},
+#ifndef NDEBUG
+	/** SQL_SESSION_OPTION_PARSER_TRACE */
+	{"sql_parser_trace", FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG},
+#endif
+	/** SQL_SESSION_OPTION_RECURSIVE_TRIGGERS */
+	{"sql_recursive_triggers", FIELD_TYPE_BOOLEAN, SQL_RecTriggers},
+	/** SQL_SESSION_OPTION_REVERSE_UNORDERED_SELECTS */
+	{"sql_reverse_unordered_selects", FIELD_TYPE_BOOLEAN, SQL_ReverseOrder},
+#ifndef NDEBUG
+	/** SQL_SESSION_OPTION_SELECT_TRACE */
+	{"sql_select_trace", FIELD_TYPE_BOOLEAN, SQL_SelectTrace},
+	/** SQL_SESSION_OPTION_TRACE */
+	{"sql_trace", FIELD_TYPE_BOOLEAN, SQL_SqlTrace},
+	/** SQL_SESSION_OPTION_VDBE_ADDOPTRACE */
+	{"sql_vdbe_addoptrace", FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace},
+	/** SQL_SESSION_OPTION_VDBE_DEBUG */
+	{"sql_vdbe_debug", FIELD_TYPE_BOOLEAN,
+	 SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace},
+	/** SQL_SESSION_OPTION_VDBE_EQP */
+	{"sql_vdbe_eqp", FIELD_TYPE_BOOLEAN, SQL_VdbeEQP},
+	/** SQL_SESSION_OPTION_VDBE_LISTING */
+	{"sql_vdbe_listing", FIELD_TYPE_BOOLEAN, SQL_VdbeListing},
+	/** SQL_SESSION_OPTION_VDBE_TRACE */
+	{"sql_vdbe_trace", FIELD_TYPE_BOOLEAN, SQL_VdbeTrace},
+	/** SQL_SESSION_OPTION_WHERE_TRACE */
+	{"sql_where_trace", FIELD_TYPE_BOOLEAN, SQL_WhereTrace},
+#endif
+};
+
+int
+sql_session_setting_tuple(struct tuple_format *format, int id,
+			  struct tuple **result)
+{
+	if (id < 0 || id >= SQL_SESSION_OPTION_max) {
+		*result = NULL;
+		return 0;
+	}
+	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));
+	if (sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN) {
+		size += mp_sizeof_bool(true);
+	} else {
+		assert(id == SQL_SESSION_OPTION_DEFAULT_ENGINE);
+		engine = sql_storage_engine_strs[session->sql_default_engine];
+		size += mp_sizeof_str(strlen(engine));
+	}
+
+	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));
+	if (sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN)
+		pos = mp_encode_bool(pos, (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_max_id()
+{
+	return SQL_SESSION_OPTION_max;
+}
+
+const char *
+sql_session_setting_name(int id)
+{
+	return sql_session_opts[id].name;
+}
+
+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_OPTION_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_OPTION_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;
+}
+
+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;
+}
+
+int
+sql_session_setting_new_values(int id, const char *value)
+{
+	if (id < 0 || id >= SQL_SESSION_OPTION_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)
+		return sql_set_boolean_option(id, mp_decode_bool(&value));
+	uint32_t len;
+	const char *tmp = mp_decode_str(&value, &len);
+	const char *decoded_value = tt_cstr(tmp, len);
+	return sql_set_string_option(id, decoded_value);
+}
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 123aa2f..da1fd54 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'}]]
+  - [380, 1, '_session_settings', 'loophole', 0, {'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 46ce868..8dcb79f 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/loophole.result b/test/box/loophole.result
new file mode 100644
index 0000000..426aacf
--- /dev/null
+++ b/test/box/loophole.result
@@ -0,0 +1,264 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+--
+-- There is only one space with loophole engine, that could have
+-- index.
+--
+s = box.schema.space.create('s', {engine = 'loophole'})
+ | ---
+ | ...
+_ = s:create_index('i')
+ | ---
+ | - error: Loophole does not support create_index()
+ | ...
+s:drop()
+ | ---
+ | ...
+
+-- Check select() method.
+s = box.space._session_settings
+ | ---
+ | ...
+s:format()
+ | ---
+ | - [{'name': 'name', 'type': 'string'}, {'name': 'value', 'type': 'any'}]
+ | ...
+
+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 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)
+        end
+    end
+    return is_right
+end;
+ | ---
+ | ...
+test_run:cmd('setopt delimiter ""');
+ | ---
+ | - true
+ | ...
+
+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()
+ | ---
+ | ...
+
+-- Check get() method.
+s:get({'sql_defer_foreign_keys'})
+ | ---
+ | - ['sql_defer_foreign_keys', false]
+ | ...
+s:get({'sql_recursive_triggers'})
+ | ---
+ | - ['sql_recursive_triggers', true]
+ | ...
+s:get({'sql_reverse_unordered_selects'})
+ | ---
+ | - ['sql_reverse_unordered_selects', false]
+ | ...
+s:get({'sql_default_engine'})
+ | ---
+ | - ['sql_default_engine', 'memtx']
+ | ...
+s:get({'abcd'})
+ | ---
+ | ...
+
+-- Check pairs() method.
+t = {}
+ | ---
+ | ...
+for key, value in s:pairs() do table.insert(t, {key, value}) end
+ | ---
+ | ...
+#t == s:count()
+ | ---
+ | - true
+ | ...
+
+--
+-- Make sure, that space with loophole engine doesn't support
+-- insert(), replace() and delete() methods.
+--
+s:insert({'a', 1})
+ | ---
+ | - error: Loophole does not support replace()
+ | ...
+s:delete({'b'})
+ | ---
+ | - error: Loophole does not support delete()
+ | ...
+s:replace({'sql_defer_foreign_keys', true})
+ | ---
+ | - error: Loophole does not support replace()
+ | ...
+
+-- Check update() method.
+s:update('sql_defer_foreign_keys', {{'=', 'value', true}})
+ | ---
+ | - ['sql_defer_foreign_keys', true]
+ | ...
+s:get({'sql_defer_foreign_keys'})
+ | ---
+ | - ['sql_defer_foreign_keys', true]
+ | ...
+s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}})
+ | ---
+ | - ['sql_defer_foreign_keys', false]
+ | ...
+s:get({'sql_defer_foreign_keys'})
+ | ---
+ | - ['sql_defer_foreign_keys', false]
+ | ...
+
+s:update('a', {{'=', 2, 1}})
+ | ---
+ | ...
+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}})
+ | ---
+ | - error: Loophole does not support update of more than one field
+ | ...
+s:update('sql_defer_foreign_keys', {{}})
+ | ---
+ | - error: Unknown UPDATE operation
+ | ...
+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', true}})
+ | ---
+ | - error: Loophole does not support not '='
+ | ...
+s:update('sql_defer_foreign_keys', {{'-', 'value', true}})
+ | ---
+ | - error: Loophole does not support not '='
+ | ...
+s:update('sql_defer_foreign_keys', {{'&', 'value', true}})
+ | ---
+ | - error: Loophole does not support not '='
+ | ...
+s:update('sql_defer_foreign_keys', {{'|', 'value', true}})
+ | ---
+ | - error: Loophole does not support not '='
+ | ...
+s:update('sql_defer_foreign_keys', {{'^', 'value', true}})
+ | ---
+ | - error: Loophole does not support not '='
+ | ...
+s:update('sql_defer_foreign_keys', {{':', 'value', true}})
+ | ---
+ | - error: Loophole does not support not '='
+ | ...
+s:update('sql_defer_foreign_keys', {{'!', 'value', true}})
+ | ---
+ | - error: Loophole does not support not '='
+ | ...
+s:update('sql_defer_foreign_keys', {{'#', 'value', true}})
+ | ---
+ | - error: Loophole does not support not '='
+ | ...
+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, true}})
+ | ---
+ | - 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}})
+ | ---
+ | - 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: 'Tuple field 2 type does not match one required by operation: expected boolean'
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}})
+ | ---
+ | - error: 'Tuple field 2 type does not match one required by operation: expected boolean'
+ | ...
+s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
+ | ---
+ | - error: 'Tuple field 2 type does not match one required by operation: expected boolean'
+ | ...
diff --git a/test/box/loophole.test.lua b/test/box/loophole.test.lua
new file mode 100644
index 0000000..d19f489
--- /dev/null
+++ b/test/box/loophole.test.lua
@@ -0,0 +1,97 @@
+test_run = require('test_run').new()
+
+--
+-- There is only one space with loophole engine, that could have
+-- index.
+--
+s = box.schema.space.create('s', {engine = 'loophole'})
+_ = s:create_index('i')
+s:drop()
+
+-- Check select() method.
+s = box.space._session_settings
+s:format()
+
+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 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)
+        end
+    end
+    return is_right
+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.
+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.
+t = {}
+for key, value in s:pairs() do table.insert(t, {key, value}) end
+#t == s:count()
+
+--
+-- Make sure, that space with loophole engine doesn't support
+-- insert(), replace() and delete() methods.
+--
+s:insert({'a', 1})
+s:delete({'b'})
+s:replace({'sql_defer_foreign_keys', true})
+
+-- Check update() method.
+s:update('sql_defer_foreign_keys', {{'=', 'value', true}})
+s:get({'sql_defer_foreign_keys'})
+s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}})
+s:get({'sql_defer_foreign_keys'})
+
+s:update('a', {{'=', 2, 1}})
+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', true}})
+s:update('sql_defer_foreign_keys', {{'-', 'value', true}})
+s:update('sql_defer_foreign_keys', {{'&', 'value', true}})
+s:update('sql_defer_foreign_keys', {{'|', 'value', true}})
+s:update('sql_defer_foreign_keys', {{'^', 'value', true}})
+s:update('sql_defer_foreign_keys', {{':', 'value', true}})
+s:update('sql_defer_foreign_keys', {{'!', 'value', true}})
+s:update('sql_defer_foreign_keys', {{'#', 'value', true}})
+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', {{'=', 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/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
-- 
2.7.4

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Tarantool-patches] [PATCH v5 1/1] box: introduce _session_settings system space
  2019-12-09 13:23 [Tarantool-patches] [PATCH v5 1/1] box: introduce _session_settings system space imeevma
@ 2019-12-11 23:47 ` Vladislav Shpilevoy
  0 siblings, 0 replies; 2+ messages in thread
From: Vladislav Shpilevoy @ 2019-12-11 23:47 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

Hi! Thanks for the patch!

Overall it is getting better and cooler each time!

See 10 comments below!

On 09/12/2019 14:23, imeevma@tarantool.org wrote:
> This patch creates the _session_settings space. This space
> contains names and values of the session settings. User can use
> this system space to view or change 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_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_recursive_triggers', true]
>   - ['sql_reverse_unordered_selects', false]
> ...
> 
> tarantool> s:select('sql_g', {iterator='LE'})
> ---
> - - ['sql_full_column_names', false]
>   - ['sql_defer_foreign_keys', false]
>   - ['sql_default_engine', 'memtx']
> ...
> 
> -- Change session setting value.
> tarantool> s:update('sql_default_engine', {{'=', value, 'vinyl'}})

1. Looks like the value should be in quotes, if that is a field
name of the _session_settings space.

> ---
> - ['sql_default_engine', 'vinyl']
> ...
> ---
> https://github.com/tarantool/tarantool/issues/4511
> https://github.com/tarantool/tarantool/tree/imeevma/gh-4511-new-engine
> 
> diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
> index 5cd5cba8..0f94a5b 100644
> --- a/src/box/CMakeLists.txt
> +++ b/src/box/CMakeLists.txt
> @@ -78,6 +78,7 @@ add_library(box STATIC
>      memtx_space.c
>      sysview.c
>      blackhole.c
> +    loophole.c

2. I propose to name this engine 'virtual_engine', and name
the files 'virtual_engine.c/.h'.

>      vinyl.c
>      vy_stmt.c
>      vy_mem.c
> 
> diff --git a/src/box/loophole.c b/src/box/loophole.c
> new file mode 100644
> index 0000000..775f5a1
> --- /dev/null
> +++ b/src/box/loophole.c
> @@ -0,0 +1,431 @@
> +#include "loophole.h"
> +#include "session.h"
> +#include "schema.h"
> +#include "engine.h"
> +#include "tuple.h"
> +#include "xrow.h"
> +
> +struct loophole_index {
> +	struct index base;
> +	struct tuple_format *format;

3. Please, write some comments. Especially why do you
store format in the index.

> +};
> +
> +struct loophole_iterator {
> +	/** Base iterator. Must be the first member. */
> +	struct iterator base;
> +	/** Format of the tuples this iterator returns. */
> +	struct tuple_format *format;
> +	/** ID of the option in global list of the options. */
> +	int current_id;
> +	/** Decoded key. */
> +	char *key;
> +	/** Type of iterator. */
> +	enum iterator_type iterator_type;
> +};

4. I don't like that the whole API below is hardcoded to use settings.
Engine API should not be like that. For example, loophole iterator
explicitly uses 'settings' API. loophole space, index too.

Virtual engine should be generic. You already have space vtab, so
you can keep all these methods, but rename them to
settings_space_*(), create a separate vtab for that space, and in
virtual_engine_create_space() you check space ID only once. If that
is ID of settings space, you use the settings vtab.

> +
> +static void
> +loophole_iterator_free(struct iterator *ptr)
> +{
> +	struct loophole_iterator *it = (struct loophole_iterator *)ptr;
> +	free(it->key);
> +	free(it);
> +}
> +
> +static int
> +loophole_iterator_next(struct iterator *iterator, struct tuple **ret)
> +{
> +	struct loophole_iterator *it = (struct loophole_iterator *)iterator;
> +	it->current_id = session_setting_id_by_name(it->key, it->current_id,
> +						    it->iterator_type);
> +	int id = it->current_id;
> +	it->current_id += iterator_type_is_reverse(it->iterator_type) ? -1 : 1;
> +	return session_setting_tuple(it->format, id, ret);
> +}
> +
> +static void
> +loophole_index_destroy(struct index *index)
> +{
> +	free_session_settings();
> +	free(index);
> +}
> +
> +static struct iterator *
> +loophole_index_create_iterator(struct index *base, enum iterator_type type,
> +			       const char *key, uint32_t part_count)
> +{
> +	struct loophole_index *index = (struct loophole_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 loophole_iterator *it =
> +		(struct loophole_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.next = loophole_iterator_next;
> +	it->base.free = loophole_iterator_free;
> +	it->key = decoded_key;
> +	it->iterator_type = type;
> +	it->format = index->format;
> +	it->current_id = iterator_type_is_reverse(type) ?
> +			 session_get_settings_max_id() - 1 : 0;
> +	return (struct iterator *)it;
> +	return NULL;
> +}
> +
> +static int
> +loophole_index_get(struct index *base, const char *key,
> +		   uint32_t part_count, struct tuple **result)
> +{
> +	struct loophole_index *index = (struct loophole_index *)base;
> +	(void)base;

5. Why is 'base' void? It is used. Not only in debug.

> +	assert(part_count == 1);
> +	(void)part_count;
> +	uint32_t len;
> +	const char *tmp = mp_decode_str(&key, &len);
> +	const char *decoded_key = tt_cstr(tmp, len);
> +	int id = session_setting_id_by_name(decoded_key, 0, ITER_EQ);
> +	return session_setting_tuple(index->format, id, result);
> +}
> +
> +static const struct index_vtab loophole_index_vtab = {
> +	/* .destroy = */ loophole_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 = */ loophole_index_get,
> +	/* .replace = */ generic_index_replace,
> +	/* .create_iterator = */ loophole_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
> +loophole_space_destroy(struct space *space)
> +{
> +	free(space);
> +}
> +
> +static int
> +loophole_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, "Loophole", "replace()");
> +	return -1;
> +}
> +
> +static int
> +loophole_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, "Loophole", "delete()");
> +	return -1;
> +}
> +
> +static int
> +loophole_space_execute_update(struct space *space, struct txn *txn,
> +			      struct request *request, struct tuple **result)
> +{
> +	(void)txn;

6. That is actually something arguable.

tarantool> box.space._session_settings:get({'sql_default_engine'})
---
- ['sql_default_engine', 'memtx']
...

tarantool> box.begin()
---
...

tarantool> box.space._session_settings:update({'sql_default_engine'}, {{'=', 'value', 'vinyl'}})
---
- ['sql_default_engine', 'vinyl']
...

tarantool> box.rollback()
---
...

tarantool> box.space._session_settings:get({'sql_default_engine'})
---
- ['sql_default_engine', 'vinyl']
...


This is definitely wrong. And I don't know what to do with that
now. Except that it should be documented at least.

> +	uint32_t len;
> +	const char *tmp;
> +	const char *data;

7. Lets avoid C89 declaration style.

> +
> +	data = request->key;
> +	uint32_t key_len = mp_decode_array(&data);
> +	if (key_len == 0) {
> +		diag_set(ClientError, ER_EXACT_MATCH, 1, 0);
> +		return -1;
> +	}
> +	if (key_len > 1 || mp_typeof(*data) != MP_STR) {
> +		diag_set(ClientError, ER_KEY_PART_TYPE, 0, "string");
> +		return -1;
> +	}
> +	tmp = mp_decode_str(&data, &len);
> +	const char *decoded_key = tt_cstr(tmp, len);
> +	int id = session_setting_id_by_name(decoded_key, 0, ITER_EQ);
> +	if (id == session_get_settings_max_id()) {
> +		*result = NULL;
> +		return 0;
> +	}
> +
> +	data = request->tuple;
> +	assert(mp_typeof(*data) == MP_ARRAY);
> +	uint32_t update_counter = mp_decode_array(&data);
> +	if (mp_typeof(*data) != MP_ARRAY) {
> +		diag_set(ClientError, ER_ILLEGAL_PARAMS, "update operation "
> +			 "must be an array {op,..}");
> +		return -1;
> +	}
> +	if (update_counter > 1) {
> +		diag_set(ClientError, ER_UNSUPPORTED, "Loophole", "update of "
> +			 "more than one field");
> +		return -1;
> +	}
> +	if (mp_decode_array(&data) != 3) {
> +		diag_set(ClientError, ER_UNKNOWN_UPDATE_OP);
> +		return -1;
> +	}
> +	if (mp_typeof(*data) != MP_STR) {
> +		diag_set(ClientError, ER_ILLEGAL_PARAMS, "update operation "
> +			 "name must be a string");
> +		return -1;
> +	}
> +	tmp = mp_decode_str(&data, &len);
> +	if (tmp[0] != '=' || len != 1) {
> +		diag_set(ClientError, ER_UNSUPPORTED, "Loophole", "not '='");
> +		return -1;
> +	}
> +	if (mp_typeof(*data) == MP_UINT) {
> +		uint32_t id = mp_decode_uint(&data);
> +		if (id - request->index_base == 0) {
> +			diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
> +				 space_index(space, 0)->def->name,
> +				 space_name(space));
> +			return -1;
> +		}
> +		if (id - request->index_base != 1) {
> +			diag_set(ClientError, ER_EXACT_FIELD_COUNT, id,
> +				 1 + request->index_base);
> +			return -1;
> +		}
> +	} else if (mp_typeof(*data) == MP_STR) {
> +		tmp = mp_decode_str(&data, &len);
> +		const char *field_name = tt_cstr(tmp, len);
> +		if (strcmp(field_name, "name") == 0) {
> +			diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
> +				 space_index(space, 0)->def->name,
> +				 space_name(space));
> +			return -1;
> +		}
> +		if (strcmp(field_name, "value") != 0) {
> +			diag_set(ClientError, ER_NO_SUCH_FIELD_NAME,
> +				 field_name);
> +			return -1;
> +		}
> +	} else {
> +		diag_set(ClientError, ER_ILLEGAL_PARAMS,
> +			 "field id must be a number or a string");
> +		return -1;
> +	}
> +	const char *value = data;
> +
> +	if (session_setting_new_value(id, value) != 0)
> +		return -1;
> +	return session_setting_tuple(space->format, id, result);

8. Well, most part of that function could be replaced by:

    struct tuple *old_tuple;
    uint32_t new_size = 0, bsize;
    struct tuple_format *format = space->format;
    const char *old_data = tuple_data_range(old_tuple, &bsize);
    session_setting_tuple(format, id, old_tuple);
    const char *new_tuple_data = xrow_update_execute(
            request->tuple, request->tuple_end,
            old_data, old_data + bsize, format->dict,
            &new_size, request->index_base, NULL);

And you have the updated tuple in new_tuple_data. Now you
create a new tuple:

    struct tuple *new_tuple = tuple_new(format, new_tuple_data, new_size);

From that tuple you get the new value. And you don't need to
implement the update by yourself. That also makes possible to
use any update operations.

Please, patch schema_def.h with definition of this new space
fields. So as you could obtain the new value by:

    const char *new_value = tuple_field(
            new_tuple, BOX_SESSION_SETTINGS_FIELD_VALUE);

> +}
> +
> +static int
> +loophole_space_execute_upsert(struct space *space, struct txn *txn,
> +			      struct request *request)
> +{
> +	(void)space;
> +	(void)txn;
> +	(void)request;
> +	diag_set(ClientError, ER_UNSUPPORTED, "Loophole", "upsert()");
> +	return -1;
> +}
> +
> +static struct index *
> +loophole_space_create_index(struct space *space, struct index_def *def)
> +{
> +	if (space->def->id != BOX_SESSION_SETTINGS_ID) {
> +		diag_set(ClientError, ER_UNSUPPORTED, "Loophole",
> +			 "create_index()");
> +		return NULL;
> +	}
> +	new_session_settings();

9. That is something I really don't like. See more in the comments
to session setting API.

> diff --git a/src/box/session.cc b/src/box/session.cc
> index 461d1cf..d72d917 100644
> --- a/src/box/session.cc
> +++ b/src/box/session.cc
> @@ -36,6 +36,7 @@
>  #include "user.h"
>  #include "error.h"
>  #include "tt_static.h"
> +#include "sql.h"

10. This is where my idea was taken wrong. I am trying to
remove explicit session's dependency on SQL. This is possible
in at least two ways:

- Way 1. All the settings are defined in one namespace.
  In one enum. That enum is defined in session.h.
  Otherwise, when you will add more settings from other
  subsystems, you will get a collision.
  In session.c you have an array of triplets
  {enum setting, get_function, set_function}. The functions
  are all NULL from the beginning. Subsystems, when they
  initialize, fill their part in that array (sql_init()
  will fill get/set_function values of its SQL settings).
  That way may be bad, because session will need to know
  names of all settings of all subsystems.

- Way 2. Make session setting consist of 2 parts:
  subsystem ID and setting ID. Then you define subsystem
  IDs in session.h, and settings of each subsystem in
  any other file. In build.c, in sql.c, whatever. For
  example, you store this in session.h:

      enum session_setting_module {
              SESSION_SETTING_BOX,
              SESSION_SETTING_SQL,
              ...
      };

  And keep your current SQL settings in build.c.

  In session.c you have an array with triplets
  {enum setting_module, set_function, get_function}.
  Get/set functions are filled exactly like in the first
  way - during module initializations.


We actually already do something similar with
session_vtab_registry global array.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-12-11 23:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-09 13:23 [Tarantool-patches] [PATCH v5 1/1] box: introduce _session_settings system space imeevma
2019-12-11 23:47 ` Vladislav Shpilevoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox