* [Tarantool-patches] [PATCH v5 1/3] box: introduce 'virtual' engine
2019-12-27 14:05 [Tarantool-patches] [PATCH v5 0/3] Introduce _session_setting system space imeevma
@ 2019-12-27 14:05 ` imeevma
2019-12-27 21:55 ` Nikita Pettik
2019-12-27 14:05 ` [Tarantool-patches] [PATCH v5 2/3] box: introduce _session_settings system space imeevma
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: imeevma @ 2019-12-27 14:05 UTC (permalink / raw)
To: korablev; +Cc: tarantool-patches
This patch introduces a new engine called "virtual" that will be
used to create a new system space.
Part of #4511
---
src/box/CMakeLists.txt | 1 +
src/box/box.cc | 4 +
src/box/virtual_engine.c | 95 ++++++++++++++++++++++
src/box/virtual_engine.h | 53 ++++++++++++
...h-4511-access-settings-from-any-frontend.result | 10 +++
...4511-access-settings-from-any-frontend.test.lua | 4 +
6 files changed, 167 insertions(+)
create mode 100644 src/box/virtual_engine.c
create mode 100644 src/box/virtual_engine.h
create mode 100644 test/box/gh-4511-access-settings-from-any-frontend.result
create mode 100644 test/box/gh-4511-access-settings-from-any-frontend.test.lua
diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index fc9d1a3..d79d52c 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -79,6 +79,7 @@ add_library(box STATIC
memtx_space.c
sysview.c
blackhole.c
+ virtual_engine.c
vinyl.c
vy_stmt.c
vy_mem.c
diff --git a/src/box/box.cc b/src/box/box.cc
index b119c92..a19151c 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 "virtual_engine.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 *virtual_engine = virtual_engine_new_xc();
+ engine_register(virtual_engine);
+
struct engine *blackhole = blackhole_engine_new_xc();
engine_register(blackhole);
diff --git a/src/box/virtual_engine.c b/src/box/virtual_engine.c
new file mode 100644
index 0000000..9a59a3f
--- /dev/null
+++ b/src/box/virtual_engine.c
@@ -0,0 +1,95 @@
+/*
+ * 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 "virtual_engine.h"
+#include "schema.h"
+
+static void
+virtual_engine_shutdown(struct engine *engine)
+{
+ free(engine);
+}
+
+static struct space *
+virtual_engine_create_space(struct engine *engine, struct space_def *def,
+ struct rlist *key_list)
+{
+ (void)engine;
+ (void)def;
+ (void)key_list;
+ /* There are currently no spaces with this engine. */
+ diag_set(ClientError, ER_UNSUPPORTED, "Tarantool",
+ "spaces with this engine.");
+ return NULL;
+}
+
+static const struct engine_vtab virtual_engine_vtab = {
+ /* .shutdown = */ virtual_engine_shutdown,
+ /* .create_space = */ virtual_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 *
+virtual_engine_new(void)
+{
+ struct engine *virtual_engine = calloc(1, sizeof(*virtual_engine));
+ if (virtual_engine == NULL) {
+ diag_set(OutOfMemory, sizeof(*virtual_engine), "calloc",
+ "virtual_engine");
+ return NULL;
+ }
+
+ virtual_engine->vtab = &virtual_engine_vtab;
+ virtual_engine->name = "virtual";
+ virtual_engine->flags = ENGINE_BYPASS_TX;
+ return virtual_engine;
+}
diff --git a/src/box/virtual_engine.h b/src/box/virtual_engine.h
new file mode 100644
index 0000000..9cb5f52
--- /dev/null
+++ b/src/box/virtual_engine.h
@@ -0,0 +1,53 @@
+#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.
+ */
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+struct engine *
+virtual_engine_new(void);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+
+#include "diag.h"
+
+static inline struct engine *
+virtual_engine_new_xc(void)
+{
+ struct engine *virtual_engine = virtual_engine_new();
+ if (virtual_engine == NULL)
+ diag_raise();
+ return virtual_engine;
+}
+
+#endif /* defined(__plusplus) */
diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result
new file mode 100644
index 0000000..9874616
--- /dev/null
+++ b/test/box/gh-4511-access-settings-from-any-frontend.result
@@ -0,0 +1,10 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+-- User cannot create spaces with this engine.
+s = box.schema.space.create('test', {engine = 'virtual'})
+ | ---
+ | - error: Tarantool does not support spaces with this engine.
+ | ...
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
new file mode 100644
index 0000000..611caef
--- /dev/null
+++ b/test/box/gh-4511-access-settings-from-any-frontend.test.lua
@@ -0,0 +1,4 @@
+test_run = require('test_run').new()
+
+-- User cannot create spaces with this engine.
+s = box.schema.space.create('test', {engine = 'virtual'})
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v5 1/3] box: introduce 'virtual' engine
2019-12-27 14:05 ` [Tarantool-patches] [PATCH v5 1/3] box: introduce 'virtual' engine imeevma
@ 2019-12-27 21:55 ` Nikita Pettik
2019-12-28 11:35 ` Alexander Turenko
2019-12-29 15:43 ` Mergen Imeev
0 siblings, 2 replies; 15+ messages in thread
From: Nikita Pettik @ 2019-12-27 21:55 UTC (permalink / raw)
To: imeevma; +Cc: tarantool-patches
On 27 Dec 17:05, imeevma@tarantool.org wrote:
> This patch introduces a new engine called "virtual" that will be
> used to create a new system space.
Such a descriptive commit message...Please, add either rfc or extend
commit message to get others a change to dive into your patch:
why do you need that engine, why alternatives are not suitable and so
on and so forth. It's not only about this patch, but the next one
as well.
As an alternative to 'virtual' name I can suggest 'service', 'system'
(not so good since we already have 'sysview'), 'sysspace' (similar to
'sysview').
> +static struct space *
> +virtual_engine_create_space(struct engine *engine, struct space_def *def,
> + struct rlist *key_list)
> +{
> + (void)engine;
> + (void)def;
> + (void)key_list;
> + /* There are currently no spaces with this engine. */
> + diag_set(ClientError, ER_UNSUPPORTED, "Tarantool",
> + "spaces with this engine.");
-> with 'virtual' engine.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v5 1/3] box: introduce 'virtual' engine
2019-12-27 21:55 ` Nikita Pettik
@ 2019-12-28 11:35 ` Alexander Turenko
2019-12-29 15:43 ` Mergen Imeev
1 sibling, 0 replies; 15+ messages in thread
From: Alexander Turenko @ 2019-12-28 11:35 UTC (permalink / raw)
To: Nikita Pettik; +Cc: tarantool-patches
> As an alternative to 'virtual' name I can suggest 'service', 'system'
> (not so good since we already have 'sysview'), 'sysspace' (similar to
> 'sysview').
More comes into my mind: 'interface', 'assessor', 'computational',
'evaluable', 'trackdown', 'derived', 'passthrough', 'gateway'.
'gateway' looks better then all others for me.
WBR, Alexander Turenko.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 1/3] box: introduce 'virtual' engine
2019-12-27 21:55 ` Nikita Pettik
2019-12-28 11:35 ` Alexander Turenko
@ 2019-12-29 15:43 ` Mergen Imeev
1 sibling, 0 replies; 15+ messages in thread
From: Mergen Imeev @ 2019-12-29 15:43 UTC (permalink / raw)
To: Nikita Pettik; +Cc: tarantool-patches
Hi! Thank you for review. I extended commit-messages a bit
and re-send a patch-set. Also, I renamed new engine from
'virtual' to 'service'.
On Fri, Dec 27, 2019 at 11:55:18PM +0200, Nikita Pettik wrote:
> On 27 Dec 17:05, imeevma@tarantool.org wrote:
> > This patch introduces a new engine called "virtual" that will be
> > used to create a new system space.
>
> Such a descriptive commit message...Please, add either rfc or extend
> commit message to get others a change to dive into your patch:
> why do you need that engine, why alternatives are not suitable and so
> on and so forth. It's not only about this patch, but the next one
> as well.
>
> As an alternative to 'virtual' name I can suggest 'service', 'system'
> (not so good since we already have 'sysview'), 'sysspace' (similar to
> 'sysview').
>
> > +static struct space *
> > +virtual_engine_create_space(struct engine *engine, struct space_def *def,
> > + struct rlist *key_list)
> > +{
> > + (void)engine;
> > + (void)def;
> > + (void)key_list;
> > + /* There are currently no spaces with this engine. */
> > + diag_set(ClientError, ER_UNSUPPORTED, "Tarantool",
> > + "spaces with this engine.");
>
> -> with 'virtual' engine.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Tarantool-patches] [PATCH v5 2/3] box: introduce _session_settings system space
2019-12-27 14:05 [Tarantool-patches] [PATCH v5 0/3] Introduce _session_setting system space imeevma
2019-12-27 14:05 ` [Tarantool-patches] [PATCH v5 1/3] box: introduce 'virtual' engine imeevma
@ 2019-12-27 14:05 ` imeevma
2019-12-27 14:05 ` [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings imeevma
2019-12-27 14:55 ` [Tarantool-patches] [PATCH v5 0/3] Introduce _session_setting system space Vladislav Shpilevoy
3 siblings, 0 replies; 15+ messages in thread
From: imeevma @ 2019-12-27 14:05 UTC (permalink / raw)
To: korablev; +Cc: tarantool-patches
This patch creates _session_settings system space. This space is
used to view and change session settings. There are no settings at
the moment, some will be added in the next patch.
Part of #4511
---
src/box/CMakeLists.txt | 1 +
src/box/bootstrap.snap | Bin 5921 -> 5975 bytes
src/box/lua/space.cc | 2 +
src/box/lua/upgrade.lua | 15 +
src/box/schema_def.h | 8 +
src/box/session_settings.c | 472 +++++++++++++++++++++
src/box/session_settings.h | 69 +++
src/box/virtual_engine.c | 54 ++-
test/app-tap/tarantoolctl.test.lua | 4 +-
test/box-py/bootstrap.result | 3 +
test/box/access_sysview.result | 6 +-
test/box/alter.result | 5 +-
...h-4511-access-settings-from-any-frontend.result | 108 ++++-
...4511-access-settings-from-any-frontend.test.lua | 37 ++
test/wal_off/alter.result | 2 +-
15 files changed, 770 insertions(+), 16 deletions(-)
create mode 100644 src/box/session_settings.c
create mode 100644 src/box/session_settings.h
diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index d79d52c..b57e90b 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -80,6 +80,7 @@ add_library(box STATIC
sysview.c
blackhole.c
virtual_engine.c
+ session_settings.c
vinyl.c
vy_stmt.c
vy_mem.c
diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index f6e96f0..01b58af 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -653,6 +653,8 @@ box_lua_space_init(struct lua_State *L)
lua_setfield(L, -2, "SPACE_SEQUENCE_ID");
lua_pushnumber(L, BOX_FUNC_INDEX_ID);
lua_setfield(L, -2, "FUNC_INDEX_ID");
+ lua_pushnumber(L, BOX_SESSION_SETTINGS_ID);
+ lua_setfield(L, -2, "SESSION_SETTINGS_ID");
lua_pushnumber(L, BOX_SYSTEM_ID_MIN);
lua_setfield(L, -2, "SYSTEM_ID_MIN");
lua_pushnumber(L, BOX_SYSTEM_ID_MAX);
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 07f1e03..4dfd571 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -951,8 +951,23 @@ local function drop_func_collation()
_func.index.name:alter({parts = {{'name', 'string'}}})
end
+local function create_session_settings_space()
+ local _space = box.space[box.schema.SPACE_ID]
+ local _index = box.space[box.schema.INDEX_ID]
+ local format = {}
+ format[1] = {name='name', type='string'}
+ format[2] = {name='value', type='any'}
+ log.info("create space _session_settings")
+ _space:insert{box.schema.SESSION_SETTINGS_ID, ADMIN, '_session_settings',
+ 'virtual', 2, {temporary = true}, format}
+ log.info("create index _session_settings:primary")
+ _index:insert{box.schema.SESSION_SETTINGS_ID, 0, 'primary', 'tree',
+ {unique = true}, {{0, 'string'}}}
+end
+
local function upgrade_to_2_3_1()
drop_func_collation()
+ create_session_settings_space()
end
--------------------------------------------------------------------------------
diff --git a/src/box/schema_def.h b/src/box/schema_def.h
index ba870ff..f86cd42 100644
--- a/src/box/schema_def.h
+++ b/src/box/schema_def.h
@@ -114,6 +114,8 @@ enum {
BOX_CK_CONSTRAINT_ID = 364,
/** Space id of _func_index. */
BOX_FUNC_INDEX_ID = 372,
+ /** Space id of _session_settings. */
+ BOX_SESSION_SETTINGS_ID = 380,
/** End of the reserved range of system spaces. */
BOX_SYSTEM_ID_MAX = 511,
BOX_ID_NIL = 2147483647
@@ -277,6 +279,12 @@ enum {
BOX_FUNC_INDEX_FUNCTION_ID = 2,
};
+/** _session_settings fields. */
+enum {
+ BOX_SESSION_SETTINGS_FIELD_NAME = 0,
+ BOX_SESSION_SETTINGS_FIELD_VALUE = 1,
+};
+
/*
* Different objects which can be subject to access
* control.
diff --git a/src/box/session_settings.c b/src/box/session_settings.c
new file mode 100644
index 0000000..dd1874b
--- /dev/null
+++ b/src/box/session_settings.c
@@ -0,0 +1,472 @@
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <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 "session_settings.h"
+#include "xrow_update.h"
+#include "virtual_engine.h"
+#include "column_mask.h"
+#include "session.h"
+#include "schema.h"
+#include "tuple.h"
+#include "xrow.h"
+#include "sql.h"
+
+struct session_setting_module
+ session_setting_modules[session_setting_type_MAX] = {};
+
+struct session_settings_index {
+ /** Base index. Must be the first member. */
+ struct index base;
+ /**
+ * Format of the tuples iterators of this index return. It
+ * is stored here so as not to lookup space each time to
+ * get a format and create an iterator.
+ */
+ struct tuple_format *format;
+};
+
+struct session_settings_iterator {
+ /** Base iterator. Must be the first member. */
+ struct iterator base;
+ /**
+ * Format of the tuples this iterator returns. It is
+ * stored here so as not to lookup space each time to get
+ * a format for selected tuples.
+ */
+ struct tuple_format *format;
+ /**
+ * ID of the current session settings module in the global
+ * list of the modules.
+ */
+ int module_id;
+ /** ID of the setting in current module. */
+ int setting_id;
+ /** Decoded key. */
+ char *key;
+ /** True if the iterator returns only equal keys. */
+ bool is_eq;
+ /** True if the iterator should include equal keys. */
+ bool is_including;
+};
+
+static void
+session_settings_iterator_free(struct iterator *ptr)
+{
+ struct session_settings_iterator *it =
+ (struct session_settings_iterator *)ptr;
+ free(it->key);
+ free(it);
+}
+
+static int
+session_settings_next_in_module(const struct session_setting_module *module,
+ int *sid, const char *key, bool is_eq,
+ bool is_including)
+{
+ int i = *sid;
+ int count = module->setting_count;
+ if (i >= count)
+ return -1;
+ if (key == NULL)
+ return 0;
+ assert(i >= 0);
+ const char **name = &module->settings[i];
+ for (; i < count; ++i, ++name) {
+ int cmp = strcmp(*name, key);
+ if ((cmp == 0 && is_including) ||
+ (cmp > 0 && !is_eq)) {
+ *sid = i;
+ return 0;
+ }
+ }
+ *sid = count;
+ return -1;
+}
+
+static int
+session_settings_prev_in_module(const struct session_setting_module *module,
+ int *sid, const char *key, bool is_eq,
+ bool is_including)
+{
+ int i = *sid;
+ int count = module->setting_count;
+ if (i < 0)
+ return -1;
+ if (key == NULL)
+ return 0;
+ if (i >= count)
+ i = count - 1;
+ const char **name = &module->settings[i];
+ for (; i >= 0; --i, --name) {
+ int cmp = strcmp(*name, key);
+ if ((cmp == 0 && is_including) ||
+ (cmp < 0 && !is_eq)) {
+ *sid = i;
+ return 0;
+ }
+ }
+ *sid = -1;
+ return -1;
+}
+
+static int
+session_settings_iterator_next(struct iterator *iterator, struct tuple **result)
+{
+ struct session_settings_iterator *it =
+ (struct session_settings_iterator *)iterator;
+ int mid = it->module_id, sid = it->setting_id;
+ struct session_setting_module *module;
+ const char *key = it->key;
+ bool is_including = it->is_including, is_eq = it->is_eq;
+ bool is_found = false;
+ for (; mid < session_setting_type_MAX; ++mid, sid = 0) {
+ module = &session_setting_modules[mid];
+ if (session_settings_next_in_module(module, &sid, key, is_eq,
+ is_including) == 0) {
+ is_found = true;
+ break;
+ }
+ }
+ it->module_id = mid;
+ it->setting_id = sid + 1;
+ if (!is_found) {
+ *result = NULL;
+ return 0;
+ }
+ const char *mp_pair, *mp_pair_end;
+ module->get(sid, &mp_pair, &mp_pair_end);
+ *result = box_tuple_new(it->format, mp_pair, mp_pair_end);
+ return *result != NULL ? 0 : -1;
+}
+
+static int
+session_settings_iterator_prev(struct iterator *iterator, struct tuple **result)
+{
+ struct session_settings_iterator *it =
+ (struct session_settings_iterator *)iterator;
+ int mid = it->module_id, sid = it->setting_id;
+ struct session_setting_module *module;
+ const char *key = it->key;
+ bool is_including = it->is_including, is_eq = it->is_eq;
+ bool is_found = false;
+ for (; mid >= 0; --mid, sid = INT_MAX) {
+ module = &session_setting_modules[mid];
+ if (session_settings_prev_in_module(module, &sid, key, is_eq,
+ is_including) == 0) {
+ is_found = true;
+ break;
+ }
+ }
+ it->module_id = mid;
+ it->setting_id = sid - 1;
+ if (!is_found) {
+ *result = NULL;
+ return 0;
+ }
+ const char *mp_pair, *mp_pair_end;
+ module->get(sid, &mp_pair, &mp_pair_end);
+ *result = box_tuple_new(it->format, mp_pair, mp_pair_end);
+ return *result != NULL ? 0 : -1;
+}
+
+static void
+session_settings_index_destroy(struct index *index)
+{
+ free(index);
+}
+
+static struct iterator *
+session_settings_index_create_iterator(struct index *base,
+ enum iterator_type type, const char *key,
+ uint32_t part_count)
+{
+ struct session_settings_index *index =
+ (struct session_settings_index *)base;
+ char *decoded_key = NULL;
+ if (part_count > 0) {
+ assert(part_count == 1);
+ assert(mp_typeof(*key) == MP_STR);
+ uint32_t len;
+ const char *name = mp_decode_str(&key, &len);
+ decoded_key = (char *)malloc(len + 1);
+ if (decoded_key == NULL) {
+ diag_set(OutOfMemory, len + 1, "malloc", "decoded_key");
+ return NULL;
+ }
+ memcpy(decoded_key, name, len);
+ decoded_key[len] = '\0';
+ }
+ struct session_settings_iterator *it =
+ (struct session_settings_iterator *)malloc(sizeof(*it));
+ if (it == NULL) {
+ diag_set(OutOfMemory, sizeof(*it), "malloc", "it");
+ free(decoded_key);
+ return NULL;
+ }
+ iterator_create(&it->base, base);
+ it->base.free = session_settings_iterator_free;
+ it->key = decoded_key;
+ it->is_eq = type == ITER_EQ || type == ITER_REQ;
+ it->is_including = it->is_eq || type == ITER_GE || type == ITER_ALL ||
+ type == ITER_LE;
+ it->format = index->format;
+ if (!iterator_type_is_reverse(type)) {
+ it->base.next = session_settings_iterator_next;
+ it->module_id = 0;
+ it->setting_id = 0;
+ } else {
+ it->base.next = session_settings_iterator_prev;
+ it->module_id = session_setting_type_MAX - 1;
+ struct session_setting_module *module =
+ &session_setting_modules[it->module_id];
+ it->setting_id = module->setting_count - 1;
+ }
+ return (struct iterator *)it;
+}
+
+static int
+session_settings_index_get(struct index *base, const char *key,
+ uint32_t part_count, struct tuple **result)
+{
+ struct session_settings_index *index =
+ (struct session_settings_index *) base;
+ assert(part_count == 1);
+ (void) part_count;
+ uint32_t len;
+ key = mp_decode_str(&key, &len);
+ key = tt_cstr(key, len);
+ struct session_setting_module *module = &session_setting_modules[0];
+ struct session_setting_module *end = module + session_setting_type_MAX;
+ int sid = 0;
+ for (; module < end; ++module, sid = 0) {
+ if (session_settings_next_in_module(module, &sid, key, true,
+ true) == 0)
+ goto found;
+ }
+ *result = NULL;
+ return 0;
+found:;
+ const char *mp_pair;
+ const char *mp_pair_end;
+ module->get(sid, &mp_pair, &mp_pair_end);
+ *result = box_tuple_new(index->format, mp_pair, mp_pair_end);
+ return *result != NULL ? 0 : -1;
+}
+
+static const struct index_vtab session_settings_index_vtab = {
+ /* .destroy = */ session_settings_index_destroy,
+ /* .commit_create = */ generic_index_commit_create,
+ /* .abort_create = */ generic_index_abort_create,
+ /* .commit_modify = */ generic_index_commit_modify,
+ /* .commit_drop = */ generic_index_commit_drop,
+ /* .update_def = */ generic_index_update_def,
+ /* .depends_on_pk = */ generic_index_depends_on_pk,
+ /* .def_change_requires_rebuild = */
+ generic_index_def_change_requires_rebuild,
+ /* .size = */ generic_index_size,
+ /* .bsize = */ generic_index_bsize,
+ /* .min = */ generic_index_min,
+ /* .max = */ generic_index_max,
+ /* .random = */ generic_index_random,
+ /* .count = */ generic_index_count,
+ /* .get = */ session_settings_index_get,
+ /* .replace = */ generic_index_replace,
+ /* .create_iterator = */ session_settings_index_create_iterator,
+ /* .create_snapshot_iterator = */
+ generic_index_create_snapshot_iterator,
+ /* .stat = */ generic_index_stat,
+ /* .compact = */ generic_index_compact,
+ /* .reset_stat = */ generic_index_reset_stat,
+ /* .begin_build = */ generic_index_begin_build,
+ /* .reserve = */ generic_index_reserve,
+ /* .build_next = */ generic_index_build_next,
+ /* .end_build = */ generic_index_end_build,
+};
+
+static void
+session_settings_space_destroy(struct space *space)
+{
+ free(space);
+}
+
+static int
+session_settings_space_execute_replace(struct space *space, struct txn *txn,
+ struct request *request,
+ struct tuple **result)
+{
+ (void)space;
+ (void)txn;
+ (void)request;
+ (void)result;
+ diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ "replace()");
+ return -1;
+}
+
+static int
+session_settings_space_execute_delete(struct space *space, struct txn *txn,
+ struct request *request,
+ struct tuple **result)
+{
+ (void)space;
+ (void)txn;
+ (void)request;
+ (void)result;
+ diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ "delete()");
+ return -1;
+}
+
+static int
+session_settings_space_execute_update(struct space *space, struct txn *txn,
+ struct request *request,
+ struct tuple **result)
+{
+ (void)txn;
+ struct tuple_format *format = space->format;
+ const char *old_data, *old_data_end, *new_data;
+ struct region *region = &fiber()->gc;
+ size_t used = region_used(region);
+ int rc = -1, sid = 0;
+ struct index_def *pk_def = space->index[0]->def;
+ uint64_t column_mask;
+
+ const char *new_key, *key = request->key;
+ uint32_t new_size, new_key_len, key_len = mp_decode_array(&key);
+ if (key_len == 0) {
+ diag_set(ClientError, ER_EXACT_MATCH, 1, 0);
+ return -1;
+ }
+ if (key_len > 1 || mp_typeof(*key) != MP_STR) {
+ diag_set(ClientError, ER_KEY_PART_TYPE, 0, "string");
+ return -1;
+ }
+ key = mp_decode_str(&key, &key_len);
+ key = tt_cstr(key, key_len);
+ struct session_setting_module *module = &session_setting_modules[0];
+ struct session_setting_module *end = module + session_setting_type_MAX;
+ for (; module < end; ++module, sid = 0) {
+ if (session_settings_next_in_module(module, &sid, key, true,
+ true) == 0)
+ goto found;
+ }
+ *result = NULL;
+ return 0;
+found:
+ module->get(sid, &old_data, &old_data_end);
+ new_data = xrow_update_execute(request->tuple, request->tuple_end,
+ old_data, old_data_end, format->dict,
+ &new_size, request->index_base,
+ &column_mask);
+ if (new_data == NULL)
+ goto finish;
+ *result = box_tuple_new(format, new_data, new_data + new_size);
+ if (*result == NULL)
+ goto finish;
+
+ mp_decode_array(&new_data);
+ new_key = mp_decode_str(&new_data, &new_key_len);
+ if (!key_update_can_be_skipped(pk_def->key_def->column_mask,
+ column_mask)) {
+ if (key_len != new_key_len ||
+ memcmp(key, new_key, key_len) != 0) {
+ diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
+ pk_def->name, space_name(space));
+ goto finish;
+ }
+ }
+ if (module->set(sid, new_data) != 0)
+ goto finish;
+ rc = 0;
+finish:
+ region_truncate(region, used);
+ return rc;
+}
+
+static int
+session_settings_space_execute_upsert(struct space *space, struct txn *txn,
+ struct request *request)
+{
+ (void)space;
+ (void)txn;
+ (void)request;
+ diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ "upsert()");
+ return -1;
+}
+
+static struct index *
+session_settings_space_create_index(struct space *space, struct index_def *def)
+{
+ assert(space->def->id == BOX_SESSION_SETTINGS_ID);
+ if (def->iid != 0) {
+ diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ "create_index()");
+ return NULL;
+ }
+
+ struct session_settings_index *index =
+ (struct session_settings_index *)calloc(1, sizeof(*index));
+ if (index == NULL) {
+ diag_set(OutOfMemory, sizeof(*index), "calloc", "index");
+ return NULL;
+ }
+ if (index_create(&index->base, space->engine,
+ &session_settings_index_vtab, def) != 0) {
+ free(index);
+ return NULL;
+ }
+
+ index->format = space->format;
+ return &index->base;
+}
+
+const struct space_vtab session_settings_space_vtab = {
+ /* .destroy = */ session_settings_space_destroy,
+ /* .bsize = */ generic_space_bsize,
+ /* .execute_replace = */ session_settings_space_execute_replace,
+ /* .execute_delete = */ session_settings_space_execute_delete,
+ /* .execute_update = */ session_settings_space_execute_update,
+ /* .execute_upsert = */ session_settings_space_execute_upsert,
+ /* .ephemeral_replace = */ generic_space_ephemeral_replace,
+ /* .ephemeral_delete = */ generic_space_ephemeral_delete,
+ /* .ephemeral_rowid_next = */ generic_space_ephemeral_rowid_next,
+ /* .init_system_space = */ generic_init_system_space,
+ /* .init_ephemeral_space = */ generic_init_ephemeral_space,
+ /* .check_index_def = */ generic_space_check_index_def,
+ /* .create_index = */ session_settings_space_create_index,
+ /* .add_primary_key = */ generic_space_add_primary_key,
+ /* .drop_primary_key = */ generic_space_drop_primary_key,
+ /* .check_format = */ generic_space_check_format,
+ /* .build_index = */ generic_space_build_index,
+ /* .swap_index = */ generic_space_swap_index,
+ /* .prepare_alter = */ generic_space_prepare_alter,
+ /* .invalidate = */ generic_space_invalidate,
+};
diff --git a/src/box/session_settings.h b/src/box/session_settings.h
new file mode 100644
index 0000000..7415e0e
--- /dev/null
+++ b/src/box/session_settings.h
@@ -0,0 +1,69 @@
+#pragma once
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <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.
+ */
+
+/**
+ * Session has settings. Settings belong to different subsystems,
+ * such as SQL. Each subsystem registers here its session setting
+ * type and a set of settings with getter and setter functions.
+ * The self-registration of modules allows session setting code
+ * not to depend on all the subsystems.
+ *
+ * The types should be ordered in alphabetical order, because the
+ * type list is used by setting iterators.
+ */
+enum session_setting_type {
+ session_setting_type_MAX,
+};
+
+struct session_setting_module {
+ /**
+ * An array of setting names. All of them should have the
+ * same prefix.
+ */
+ const char **settings;
+ /** Count of settings. */
+ int setting_count;
+ /**
+ * Get a MessagePack encoded pair [name, value] for a
+ * setting having index @a id. Index is from the settings
+ * array.
+ */
+ void (*get)(int id, const char **mp_pair, const char **mp_pair_end);
+ /**
+ * Set value of a setting by a given @a id from a
+ * MessagePack encoded buffer. Note, it is not a pair, but
+ * just value.
+ */
+ int (*set)(int id, const char *mp_value);
+};
+
+extern struct session_setting_module session_setting_modules[];
diff --git a/src/box/virtual_engine.c b/src/box/virtual_engine.c
index 9a59a3f..36eec83 100644
--- a/src/box/virtual_engine.c
+++ b/src/box/virtual_engine.c
@@ -29,8 +29,11 @@
* SUCH DAMAGE.
*/
#include "virtual_engine.h"
+#include "tuple.h"
#include "schema.h"
+extern const struct space_vtab session_settings_space_vtab;
+
static void
virtual_engine_shutdown(struct engine *engine)
{
@@ -41,13 +44,50 @@ static struct space *
virtual_engine_create_space(struct engine *engine, struct space_def *def,
struct rlist *key_list)
{
- (void)engine;
- (void)def;
- (void)key_list;
- /* There are currently no spaces with this engine. */
- diag_set(ClientError, ER_UNSUPPORTED, "Tarantool",
- "spaces with this engine.");
- return NULL;
+ /*
+ * At the moment the only space that have this engine is
+ * _session_sessings.
+ */
+ if (def->id != BOX_SESSION_SETTINGS_ID) {
+ diag_set(ClientError, ER_UNSUPPORTED, "Tarantool",
+ "non-system space with this engine.");
+ return NULL;
+ }
+ const struct space_vtab *space_vtab = &session_settings_space_vtab;
+
+ struct space *space = (struct space *)calloc(1, sizeof(*space));
+ if (space == NULL) {
+ diag_set(OutOfMemory, sizeof(*space), "calloc", "space");
+ return NULL;
+ }
+ int key_count = 0;
+ struct key_def **keys = index_def_to_key_def(key_list, &key_count);
+ if (keys == NULL) {
+ free(space);
+ return NULL;
+ }
+ struct tuple_format *format =
+ tuple_format_new(&tuple_format_runtime->vtab, NULL, keys,
+ key_count, def->fields, def->field_count,
+ def->exact_field_count, def->dict,
+ def->opts.is_temporary,
+ def->opts.is_ephemeral);
+ if (format == NULL) {
+ free(space);
+ return NULL;
+ }
+ tuple_format_ref(format);
+ int rc = space_create(space, engine, space_vtab, def, key_list, format);
+ /*
+ * Format is now referenced by the space if space has beed
+ * created.
+ */
+ tuple_format_unref(format);
+ if (rc != 0) {
+ free(space);
+ return NULL;
+ }
+ return space;
}
static const struct engine_vtab virtual_engine_vtab = {
diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
index 7a07860..4d70595 100755
--- a/test/app-tap/tarantoolctl.test.lua
+++ b/test/app-tap/tarantoolctl.test.lua
@@ -415,8 +415,8 @@ do
check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1", "\n", 3)
check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1 --replica 2", "\n", 3)
check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 2", "\n", 0)
- check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 24)
- check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 52)
+ check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 25)
+ check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 53)
end)
end)
diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
index 938a763..f2ad75e 100644
--- a/test/box-py/bootstrap.result
+++ b/test/box-py/bootstrap.result
@@ -96,6 +96,8 @@ box.space._space:select{}
'type': 'boolean'}]]
- [372, 1, '_func_index', 'memtx', 0, {}, [{'name': 'space_id', 'type': 'unsigned'},
{'name': 'index_id', 'type': 'unsigned'}, {'name': 'func_id', 'type': 'unsigned'}]]
+ - [380, 1, '_session_settings', 'virtual', 2, {'temporary': true}, [{'name': 'name',
+ 'type': 'string'}, {'name': 'value', 'type': 'any'}]]
...
box.space._index:select{}
---
@@ -153,6 +155,7 @@ box.space._index:select{}
- [364, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'string']]]
- [372, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]]
- [372, 1, 'fid', 'tree', {'unique': false}, [[2, 'unsigned']]]
+ - [380, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]]
...
box.space._user:select{}
---
diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result
index 1f33dec..799d19f 100644
--- a/test/box/access_sysview.result
+++ b/test/box/access_sysview.result
@@ -246,11 +246,11 @@ box.session.su('guest')
...
#box.space._vspace:select{}
---
-- 25
+- 26
...
#box.space._vindex:select{}
---
-- 53
+- 54
...
#box.space._vuser:select{}
---
@@ -282,7 +282,7 @@ box.session.su('guest')
...
#box.space._vindex:select{}
---
-- 53
+- 54
...
#box.space._vuser:select{}
---
diff --git a/test/box/alter.result b/test/box/alter.result
index 9a2f991..f150faa 100644
--- a/test/box/alter.result
+++ b/test/box/alter.result
@@ -92,7 +92,7 @@ space = box.space[t[1]]
...
space.id
---
-- 373
+- 381
...
space.field_count
---
@@ -137,7 +137,7 @@ space_deleted
...
space:replace{0}
---
-- error: Space '373' does not exist
+- error: Space '381' does not exist
...
_index:insert{_space.id, 0, 'primary', 'tree', {unique=true}, {{0, 'unsigned'}}}
---
@@ -220,6 +220,7 @@ _index:select{}
- [364, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'string']]]
- [372, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]]
- [372, 1, 'fid', 'tree', {'unique': false}, [[2, 'unsigned']]]
+ - [380, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]]
...
-- modify indexes of a system space
_index:delete{_index.id, 0}
diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result
index 9874616..75d53cf 100644
--- a/test/box/gh-4511-access-settings-from-any-frontend.result
+++ b/test/box/gh-4511-access-settings-from-any-frontend.result
@@ -6,5 +6,111 @@ test_run = require('test_run').new()
-- User cannot create spaces with this engine.
s = box.schema.space.create('test', {engine = 'virtual'})
| ---
- | - error: Tarantool does not support spaces with this engine.
+ | - error: Tarantool does not support non-system space with this engine.
+ | ...
+
+-- Check _session_settings space.
+s = box.space._session_settings
+ | ---
+ | ...
+s:format()
+ | ---
+ | - [{'name': 'name', 'type': 'string'}, {'name': 'value', 'type': 'any'}]
+ | ...
+
+-- Make sure that we cannot drop space.
+s:drop()
+ | ---
+ | - error: Can't drop the primary key in a system space, space '_session_settings'
+ | ...
+
+--
+-- Make sure, that session_settings space doesn't support
+-- create_index(), insert(), replace() and delete() methods.
+--
+s:create_index('a')
+ | ---
+ | - error: Session_settings space does not support create_index()
+ | ...
+s:insert({'a', 1})
+ | ---
+ | - error: Session_settings space does not support replace()
+ | ...
+s:delete({'b'})
+ | ---
+ | - error: Session_settings space does not support delete()
+ | ...
+s:replace({'sql_defer_foreign_keys', true})
+ | ---
+ | - error: Session_settings space does not support replace()
+ | ...
+
+-- Check get() and select(). They should return nothing for now.
+s:get({'a'})
+ | ---
+ | ...
+s:select()
+ | ---
+ | - []
+ | ...
+s:select({}, {iterator='EQ'})
+ | ---
+ | - []
+ | ...
+s:select({}, {iterator='ALL'})
+ | ---
+ | - []
+ | ...
+s:select({}, {iterator='GE'})
+ | ---
+ | - []
+ | ...
+s:select({}, {iterator='GT'})
+ | ---
+ | - []
+ | ...
+s:select({}, {iterator='REQ'})
+ | ---
+ | - []
+ | ...
+s:select({}, {iterator='LE'})
+ | ---
+ | - []
+ | ...
+s:select({}, {iterator='LT'})
+ | ---
+ | - []
+ | ...
+s:select({'a'}, {iterator='EQ'})
+ | ---
+ | - []
+ | ...
+s:select({'a'}, {iterator='ALL'})
+ | ---
+ | - []
+ | ...
+s:select({'a'}, {iterator='GE'})
+ | ---
+ | - []
+ | ...
+s:select({'a'}, {iterator='GT'})
+ | ---
+ | - []
+ | ...
+s:select({'a'}, {iterator='REQ'})
+ | ---
+ | - []
+ | ...
+s:select({'a'}, {iterator='LE'})
+ | ---
+ | - []
+ | ...
+s:select({'a'}, {iterator='LT'})
+ | ---
+ | - []
+ | ...
+
+-- Currently there is nothing to update, but update() should work.
+s:update('some_option', {{'=', 'value', true}})
+ | ---
| ...
diff --git a/test/box/gh-4511-access-settings-from-any-frontend.test.lua b/test/box/gh-4511-access-settings-from-any-frontend.test.lua
index 611caef..3304454 100644
--- a/test/box/gh-4511-access-settings-from-any-frontend.test.lua
+++ b/test/box/gh-4511-access-settings-from-any-frontend.test.lua
@@ -2,3 +2,40 @@ test_run = require('test_run').new()
-- User cannot create spaces with this engine.
s = box.schema.space.create('test', {engine = 'virtual'})
+
+-- Check _session_settings space.
+s = box.space._session_settings
+s:format()
+
+-- Make sure that we cannot drop space.
+s:drop()
+
+--
+-- Make sure, that session_settings space doesn't support
+-- create_index(), insert(), replace() and delete() methods.
+--
+s:create_index('a')
+s:insert({'a', 1})
+s:delete({'b'})
+s:replace({'sql_defer_foreign_keys', true})
+
+-- Check get() and select(). They should return nothing for now.
+s:get({'a'})
+s:select()
+s:select({}, {iterator='EQ'})
+s:select({}, {iterator='ALL'})
+s:select({}, {iterator='GE'})
+s:select({}, {iterator='GT'})
+s:select({}, {iterator='REQ'})
+s:select({}, {iterator='LE'})
+s:select({}, {iterator='LT'})
+s:select({'a'}, {iterator='EQ'})
+s:select({'a'}, {iterator='ALL'})
+s:select({'a'}, {iterator='GE'})
+s:select({'a'}, {iterator='GT'})
+s:select({'a'}, {iterator='REQ'})
+s:select({'a'}, {iterator='LE'})
+s:select({'a'}, {iterator='LT'})
+
+-- Currently there is nothing to update, but update() should work.
+s:update('some_option', {{'=', 'value', true}})
diff --git a/test/wal_off/alter.result b/test/wal_off/alter.result
index 62cb11d..97f7e6f 100644
--- a/test/wal_off/alter.result
+++ b/test/wal_off/alter.result
@@ -28,7 +28,7 @@ end;
...
#spaces;
---
-- 65502
+- 65501
...
-- cleanup
for k, v in pairs(spaces) do
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings
2019-12-27 14:05 [Tarantool-patches] [PATCH v5 0/3] Introduce _session_setting system space imeevma
2019-12-27 14:05 ` [Tarantool-patches] [PATCH v5 1/3] box: introduce 'virtual' engine imeevma
2019-12-27 14:05 ` [Tarantool-patches] [PATCH v5 2/3] box: introduce _session_settings system space imeevma
@ 2019-12-27 14:05 ` imeevma
2019-12-30 11:21 ` Nikita Pettik
2019-12-27 14:55 ` [Tarantool-patches] [PATCH v5 0/3] Introduce _session_setting system space Vladislav Shpilevoy
3 siblings, 1 reply; 15+ messages in thread
From: imeevma @ 2019-12-27 14:05 UTC (permalink / raw)
To: korablev; +Cc: tarantool-patches
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']
...
---
src/box/errcode.h | 1 +
src/box/session_settings.h | 1 +
src/box/sql.c | 5 +
src/box/sql/build.c | 222 ++++++++++++++++++++
...h-4511-access-settings-from-any-frontend.result | 225 +++++++++++++++++----
...4511-access-settings-from-any-frontend.test.lua | 105 ++++++++--
test/box/misc.result | 1 +
7 files changed, 506 insertions(+), 54 deletions(-)
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 ''");
---
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings
2019-12-27 14:05 ` [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings imeevma
@ 2019-12-30 11:21 ` Nikita Pettik
2019-12-30 12:38 ` Mergen Imeev
0 siblings, 1 reply; 15+ messages in thread
From: Nikita Pettik @ 2019-12-30 11:21 UTC (permalink / raw)
To: imeevma; +Cc: tarantool-patches
On 27 Dec 17:05, imeevma@tarantool.org wrote:
> 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.
Do you have instruction for developers how to insert to this space
new values and remove obsolete ones? Sooner or later we will have to
introduce new SQL settings and clean-up unused.
> 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
Is user capable of setting default values for these options?
> 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
Imho there are too many debug options. We can easily merge
half of them (where + select traces, vdbe eqp + debug).
What is more, I think different set of options on debug and
release builds is quite error prone. Mb it is worth keeping
debug options all the time, but disallow setting their values
to true?
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings
2019-12-30 11:21 ` Nikita Pettik
@ 2019-12-30 12:38 ` Mergen Imeev
2019-12-30 12:41 ` Mergen Imeev
2019-12-30 13:11 ` Nikita Pettik
0 siblings, 2 replies; 15+ messages in thread
From: Mergen Imeev @ 2019-12-30 12:38 UTC (permalink / raw)
To: Nikita Pettik; +Cc: tarantool-patches
Hi! Thank you fore review! My answers below.
On Mon, Dec 30, 2019 at 01:21:28PM +0200, Nikita Pettik wrote:
> On 27 Dec 17:05, imeevma@tarantool.org wrote:
> > 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.
>
> Do you have instruction for developers how to insert to this space
> new values and remove obsolete ones? Sooner or later we will have to
> introduce new SQL settings and clean-up unused.
>
There is no such instruction for now. I will write an
article in our wiki a bit later.
> > 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
>
> Is user capable of setting default values for these options?
>
No, at least for now. Even if user will be able to do this,
the default value of setting is a global setting. At least
until we will be able to differentiate users.
> > 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
>
> Imho there are too many debug options. We can easily merge
> half of them (where + select traces, vdbe eqp + debug).
> What is more, I think different set of options on debug and
> release builds is quite error prone. Mb it is worth keeping
> debug options all the time, but disallow setting their values
> to true?
>
It is possible, but I don’t think there will be problems,
since there are no numerical identifiers. Still, I do not
think that user should be able to see internal settings.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings
2019-12-30 12:38 ` Mergen Imeev
@ 2019-12-30 12:41 ` Mergen Imeev
2019-12-30 13:15 ` Nikita Pettik
2019-12-30 13:11 ` Nikita Pettik
1 sibling, 1 reply; 15+ messages in thread
From: Mergen Imeev @ 2019-12-30 12:41 UTC (permalink / raw)
To: Nikita Pettik; +Cc: tarantool-patches
Sorry, forgot to answer one question. The answer below.
On Mon, Dec 30, 2019 at 03:38:13PM +0300, Mergen Imeev wrote:
> Hi! Thank you fore review! My answers below.
>
> On Mon, Dec 30, 2019 at 01:21:28PM +0200, Nikita Pettik wrote:
> > On 27 Dec 17:05, imeevma@tarantool.org wrote:
> > > 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.
> >
> > Do you have instruction for developers how to insert to this space
> > new values and remove obsolete ones? Sooner or later we will have to
> > introduce new SQL settings and clean-up unused.
> >
> There is no such instruction for now. I will write an
> article in our wiki a bit later.
>
> > > 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
> >
> > Is user capable of setting default values for these options?
> >
> No, at least for now. Even if user will be able to do this,
> the default value of setting is a global setting. At least
> until we will be able to differentiate users.
>
> > > 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
> >
> > Imho there are too many debug options. We can easily merge
> > half of them (where + select traces, vdbe eqp + debug).
I agree, but I suggest to leave this to the next release.
Should I create an issue?
> > What is more, I think different set of options on debug and
> > release builds is quite error prone. Mb it is worth keeping
> > debug options all the time, but disallow setting their values
> > to true?
> >
> It is possible, but I don’t think there will be problems,
> since there are no numerical identifiers. Still, I do not
> think that user should be able to see internal settings.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings
2019-12-30 12:41 ` Mergen Imeev
@ 2019-12-30 13:15 ` Nikita Pettik
2019-12-30 16:48 ` Mergen Imeev
0 siblings, 1 reply; 15+ messages in thread
From: Nikita Pettik @ 2019-12-30 13:15 UTC (permalink / raw)
To: Mergen Imeev; +Cc: tarantool-patches
On 30 Dec 15:41, Mergen Imeev wrote:
> Sorry, forgot to answer one question. The answer below.
>
> On Mon, Dec 30, 2019 at 03:38:13PM +0300, Mergen Imeev wrote:
> > Hi! Thank you fore review! My answers below.
> >
> > On Mon, Dec 30, 2019 at 01:21:28PM +0200, Nikita Pettik wrote:
> > > On 27 Dec 17:05, imeevma@tarantool.org wrote:
> > > > Part of #4511
> > > >
> > > > Currently, this space contains only SQL settings, since the only
> > > > session settings are SQL settings.
> > > >
> > > > 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
> > >
> > > Imho there are too many debug options. We can easily merge
> > > half of them (where + select traces, vdbe eqp + debug).
> I agree, but I suggest to leave this to the next release.
> Should I create an issue?
Why can't you do it right now? Nevertheless they are dubug
options it's not okay to release settings which are fated to
be remove the next day after release.
> > > What is more, I think different set of options on debug and
> > > release builds is quite error prone. Mb it is worth keeping
> > > debug options all the time, but disallow setting their values
> > > to true?
> > >
> > It is possible, but I don’t think there will be problems,
> > since there are no numerical identifiers. Still, I do not
> > think that user should be able to see internal settings.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings
2019-12-30 13:15 ` Nikita Pettik
@ 2019-12-30 16:48 ` Mergen Imeev
0 siblings, 0 replies; 15+ messages in thread
From: Mergen Imeev @ 2019-12-30 16:48 UTC (permalink / raw)
To: Nikita Pettik; +Cc: tarantool-patches
Hi! Thank you for review. My answers and diff below. I send
a new patch in a different trend.
On Mon, Dec 30, 2019 at 03:15:18PM +0200, Nikita Pettik wrote:
> On 30 Dec 15:41, Mergen Imeev wrote:
> > Sorry, forgot to answer one question. The answer below.
> >
> > On Mon, Dec 30, 2019 at 03:38:13PM +0300, Mergen Imeev wrote:
> > > Hi! Thank you fore review! My answers below.
> > >
> > > On Mon, Dec 30, 2019 at 01:21:28PM +0200, Nikita Pettik wrote:
> > > > On 27 Dec 17:05, imeevma@tarantool.org wrote:
> > > > > Part of #4511
> > > > >
> > > > > Currently, this space contains only SQL settings, since the only
> > > > > session settings are SQL settings.
> > > > >
> > > > > 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
> > > >
> > > > Imho there are too many debug options. We can easily merge
> > > > half of them (where + select traces, vdbe eqp + debug).
> > I agree, but I suggest to leave this to the next release.
> > Should I create an issue?
>
> Why can't you do it right now? Nevertheless they are dubug
> options it's not okay to release settings which are fated to
> be remove the next day after release.
>
Done, now we have onle sql_vdbe_debug, sql_parser_debug
and sql_select_debug.
> > > > What is more, I think different set of options on debug and
> > > > release builds is quite error prone. Mb it is worth keeping
> > > > debug options all the time, but disallow setting their values
> > > > to true?
> > > >
> > > It is possible, but I don’t think there will be problems,
> > > since there are no numerical identifiers. Still, I do not
> > > think that user should be able to see internal settings.
Done.
Diff:
commit 97e24d0d342f287253ef3476954f3fb770d2005b
Author: Mergen Imeev <imeevma@gmail.com>
Date: Mon Dec 30 18:35:07 2019 +0300
Review fix
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 570a064..bc50ecb 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3324,21 +3324,11 @@ enum {
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_PARSER_DEBUG,
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_SELECT_DEBUG,
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,
};
@@ -3347,21 +3337,11 @@ static const char *sql_session_setting_strs[sql_session_setting_MAX] = {
"sql_defer_foreign_keys",
"sql_full_column_names",
"sql_full_metadata",
-#ifndef NDEBUG
- "sql_parser_trace",
-#endif
+ "sql_parser_debug",
"sql_recursive_triggers",
"sql_reverse_unordered_selects",
-#ifndef NDEBUG
- "sql_select_trace",
- "sql_trace",
- "sql_vdbe_addoptrace",
+ "sql_select_debug",
"sql_vdbe_debug",
- "sql_vdbe_eqp",
- "sql_vdbe_listing",
- "sql_vdbe_trace",
- "sql_where_trace",
-#endif
};
/**
@@ -3389,33 +3369,18 @@ static struct sql_option_metadata sql_session_opts[] = {
{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_PARSER_DEBUG */
+ {FIELD_TYPE_BOOLEAN, SQL_SqlTrace | PARSER_TRACE_FLAG},
/** 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_SELECT_DEBUG */
+ {FIELD_TYPE_BOOLEAN,
+ SQL_SqlTrace | SQL_SelectTrace | SQL_WhereTrace},
/** 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
@@ -3463,12 +3428,19 @@ 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);
+#ifdef NDEBUG
+ if ((session->sql_flags & SQL_SqlTrace) == 0) {
+ if (value)
+ session->sql_flags |= option->mask;
+ else
+ session->sql_flags &= ~option->mask;
+ }
+#else
if (value)
session->sql_flags |= option->mask;
else
session->sql_flags &= ~option->mask;
-#ifndef NDEBUG
- if (id == SQL_SESSION_SETTING_PARSER_TRACE) {
+ if (id == SQL_SESSION_SETTING_PARSER_DEBUG) {
if (value)
sqlParserTrace(stdout, "parser: ");
else
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 b021c29..64532a1 100644
--- a/test/box/gh-4511-access-settings-from-any-frontend.result
+++ b/test/box/gh-4511-access-settings-from-any-frontend.result
@@ -50,6 +50,19 @@ s:replace({'sql_defer_foreign_keys', true})
-- the same way as an ordinary space with an index of the type
-- "TREE".
--
+s:select()
+ | ---
+ | - - ['sql_default_engine', 'memtx']
+ | - ['sql_defer_foreign_keys', false]
+ | - ['sql_full_column_names', false]
+ | - ['sql_full_metadata', false]
+ | - ['sql_parser_debug', false]
+ | - ['sql_recursive_triggers', true]
+ | - ['sql_reverse_unordered_selects', false]
+ | - ['sql_select_debug', false]
+ | - ['sql_vdbe_debug', false]
+ | ...
+
t = box.schema.space.create('settings', {format = s:format()})
| ---
| ...
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 9049fab..3668749 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
@@ -24,6 +24,8 @@ s:replace({'sql_defer_foreign_keys', true})
-- the same way as an ordinary space with an index of the type
-- "TREE".
--
+s:select()
+
t = box.schema.space.create('settings', {format = s:format()})
_ = t:create_index('primary')
for _,value in s:pairs() do t:insert(value) end
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings
2019-12-30 12:38 ` Mergen Imeev
2019-12-30 12:41 ` Mergen Imeev
@ 2019-12-30 13:11 ` Nikita Pettik
1 sibling, 0 replies; 15+ messages in thread
From: Nikita Pettik @ 2019-12-30 13:11 UTC (permalink / raw)
To: Mergen Imeev; +Cc: tarantool-patches
On 30 Dec 15:38, Mergen Imeev wrote:
> Hi! Thank you fore review! My answers below.
>
> On Mon, Dec 30, 2019 at 01:21:28PM +0200, Nikita Pettik wrote:
> > On 27 Dec 17:05, imeevma@tarantool.org wrote:
> > > 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.
> >
> > Do you have instruction for developers how to insert to this space
> > new values and remove obsolete ones? Sooner or later we will have to
> > introduce new SQL settings and clean-up unused.
> >
> There is no such instruction for now. I will write an
> article in our wiki a bit later.
>
> > > 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
> >
> > Is user capable of setting default values for these options?
> >
> No, at least for now. Even if user will be able to do this,
Please, document this behaviour.
> the default value of setting is a global setting. At least
> until we will be able to differentiate users.
>
> > > 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
> >
> > Imho there are too many debug options. We can easily merge
> > half of them (where + select traces, vdbe eqp + debug).
> > What is more, I think different set of options on debug and
> > release builds is quite error prone. Mb it is worth keeping
> > debug options all the time, but disallow setting their values
> > to true?
> >
> It is possible, but I don’t think there will be problems,
Ok, then cover this case with tests.
> since there are no numerical identifiers.
How numeric ids are related to the question?
> Still, I do not
> think that user should be able to see internal settings.
Don't understand what you mean.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 0/3] Introduce _session_setting system space
2019-12-27 14:05 [Tarantool-patches] [PATCH v5 0/3] Introduce _session_setting system space imeevma
` (2 preceding siblings ...)
2019-12-27 14:05 ` [Tarantool-patches] [PATCH v5 3/3] box: add SQL settings to _session_settings imeevma
@ 2019-12-27 14:55 ` Vladislav Shpilevoy
3 siblings, 0 replies; 15+ messages in thread
From: Vladislav Shpilevoy @ 2019-12-27 14:55 UTC (permalink / raw)
To: imeevma, korablev; +Cc: tarantool-patches
I've filed a couple of follow-up issues:
https://github.com/tarantool/tarantool/issues/4712
https://github.com/tarantool/tarantool/issues/4711
On 27/12/2019 15:05, imeevma@tarantool.org wrote:
> This patch-set creates _session_settings system space. This space
> is used to view and change session settings.
>
> https://github.com/tarantool/tarantool/issues/4511
> https://github.com/tarantool/tarantool/tree/imeevma/gh-4511-new-engine
>
> Mergen Imeev (3):
> box: introduce 'virtual' engine
> box: introduce _session_settings system space
> box: add SQL settings to _session_settings
>
^ permalink raw reply [flat|nested] 15+ messages in thread