* [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view
@ 2019-12-19 8:32 imeevma
2019-12-19 8:32 ` [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine imeevma
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: imeevma @ 2019-12-19 8:32 UTC (permalink / raw)
To: v.shpilevoy; +Cc: tarantool-patches
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
src/box/CMakeLists.txt | 2 +
src/box/bootstrap.snap | Bin 5921 -> 5975 bytes
src/box/box.cc | 4 +
src/box/lua/space.cc | 2 +
src/box/lua/upgrade.lua | 15 +
src/box/schema_def.h | 8 +
src/box/session_settings.c | 409 +++++++++++++++++++++
src/box/session_settings.h | 62 ++++
src/box/sql.h | 47 +++
src/box/sql/build.c | 273 ++++++++++++++
src/box/sql/main.c | 8 +
src/box/virtual_engine.c | 134 +++++++
src/box/virtual_engine.h | 55 +++
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 | 275 ++++++++++++++
...4511-access-settings-from-any-frontend.test.lua | 103 ++++++
test/wal_off/alter.result | 2 +-
20 files changed, 1409 insertions(+), 8 deletions(-)
create mode 100644 src/box/session_settings.c
create mode 100644 src/box/session_settings.h
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
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine 2019-12-19 8:32 [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view imeevma @ 2019-12-19 8:32 ` imeevma 2019-12-21 17:59 ` Vladislav Shpilevoy 2019-12-26 4:45 ` Konstantin Osipov 2019-12-19 8:32 ` [Tarantool-patches] [PATCH 2/3] box: introduce _session_settings system space imeevma ` (2 subsequent siblings) 3 siblings, 2 replies; 13+ messages in thread From: imeevma @ 2019-12-19 8:32 UTC (permalink / raw) To: v.shpilevoy; +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 | 96 ++++++++++++++++++++++ src/box/virtual_engine.h | 55 +++++++++++++ ...h-4511-access-settings-from-any-frontend.result | 10 +++ ...4511-access-settings-from-any-frontend.test.lua | 4 + 6 files changed, 170 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 5cd5cba8..c011bfc 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 + 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..45993b9 --- /dev/null +++ b/src/box/virtual_engine.c @@ -0,0 +1,96 @@ +/* + * 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" +#include "tuple.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..80ada45 --- /dev/null +++ b/src/box/virtual_engine.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 * +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] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine 2019-12-19 8:32 ` [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine imeevma @ 2019-12-21 17:59 ` Vladislav Shpilevoy 2019-12-26 17:59 ` Mergen Imeev 2019-12-26 4:45 ` Konstantin Osipov 1 sibling, 1 reply; 13+ messages in thread From: Vladislav Shpilevoy @ 2019-12-21 17:59 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches Hi! Thanks for the patch! I've pushed my review fixes on top of this commit. See it below and on the branch. If you agree, then squash. Otherwise lets discuss. ================================================================== commit b3d8950d07a620ecbb19318495d33a59af5733b5 Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Date: Sat Dec 21 18:02:43 2019 +0100 Review fixes 1/3 diff --git a/src/box/virtual_engine.c b/src/box/virtual_engine.c index 45993b91f..9a59a3f6a 100644 --- a/src/box/virtual_engine.c +++ b/src/box/virtual_engine.c @@ -30,7 +30,6 @@ */ #include "virtual_engine.h" #include "schema.h" -#include "tuple.h" static void virtual_engine_shutdown(struct engine *engine) diff --git a/src/box/virtual_engine.h b/src/box/virtual_engine.h index 80ada450d..9cb5f520f 100644 --- a/src/box/virtual_engine.h +++ b/src/box/virtual_engine.h @@ -29,8 +29,6 @@ * 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) */ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine 2019-12-21 17:59 ` Vladislav Shpilevoy @ 2019-12-26 17:59 ` Mergen Imeev 0 siblings, 0 replies; 13+ messages in thread From: Mergen Imeev @ 2019-12-26 17:59 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches Thank you very much for your review fixes! I haven't found any issues with them. Result after squash below: On Sat, Dec 21, 2019 at 06:59:17PM +0100, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > I've pushed my review fixes on top of this commit. See it below > and on the branch. If you agree, then squash. Otherwise lets > discuss. > > ================================================================== > > commit b3d8950d07a620ecbb19318495d33a59af5733b5 > Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> > Date: Sat Dec 21 18:02:43 2019 +0100 > > Review fixes 1/3 > > diff --git a/src/box/virtual_engine.c b/src/box/virtual_engine.c > index 45993b91f..9a59a3f6a 100644 > --- a/src/box/virtual_engine.c > +++ b/src/box/virtual_engine.c > @@ -30,7 +30,6 @@ > */ > #include "virtual_engine.h" > #include "schema.h" > -#include "tuple.h" > > static void > virtual_engine_shutdown(struct engine *engine) > diff --git a/src/box/virtual_engine.h b/src/box/virtual_engine.h > index 80ada450d..9cb5f520f 100644 > --- a/src/box/virtual_engine.h > +++ b/src/box/virtual_engine.h > @@ -29,8 +29,6 @@ > * 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) */ From 6df53f4b5acb3282627e5088c953094dbce261e6 Mon Sep 17 00:00:00 2001 From: Mergen Imeev <imeevma@gmail.com> Date: Sat, 30 Nov 2019 12:59:45 +0300 Subject: [PATCH] box: introduce 'virtual' engine This patch introduces a new engine called "virtual" that will be used to create a new system space. Part of #4511 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'}) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine 2019-12-19 8:32 ` [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine imeevma 2019-12-21 17:59 ` Vladislav Shpilevoy @ 2019-12-26 4:45 ` Konstantin Osipov 2019-12-27 13:45 ` Vladislav Shpilevoy 1 sibling, 1 reply; 13+ messages in thread From: Konstantin Osipov @ 2019-12-26 4:45 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches, v.shpilevoy * imeevma@tarantool.org <imeevma@tarantool.org> [19/12/19 11:33]: > This patch introduces a new engine called "virtual" that will be > used to create a new system space. any engine is virtual. while the previous name was a disaster, this is not a descriptive one either. -- Konstantin Osipov, Moscow, Russia ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine 2019-12-26 4:45 ` Konstantin Osipov @ 2019-12-27 13:45 ` Vladislav Shpilevoy 0 siblings, 0 replies; 13+ messages in thread From: Vladislav Shpilevoy @ 2019-12-27 13:45 UTC (permalink / raw) To: Konstantin Osipov, imeevma, tarantool-patches Hi! Thanks for the comment! You are free to propose your own name. On 26/12/2019 05:45, Konstantin Osipov wrote: > * imeevma@tarantool.org <imeevma@tarantool.org> [19/12/19 11:33]: >> This patch introduces a new engine called "virtual" that will be >> used to create a new system space. > > any engine is virtual. while the previous name was a disaster, > this is not a descriptive one either. > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Tarantool-patches] [PATCH 2/3] box: introduce _session_settings system space 2019-12-19 8:32 [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view imeevma 2019-12-19 8:32 ` [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine imeevma @ 2019-12-19 8:32 ` imeevma 2019-12-21 17:59 ` Vladislav Shpilevoy 2019-12-19 8:33 ` [Tarantool-patches] [PATCH 3/3] box: add SQL settings to _session_settings imeevma 2019-12-27 13:45 ` [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view Vladislav Shpilevoy 3 siblings, 1 reply; 13+ messages in thread From: imeevma @ 2019-12-19 8:32 UTC (permalink / raw) To: v.shpilevoy; +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 | 409 +++++++++++++++++++++ src/box/session_settings.h | 61 +++ src/box/virtual_engine.c | 52 ++- test/app-tap/tarantoolctl.test.lua | 4 +- test/box-py/bootstrap.result | 3 + test/box/access_sysview.result | 6 +- test/box/alter.result | 5 +- ...h-4511-access-settings-from-any-frontend.result | 108 +++++- ...4511-access-settings-from-any-frontend.test.lua | 37 ++ test/wal_off/alter.result | 2 +- 15 files changed, 697 insertions(+), 16 deletions(-) create mode 100644 src/box/session_settings.c create mode 100644 src/box/session_settings.h diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt index c011bfc..2cb81e8 100644 --- a/src/box/CMakeLists.txt +++ b/src/box/CMakeLists.txt @@ -79,6 +79,7 @@ add_library(box STATIC sysview.c blackhole.c virtual_engine.c + session_settings.c vinyl.c vy_stmt.c vy_mem.c diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap index 7ff3aa5984dbe41a6d4fef7a9ff427336691784f..c573eecd01cf7c7ada949571750f5ba7f30a0e58 100644 GIT binary patch literal 5975 zcmV-d7pUk{PC-x#FfK7O3RY!ub7^mGIv_GGGcGVKGB+_TXEtUxFgamj3Q2BrbYX5| zWjY`-FgP?}HfCfkVliSjEi^JPHZ5XiVPh>}W-~ZuIW#jeW;109RzqxWV{1AfdwmKD z)w&D1%@*GP&H{cG{G|W@0000ewJ-euP_?!I`p{@ENf2nO>HuFnz!wkj#k+nW%pPgt zBZ0)WtpN`tMI1&W;^gZsQ&N;l)}E-@Bxx#$t*xi7PvyfprWdR<qHD&edh~*lhI(p? zO)caC=K`_<Z~kv%<)pw3o8_FK4QC~wfU^-yz*){hAi)C>(4G!J07hhyME2Rh12qO5 z03$jVXaJ1pRDgk60$>Z!*eC$j5|Pg}34qnqumK~c0b2t`L_iG~k?1@i4HzdHfB+|Q zBANqOVgUxQ#40o_ktH4rAQGvnh(zbqk(fnv8maP`cPgLx#(XB9@vq<g`z_`D_S@~l z-AWBNcZYkpi|uh``Eex{kLz(g!@lb#zUzACUEg)M*HyT#s~)baQx&djs@FBBuFjll zO+Zj<G;2Ko0kx+9DD4@*08@EIPUZCpfB@@iCV+J<1HigE$(VVEW6HckF=XE1I|PoY z)%l5`)$sw9wj0`ob$UOcFpN(qjOq!UyfEx@7e#wQ@z^ayK6Sg=7Z|85{G!`JI{PaJ zW(%LbY@wIj7H*-l%ac%u^CA>FbrA}kH{AbCPXF6(|2w%f)%I+$Y2l*g{=M$Xyxo7R zxqlS(dQX-uShQ$;C5zTpu(X=#N~@Qww7S(*n6##{r1g|4T05ms3HoPkO3*)PQi5JZ z2K`g1R@;+AqwR^J^s_1bY<r+2)xMrYp?x`lLTpp?MR883R=tRA8`QjAT@=+{O`c?x ziIc1%HOXpLkC+^nARUiNj*iD9OFR-K-XBTgy%8iib<U3D)M;cSr&&357Maz&ii~Jp zM2axke!O`NDe^*ycpqel_e4VYP!P_65OMxvNC$lY(LtdfI;ev^d{kBO5UQ%$(W9y= zh==z&c6gs7$a@?*dcTpQcN;i*uW_STq+z32oKd4#%uX3JYB6T0yiK*&qQQ_M%vnkv z6=u}=BF3?2zJp637{%RP=1$Qq*$W_9jLos<{k#6n^$)r3-#69F0p4;*tkpK%{AFUS znGg_=EXF3yoH9MzF4wfgnzuX|!<q>J$zp6Ukc_w6cWq?C!9cPYn?E>A-6&NqF0E!k zI2Q90w8jVx3r9_$vBB2D3t4PUOe4yLeFSlH5$V6%CUe@dOZWJ@=2NXQ3yZh=XO?YK zbPEd+_0eoFv+T=Jzq$0wW~1`RGm_0w6Olu{`}fN=R%v2Xzo?7KT-0moQ6>(;VY&CL z)U5k{CHE{+)qJ=;f1BDp{-nB5oo3fJbushgnm1V8sQb2k*i;kKY`YTE%m%ge*Q{1i zRrjuiT1D9--$oP<#yBj_YzT<Zj~+hWwTg$(jvhfgG_md9>)27dlOu?B2ZxS*<lDV- zBS-Da1`gU+jT=vzQjHcIe$lW|!!x4>O$H9bVQUqgyiOVJAc`l(jI|1ejJ4`TjJ0a5 z0)kPK;V>LV!#E7+c#PpN99H`ShBXtyVN+AiE@C)r%E`r_+H;F8XiqJ;$WzEX^~_?6 z)`*1`t?`O1T4QUp0*h^|;)-pg!isI&#wn`kuM|}DH;O6x+g~W8c;6?YcwZ-=c%4l= zQS)NL37XduO@O^w!`Dj*Ci+ieiT;sLqW}JnNaBAZkododBmQ?1VZ{GK6!E`P2qLna zK@72eAcR=|4?(Pd{XPKke-A(W&w~&DJ9p?Ie;s(pKZhOi&mRXJYI$zRftJSx9E`q> zs*Al9>W-pDpQnZ!yr;ni?`NpNJ7u7u<`u&XG%pxrVE(K_yZ3q_hVET}p?g0sbl-iL z3sN4;kCbmAOv+1n7Qnn;p#|?%V8QzoR`4DL6}&$o1@ANgg_@o|C(!ipF(KWm@9ODR zQFMDfCG^>z1U}o3P{Q`xi$H?*A&j6s2qI{&{RbgrzXycu^?Z<h_FsG2o<~pH@qD$t z?RWC9t_OHnw}U&Z%er13WY<my*{$Q1-R#n_)7xjA-a6^@wzpl+V$0<$wp(4rw%BTM zHMd&NnOb0;BmHyP<Vq&z$tCAVWsqkjgPa3okO%pktiJ&V|7$Sk{|q+h7--P@1sU{i z0S2s`3s|-S3s^Re9AXixf~z>hBC!Yu%V>OaWwe50G`a~r+tV$HW1PV`@ou>tkcnaJ z-!9jSTH+UL7alIE%x{T-hHZAapDqV%LUmz$To@|X`z5hV$V9VJQ?kOOWK>gnLQ{Ie zq?APKU``qvP=|EWhYugk+cx|aHs{!YIwWp|d_(Elzfho1wLuRs?h)HzcBIj$G9eQG z^at6DRRXM3w224!3GgKV`~<+08O)^&7cxv^n8Yvvt(LN-Uu?+-Wc6mLrxZphN{N@W zOM*($A%Q{q;sz;q^mOFK%~6*b&T8glbh9#QDzX)EDcZ97yb{qeB+C#iL#_~6Az~t0 zlF5duU@JF#@DP=uDL$n5s9{VKQi@r*>7!q%&EKUOt@}^Lf^Znt%qWRxWS?Wty>*NF zYpId3`TczxnSRLk|JtLx-QH`D+QDd|Fq?0<@Q1<%GuNo!TWXsozei%F=o7O~xY#z` z%BuVFRxACvsN~|hXID#Q_nON2eCPf>Z&Q>gM4wo7lHH`TYp#c=-(6f+bnvf<(n|lF z^0$9f3x8qqW;opaqfPvT^d_yof7ujmVlGF<-Cd?7L(OjE?ygqPY)-e<PxYB#%cXVu zdG14|ad#J0;w2?lUHjLFF0oRQ^}qS5YyYB3oP-2U%3q-oU1FpoU4J&C-)FY#XWdh# zE*9IA*}7Dk$+9^`S@z|srfP#K&mQ%=E8X93zKdOdt)hO{@j6X2%1U`UKM!>u4zrf$ z{r$Q#{btc(?%CZ!HQ)bjd)HBIgUMljhdi^*_iNuHwnNHaA*#ejMzD-16VGNnyULBk zqS7kg=n@+l$uXi^xTwg_D*D7kig?y++XmV6sMj>7%HQ+%N~+9v5!v-nf4-Tn{yr&( z>sLy9R0|Iw632)#G0~8pi@g;thnW>BKY!a4Raw1-Tzn6?UdBO}cu2^U-bR1)0Y$pN zTyO!}LiR-qrYw}OK)PaPMNt(*RA3)d$b@j1Qi@v?O;FgL;NrxVCbTe-WeF@wTuH(T z5==)d8BwJODnv{fLW&TzAZS1+{gC8Clpdh)@MH%kIyA|F2@Xg%9NDm>1|>8knE{Cm zM`ADn!|Vkk7mD~P3L^qGqy-@?1X+Pdg`pIHpzz}aA0`O(>A{Cb2|Gv#8bOB$IYPhz z!i^7X?5UxL#@CEIGVsK>1H*$C9RPXB>|hzbnl0h5KY#xC!E}aV+IMZ-{B1s8$YS_v zw&d*EeZ3MF=}I^2>du9MxHQ}LWV+el>n^eEYL<k<_%5nhiSgAeNh`}xxVlHr+Q4*4 zn}ji=r;fXy<bs*f#?=UdQeOQNH?OIdny&qu{9eC9C5~OqlA1Q0PKj?!UlTVibzyu+ zD>o|FMo33UYHr!1-?jJI$OuWRZF(dH2b=P5zPTh<|GH1S1^M+5{rP*8mGY<)YsuOH zn>Y&xON?a>?<uFlfywE%@Rbdmc0itwiLGeh^Wk($%#^{;hU<A@;w21tz9g=4!OhnD zk-;%B6$^g892wjaPm$BZiNPtclnQ(}osD;E;#wRV6cW?0a!^RDbj9o8%rtQloSaSv z+%0jGH%&ZU(-wweCB}xtPu$pkyq^*~X#?~jaT5x9VSGu<WP&IqMv_36g^wa2gG1sa z5cryGNUY=mFgaZkCviZDku<Oa@^0uDUQY`jVc>`R-FRKt$O1elB%Ue2<LSC6u~7oH zryCb8!UpZ(em7gs_H?U*i`IDA<#ajRjfsh<$>DBEJR}Vi&uM4tEwK;;bUNU!OKypS z959BbQ(__ma8Xe}B_2ZKv;*>bN(`icU(UyiYT_SNPd_g^oNkGI9EGT&plm;%9o%CI z{q1nQ9`M$Mc^JF&e!eE&@#20qrzQ^iP`O_c>+m+GB+k*x*qnB@oDTR!3*(Ub*={@D z4UGcwdS1Hl4fp430{WKNMtigAqHsMXuEBpiU>GH)QQYxzz8+2t6VG7D;GU3J1~G@Y zsDpzmno@{n0000$001)^1Q3MPtdp<`699mLf`h{3QfMR;2x1s0lU0-<1Aw6b0+2ue zNN50D9hCubUzk=eTWMHeEeQ&&B|(90jA<`P9Ct&ez~Rer*3`U!r6X+ry+u20F;1$I z70`5q%&)abhb_iQRk8w_j*$7a7U{6XIH^ijK+_R2zt$ohwiqW>$qHyXLgv?6q{9~D zq$*hfO-IQ5T8nhpVw_YZE1>BJnO|#>4qJ?WR6Jci8k8niGsf3Nrrw5(3?4)kg3YMe zXbNE}MyPcxl_H}!5o7*%K2cd=DZXFVK|h8w7!m{K#c}>?;JLZS<Klsk+KSw=oTlUV z+&@~K?kE90x2e-iAri04kM@1as-x|jjuJnE&D+2)3cZ0xkZ1#TB-P!;`M&lkkwWO@ zRKScMR2Gp;Nt`^`w$S89<bWx0awIev!{CadYNB>gRG7BO!9wB4@@L>g$VDhe2ucz5 zU~NTvkE#RWIw2^b=UX~OAHtj+AzxBCNI752$H@ciA{0tkx)$?Z)`EzLv0KnE^rh5E z!S&j4Qehm$J5tgPI?U!0yZc}qip7vNMR?ua0)8Mex6@1@n!>M*>k{K!bh|^BqQ~=E zQSM>5D<X-c(sjQaI&5staeJz!R$jS*rNVe~aB~8xaKrYSDBcTSuD@C3V;N)Dm1v^7 z)ca{l;n!~O0;89}4vjrUDMIn?1$_NDC%!yI+E4Ge{H~9j7>OkG@t(`mbHBvb{S6Fm zGt)UTp4D9AH_<*CLwUsdxSxAo<c&zT?CBRW)k%Wv_RW?pFc4THd2P&=+QwO{A^y-a zL`uZJ>+mMPHArQ!-_+@1hN@InfbB6I`=t<7)YGHq<by6fKJlTnzbI-vry=#npA{lz z2&#u{@OwKD-+g@K%L}C54P9(-oYB@G;}=2b@npk`pw>8(p+yi&hCJk+j9Wrb-r5g- zBI(pP-Y`UQ7<~o&C;BBp=$YQb=mS|oP_^2Rz`z+rH&4bb5)(n`@g~Cy34*7`lg$Rm zBXcuMdJ7C0bC}d{If&GR&G)!C&cI`y`B(1MdL<Z5H};=Z&M)5piZUTog+ax^r}`Wz zZr>vVetgQ8ff1`^Jr$#=;ClYWHM5vW-`4ZOUxT^~9eO5EDqDWVDz0MiU&UhCHMx&w zPIcwu6A+n|dD5|~wtkbm-&Jj2yi-Ikjg3|Vz0k@uO12CVz_R1A_BU(!Y989Y_ zCw+23J8Q0bRE^$P09@>CjuwnMsoA_U$qT9vt`Wqh%oL7OhqB(_p+&~hmz4a?lsyDD z@1hT0L4HPXODI)m2z^bLHeZc7YZw#Hu*MKQKjx`v><(#5I{RK?XDE<Gd*k@uZaZP? zx{R`t5EK4QX0lmL8c}W{gJ@dzSz=kyj0;#M{vpLZ!tW>G_Sj+S(pGg*h;D<e&=dn< zoAuvTI=)UJCnhAEJ9yVP@h5U47{~eG4F&&aiiJu#OgGcKO`F@wUztwyp<Cz3QGB^A zY_%Zt;jkQUb%=4S2I+X}FC6YR(Ixj?dv*@nNxy;gX5m6xh@;NME1}UROo<$J=&v%X z;4;!>wp0Y=tCjQe632739JB^jPAc(pnd6wFVgiims0HUm!gQ!nt1K$GHVIM0bS&l^ z8-u0$c8Vo3=ZKF;J9q28e-@oEcfvD%KIlim$qBr}rGyBig;ItnSjLLlxCsw?N;;uj z2?YHwV!<3cRVhHMbBXe*J(<F*LBK6Po@qu-(F891kKcS6MRRl!N^0p+tMe?bRiPyS z8}`MXq#jMy+!2Bo7kJW+L34UOG=7qVH}k!&2jSHwa;~Q9O^w3*$Z3rcn8UqG@dhbw zx44wC{?pQo87{QHkZACS43EUEnX#|+2jNZ<gEBN?z+eXJo~{d+i{B|@q+~!K<pU4P z2v~HIOE-NfyL_fE&E3?k4yu%qQWzF=5270!;PQN4)fiNhvi!{{`hRqx+d<?v@bpI{ zK*)!pf6=*VXtFP2J3*3C6ryy%4Jo70q~g8`B~M9Wp*@X2iG=l@=<$N83m~^92O)Zm z7S0T_j-C;e7T1tkP7n@8N0O*x=*KO{A3N6`4bh?19#8>U{I_oCO&fIiIU#R1>xSmQ zG~0-3Nr?;y<1&LGbdlIF`{ck4@l)s2!(eK#H6ScN6aTIm{cYEpEQClbHhk0;SJP)t zYLul#z@uBlgPpez!>h<*NvMzx_S-k~y?G09fMFLAEp8dc=qs{V68h43G~R=kG@8|z ze&-$2Z6ELTF>$u)oV;isdO|I^K?nb>8~M5&AC%c}dxx*|nC-r?tV?Klhfu>>qv#L+ zL&Q>hHZVlAV6CKKk<kaj!lnw+j|tMrXj3kD3H>Cq*ucNfWu~pSs2F;6*5pF-iM1o5 zwBBu&x#j&==*!*PK07+v?kBODnSbp9Ttqq3U?#+LAXE$X${Dr}UO1E%Rgh*3uuhGh zQlVeyhb@Q)8t+~W*P+!OP(cu6DWT6&1Apu&hte_%(ue_OQ^Qjt^bhUe1@S=R-K*g` zwAuqI2!bpn^jT`)j~(SuT1G({F~DqUcuIu+p&b?^wo~9C|AFiSmM#F+VXA$>CZw>^ z1<HcTz<zXm(KrylV~Z2RB7$!KrL_)HHdIFL<MWDKcF6uE=3gF9+`UEqCnEpR`9*dj ze2JFXZAQLN6zMuwq4Y{Ocl~KT(d=p4pO7%{dy;92{{#t?P2zJPR1bE`8M_8vJV+Le z%rq%>UOY5C9%(`XqY?Mc!pTUvAjP<_-Fl90Lx?<%_%O1SsqMxM1BJI$=y)ENqMeL5 zXvFYZ$x0;Q?mPOiGDiae@_`CM<46KoMW0w7&Hs$#kdBudfmRg^gA#TX`F=m94KVVT z7i}Sa=rbX^!K7jM6;rJwd^tsEoP3u=`zb-mg~=I34z3jvg<)%=T3~J%e2J9Ph+57L z*NZ3u8T981UI-7gtby9lX&7EbR8tZ~4vzkdC<YA7VWry=AKOg$YA})c`fZv_-xa^e zu%@*7DkbFo+Hdn1`QD%x1_BG+O1UoN2(aAA>v*GyjL9(#bkGVa117kEYZ$a~zqvUR zX~CJXkJE_~kvq0hB$^<h`vXkIQ?3)PsFZ>wGK0FH&EVi++YNFp8x8`?utQ_<GRM!c zEDpL&gcX!yaE)oUZ&`%htF$2oM+a2|)be|H!2R-eiNhmmI{;o`r7YIOp_0LuoY)(Y zbwrV(#}Wr#QeJ(XQ~%ljAcqK<&LkJsfI1dG=3x2yuPsc#dO_oPx#dea98RcD<|kGA zTUkkKncfnZpT~ZSr|<fl?8e+8jTJ&4r=#abs?CD>RZjnpiXSn9r)&sA_Xg|WY6BRD zo}bhOpB@;BLevumqE&Tcm(BJ)F0fA6t&bK*bxKLBtV35r4(Ws)EM0#K*8k<S!9frk z4BP~W&fjE;l7wqShOI0hc*m7EKcYdfXZ0-gFc7f5e4s4{@JIk42fgI72F-U4v|vv^ zLI6CLhWO#^Dy&%CO31loB$OnWO!H25)=Y=?7{s>5CeJ{w_ietMSep)Hp#T83n;%r; z20jQv%=@1hy_g)9h}q+nqS7JU3Chl1dCr@>M^a{TOLN@hJ(4n$Tbko0?~#<5+)~e^ z>5R)SIole7{wtP|Eg}7NKgX)nif!^Y`y)Da=L=$iK1CAMD>;voojrs}%5Q1-p%2v% Ft?dNQUb6rI literal 5921 zcmcJR<x>=n*Tom91(r^wTT+$<mWBoC4v~%pmi*E!(%oH)(%r3;NP~oQw+PZLx%_<o zi096kx$}8-Uz|B-u8ulP6PJ${48hg2c5}CLb`oXa=Y{a{@$f^SJT?|^OFm&s3n&;@ z$;sWr!pYKFlz~r3$O0;8$<Kp;3kdNDzy+;&;6e})9)7sB2t)({hd>}!xO%dV&Xx|M z3?Dw?;?A|>wU1Mj0Ve*eqVtph{%>Blym<m>UN-|?O)T;%5xE$8@{RHJfgocHMn!R9 zZ+eEAlJ|W3038zm5MR)D1}+`#H0Al)@+B5K8R*T8%Oo0WrKzQZviHaHvRfu2yQ7I5 zu0MjLI7=@!UevrG`?7U}LGGgW0$2FQ5|AxOIwbot7}y<g<T@-$8x%C(gKpNxhECv* zkHiy@*4c~=Hb7!_FK<na$Ope!&Ji+`6*P0~j=-tDdqsc*mfvmnp_6RNN(unby!3m} zj*><JUc&_On4m}<-98JQQQ`hqLa{m`<6P-8k8t(WncIa^)KkJmz{9cEmQzs}vZI%0 zwSWFg<LVcJ=)t)!r4}2Wgd3h}m$j#ytB0?Q7vAA5v{Ya%2+kei>MkJETi5{Qf;la8 zL4X_`fX8o;IiRAGuH@nXLpsP)j<l!!Gn%;x67jRMm*2*e@g=10j2t!k^Wb)P?BFV3 zBnSWayQsJspKvK9Naj3be2r?pA~Z)N&+CYQ?vn`ZxXRcJLBz;0!>8kL(xy;#3ojy& z6SjB#-2ewwXD|*HvKu2*ZRI$ywDj4b^%-fJW$V{I`^87g{n^yrdi~_v+^STJUyY3l zC696QM5CMox1nKITFIQUzTw&dmx`g0)^z(Qc@0vx8qk%hhq?a=!YN%x7rdVRw&)Km zXZ%W4>0&O$Ur<6j$<|b>bgVO9@ntK=ePJ7$Zk0)2E2jN=EpLo{9lKWJ0fw?(;%6(t zDtYxdwS+i%HhKA-6gE>F($iG#bGqo1Tp?>Ew`Fdb6rJi^p~PWZlecnByvzz2j?m@y z7$RRL`A&N6{s%@TV_V>!OoT8YQ*p3#n60@c5&mosy>xlGKZcgp^({;e^H<*oI*HB= zv|n8WbX|LkQ(^?PT^qwNmgOE8%lsD@%WpP(>gJb%y6dIOHse-5frx6BL@da19Jp^0 zvLU^j2--e5aVsm=;~vxi%?~bKqD%sR--jFjJ<WFb{pk3GdEOrBgyKC<=ic9A$8sn_ zOuRHzi?q~+j~gX~nT&)4!0Ux>*#}pSJ9v3{dZfVmNXgp@Cl5~2^mqIWpwIz8KNf(k zrBtFs7C(XtOobfW5X`hs)qmf)Ce`GKEHo6hjY>oWXIq;k+;^3j2~noXe@^@_&d-PJ zV(r`e#tLn8<o#PURYakwP9iUuDB$<HyLQvbL+Wai!aMh^n@4N=h1=u<dCwe^$=Uv~ zWXDZ&&!UqRFGhEK)uv-nRS4eLv{{p(+`F|70()}ncz?<%F{&LKn(U;cd<LnYPOkLC zd^)Kh|9y*<Z+lar2pjC5V+jOwE5mIL>_Y@OlmxVpI%<*JCYEK;>#N4#P_MGV_?P&P z{gdf2>ZVmdC{Y)rXi%9obWoYxa!|QN`)N3T;EYtthDs_hPM#`Yo4saoA)Om2RZ^T| z(Q~F#i6c*+a~aErQ2$pC+C$vCN87Qj!{L<wxyJG?!;NM`Vok>2u_pa8u_mhnT#fs6 z<oZ`_F<kd6W}MYN8yuBBW*p@{NG(;^VTeqO!2+q4f0ur5+Yg-A_s-kVsBD+QXH=%` zb|7oktyDBC%9*L4|5<_MJa7icQlBG7(-6QEr~gbOR~`5yLv$WUB>PAhM)V8~1TqEE z2MvFVq`#QV!=|_O@!GeDP{5{(=}xGO?!rGvD3nWNkEHD!f?=OJ4}XXL*AqPdt`EP7 z_m!hQDp$7SG^Zfq#5%v=feNwT{ZIzdLUBdBChUWZD%??acY32?of`Rk|3G1U77$L+ z`92tPL)8M)kWP|w?BYm?ajkOi;E$e`$AkFmvn5oR#0oN~_hu#2CFF)BEB^_|YFpO@ zWU;EV2|K!sO8R(X>bIQ0uCa3J)6#Y9byn{L^wy`u5Uc*tZo+8w7oDilsEn&)spJyo z{K?aArFdp#-AyF=STA+a-j&!0j%*p2wU;nMeDn>tQfEzN&&*{`&!tb5P6)?vBTSGc z_~l+7fKB%}`91d;x}WdR13P``_)`q)RlOU(`4S$V`I4|(M1K`t^)@J4wh;6E0256W zJRNgjn_Bp}qrzVAEMmYO3(Buz%p9MMoFSfB?955UkG839EB%CD_Q2BsD$t1BfpNIT zZDy|FAH-=y2R2jFPGt5yW&K>uTBiu;J@;3n(WzpwFp%lxm*tcxlv7fUX-o?fK(|Q9 zS$^Z~-)A#PH*N%d#OL3e4fx9J6zZ??XZf3Uv1&^X5g!We0ftB$3{iLtmTJN<C$G`Q zHix76qlKXX#0p$RL%XuhwGq){)6OIBOX4`L&6PXzIE9rhQ}R>1lthc!i)m7_iue@1 zKo!*G&EvW|!R;eCl~%1xrIm%Rk|_&$-x}-Xy=OLxvE@i90cI4GFqRZ>HDo1`8($*` zDJq%XVgm7pK|A@y3@rL{)ROn{%a8ABr`iKqKtQQFD><DERbiuB6^r0(hgoNy7PaIJ z`a12|=%v?2j!1j!xsSfcbd;*9lj~X&vZV2Y+OB`M`}5~E4pC5{dbP|5FaMI@g3PRQ zEf1II;$Ienl0fyl6{(!?KH*?yvJ;dRo0q6x^1F?1jxqcVltusQOq;X-;qB@Tt+Ej+ z6FV#u#jjtRZhsQ@-fH~P5zE#<LRwY(Wb`&&*ly&0!*Q-9aEe7<8x9-YFeiSuUBqG% zC{(mzlW}OXHdDezvf{DSuH!e-C?aQYvs?MuKA3Zo?i$Ipw&C0zZ4y{h7_e|Iss<UA z_Z5}G^Q6Re&)9o<YJc;D(1M>V&B+LEJpU(_sr6s5qD4Tsc+=QlhiCK~+0hM#oPK%5 z+L(QkA660_BOg0+ra3ptNhrxjI0_B3_J=1q5BlY!eC8cjUM$U)F9?*4Y+U{wsThNv zFScj~c4yY2=r|$|p~@cw@F>9~Wl_B6n#p<pvdS5rjn5_(xI*^MpexkKAMdN)AS_)! zDk^PV&iU#GQ+s$Y$Q5$F|80fIeYsH?s?gnI>T~ypi6SDlgeRq0cvL0=bBbJ6&z#|1 zf5;g`Df~ZNHV%6&pWwnDq{g|6(_s{BjoiU<(IqTtNnhN;$eAn2VBx@JA_!wji1Z8i zD5180)HFI7-RGmtw~<9OaW1xQ=%mycv-x2WED(ld7=`TA<uD~NNZ$1I_KReSh<d)} zkR)6gZD23<@Mi|HfZWZ#FgjL1a*t&gEz|zbRAxegI%*%s)e4J9c_2hOX^MO&G*1*P zDjZQ`rF^?orjbYGq5~}bxG*qo#`Zpr3S=~r*4pl6me;S)JU!mt?!1-9J>E*2?L=|g zu1wRE!nno0@m7!>+?vA3(*_K!a=3C-8_T>Jp%O40Eq_p~rQ$F#j%8-qol|hV%t6%r z3)k@JHJ2(pD;=a{(6O7cm~jLKlk!7Ly{r9-+)O;IJc@s32;Q4~$cv3hXkIRr*Yzo< zeg3_IH-BKZ5Z&Q()<X1j_aQC*!j{xMyXXOYo<WeNkB2SBZhXRql{wREAY?oJ<kiJG zSO}~0{=spsjn%yzkBx{<=Uco^x{aYRir<5{8^O6KZ~cTJP}W+KNN;9$6*%YnE~DDk zCNOxs^2Vbdg6E>4kc(k6)KOpoFXlAn{X4^Po(5Izph>03*p5&=jw;gCygc?-0)>Z| zokr!U_E68r!r4irS<kgFM7SL>g0okMp}zS;x?*rPf~fDiNj^&GO4VU#YtA;ZKaPA1 zghyU<BPUyTdKJkQC1@L}$DJ}CQu^f_?bTa({H^duJMJ6>O~*&{ut_V>iu!h4perQQ zN&xRBMpt5t&t7WnuQZ~gX_9_Cd6>B<i}wfdcNmiplDmY4+_N?iy-Pi5+xfSP2p3jh zVAmuLc6zEcp0a-F;*c0k4MDS0^v4~!kL}cnf16w_=%4W>7c95wP`|4L!C=k6-n*nM z1>G8)XzwpJ_W$ni{>4iT_iXOYQEO`|(ELVA2%?<${!yaTOe5Nr=)V@4+4#18sQ$(Y zlr8!{kf)e&B4uP<d8oTsj&>B{-#;%606?fQh{EQoQ|oa_0X@MXeIXtk0wlN?U{Kl! z=V%gIk0t<3kpaLW05E>pj3$vP;I@vG)i?J9;#L#Dg4M?C%#>*UN{j*cr%;F8s6!OW z`T54-+y8@}wO%Os&nC0kuy!tZg7EQe2*=gP6UNJz>~LY6ptL}4rKcjFC|7@R`iknG z#N%~`^_RVQL1AbsC&JHY!1dL?T4&PxE^=@Y<C*ghCuTM3-c&(!Un*vbiAvtjE2K(_ z%vBvD)gw2|FCo9$W)k|C6<4H%n+{G+bS)m#tBL*OI5&65)8#;^?(BqYDs~sNJQm90 zG`wnq417vjg0-1s<Oww&>NPc4o**tXh9slOLAhZPNg2gm9qnOX)L*#$xy#U<iiCtJ z3@7^&YA4iQ?fk-egA<PRyKz9A2S@g;PbZ&%s=t$`cE0GjFbC55oR@4Ulz<uXb*(Eq z9S$z=Brz@H#gW={JQpAhX+L+675wTtL3!FQ^5@)^mBX8wD^?MA!lOcikam8>Z&k8# zBK`d=pi+IErc)SJc~bu*_jLiDeW)=1q#sewp(1eNaE;1Zy=_T_Mo-0_D_hbhr)r9O zl5}@Fe|<y0bvvV@8o1E0JIKKMQGA^2&sXQ|LsUd2-kiJF%Sqh1TLa_78?Ej-$&*Nv zwW>JlN*(M*LQOp*pB%oX$%T5FpJM^h_-|!nhV+RB*vh0Z7x<6`sQ0CE*;VkRpqWSn zeui|KExtw`GeP^6PkGrisxaz2H)4+7Mh}?9SWeTpbb9P0Tc_k(hnb`-y;b<#Rb^!g zyAdQ4&-~I5qhT-`#1o>=zq#aT^+*zOp%-v9qs~5f8NnE{v&nHLHT4EZTH}gNo^YQe zq;f6*18ahz-Iu~CISHhk2z_a)s3|V-*|AxMZUkvDZB(zI+4d_U7FLYKeH#!uk;TMU zMRd4>fyPYaoU!OXuauvOmMS^^gMp+<QG&du;%OT-&{3aN$92d!Q}^BDE80~YPkQ`W zOo{BkrojMwL-4DPjrzOYomWIz=?jYuRn2sHos-JWr4z(HEU_UxxgpaxNVLpc2Xy{C z=i_(e`wXH=^uj)eIP8M{unv2t54xD$4bsMrz6tYPq7`$Um!o(Ggtf#&rL$xSI%SRb zn+1r$>Gv6r8Z8nS(9=RfQ&@n!x$0|OS>Ux=Ca;;AqyrV{<j+AO38~*zu1Df_Dxx1u z;R>&}6_KuC>rP@8hwX8VDO98=HLpekH5x6afKXe*8w0q>)Mp(71d7o{qW4sT?C892 zwuIHU5U`3cDlow6RX_a3bL5jNBdMmy=RH<?K9Cm{Z=QSc{Q(zs8WY|ls>)S*r;1!T zGRyA0E-=~~*7E<#<JiJ*wN+2!>>1_SU3uhl@P_V0XbN0r`awMM*yS7S8o?-Iq+c*G z@TOpjT@{DcYrn3sYLkE3E@l>Uw<F#Cie$*Wg{B!xcc1gz?d49E`+z{)WVz>_oTx{+ zR+TE}7a<&-{wwZH8m`cWD!&+3{=KL~MKSNg?m&u9eBQl~0Le`SwB5Je(CRQ?akw;` zLw_(P=PM5A1K+28V-bue^oDYpzREO4J=^HW>QCZ3b3Hp5^pgTfIs)-{&pXgOz!Z&% zRf?hL;DLC53tKbBY!?DJGlTwE5qvB?NUIxPG($6q{hiOJ#m3k9=c{Aj-UueD*iNtG zDm$C`#R-k`ZJzkRF<YCCm%m>QxG(&MoF5<cb1S)15OULU8}z7TsTt9VFFa;$GUJKn z-XK;Imj(n@KOR^Zpv9_t{26?+o9Vt+@-?iTr@=~4o2EJ|0CrWCo#;U*D~OJKpMfbN z7pM?xL_EBK4gMH}0esNB5?W@$Rl9k1Vpr6Tk%>)TZPpG~=NGDyc||UdLoh>vP}Doo z2%?-d0!a1z#XtW+ZZUXvNdC0>=P%~n9};%U$6cUk@6l4o<8{nSOL40hde;k;e&|$Q zVRle0%zYcvYOUNP%bHq;doGFC!kCROj8~Kzk(~R5F`xxLKt(6AO(6cnvNOJ)Xj|6e z)r#SmPXBEgZg{ennlnriVrLt+xqDS@e&%t(`po>R=yCR?g!8(LKE}R`PZo6q*l)}* zeLO&tE(y9xoZxm{wc)4Wf;ZIkaV)P!l>JS6#AnSFl8dRp2i4C`L)ZmEi|bol*S768 z+=m44GrJ0lZ?9$gW{A%#+3_Q(l%7=(=2usR&&1@>M-@+85Ppvv?Prpoa{>2b-Q%pf z812F9WGz+&^6*q(PfZ}w#>wtk@ZQC{+MR~BrB|t1{kK^sfKqExK)RYj7AUXgjDuyW z5x5^1!Za+|tbxg_VPm!(ctO<v`RW-Xl)Z@EhaN^`m3KcG5=`7D_0M!*Bi^p1cIeL$ z`o}~^j0Ws$kqX%U5g|%}0%IxizF}5+_KFb2P8f0|bC)&C;BkTd%*^~aUKG^wKy6^e zBLtWSzk5hgn^cHEJYA#)Ki_p0x-v-q{?8IbTRbSUs}w|Vn+IAQ+N1n>ob{wU{4AbV zu9A49frZ+;j5(6_Nm~J5f;;`75Pl2;eV--p^YmlkNJ^_Xqs8Xm{#cwl90uE*Jf}!u zzkzrgaZj8LVIRtoOD!*yhOIApN-}@k@1sw<&}6r4GM#?pbKHCwhA-Wgg%wz5CH@r~ zpL5ljt0cjIn1RL5sIAbHbjABm9v)`a2^1v1NXf$nlQG<4cpD^h0;xwBY2OgaG;x}< zv7Fgoe7m1qw45y{&XT!m_42zP${6}tq}nEPiz4geFEJ%~DhpF1>4Ug+$wv^yYThTJ z8lI>;7+#mL#?*ZJ2fN@a2JLFrw0EEAQB8Yb4+c$D6*40c`X|c=@V>OB`acl+2X$i$ z&aqBzwTrgqf@r}4?-ceIZj$#d*+`f<z-c`-X@@ee;=@%VK2r~E4#(RaV?SwTOC~uR zxDrK0(B^~+P@>{Q)6z7fP<raLf1_#dCBDZA#fufhD4!d!LA>w|u%NiDJuD(3Aut_J zB$WeI>SgZg_EAl=Nm-}NBCzS>52hp25`1v8@LMMC%)UO%)66y%8ajuLUw+}_jf<6q zzd_dT1Aj!lt%J$X*O}?#X!1F)`68aJ4C{SKDw5L(4!~DT*gxt5|NG9g^nrTv+{i1W ze=F+NYQ21UXnfT4$3=S2V+>lm-=xj&*WjC~dt-)QzF1hL9)OM!yUG(CT~#(6AI`}7 zu7#Ysy4LY87mqe3*1v)k$a47X81nU`ycYhO+>vqUJz2JOnJsmEd-Li_;8z;kk=FEi zn>npn1}!y4!}lJIpv6bE(y&;?h-lUY+_$w0F8j&^#N|}j9$G+#`Q*^%EollIMyvcx z<H3Ai_E^fMKZn3!%LXZ_PbTB!)Gxn^K66K?<3-vP5>Fsb{Sz&g=2w3S4GVr9Psih$ z3FamP^mw@yI(_O<fB_8lNQ>IiCtrXkAw(`#dl^D$1)JlpsectmRh{e-kev$J2sah? z)n5`R{HaC<U^~v83zZ3>h(hNf{wCJqZztG>>B{wJTQtJj+DhM4MK)XcaIO#bTuJI9 R`4aDV?4_>I=7?&J{|DXTQmz01 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..b5b5db8 --- /dev/null +++ b/src/box/session_settings.c @@ -0,0 +1,409 @@ +/* + * 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 "session.h" +#include "schema.h" +#include "tuple.h" +#include "xrow.h" +#include "sql.h" + +struct session_settings_modules + session_settings_modules[SESSION_SETTING_MODULE_max] = {}; + +struct session_settings_index { + /** Base index. Must be the first member. */ + struct index base; + /** Format to create tuples on the fly. */ + 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. */ + struct tuple_format *format; + /** ID of current module in global list of the modules. */ + int module_id; + /** ID of the setting in current module. */ + int current_id; + /** Decoded key. */ + char *key; + /** Type of iterator. */ + enum iterator_type iterator_type; +}; + +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_iterator_next(struct iterator *iterator, struct tuple **result) +{ + struct session_settings_iterator *it = + (struct session_settings_iterator *)iterator; + if (it->module_id >= SESSION_SETTING_MODULE_max || it->module_id < 0) { + *result = NULL; + return 0; + } + + struct tuple *ret = NULL; + int i = it->module_id; + int j = it->current_id; + enum iterator_type type = it->iterator_type; + struct tuple_format *format = it->format; + struct session_settings_modules *module; + const char *key = it->key; + + if (!iterator_type_is_reverse(type)) { + for (; i < SESSION_SETTING_MODULE_max; ++i) { + module = &session_settings_modules[i]; + if (module->get(format, j, key, type, &j, &ret) != 0) + return -1; + if (ret != NULL) + break; + j = 0; + } + *result = ret; + if (i == SESSION_SETTING_MODULE_max) { + it->module_id = SESSION_SETTING_MODULE_max; + it->current_id = 0; + return 0; + } + ++j; + if (j >= (int)module->size) { + ++i; + j = 0; + } + it->module_id = i; + it->current_id = j; + return 0; + } + + for (; i >= 0; --i) { + module = &session_settings_modules[i]; + if (module->get(format, j, key, type, &j, &ret) != 0) + return -1; + if (ret != NULL) + break; + if (i > 0) + j = session_settings_modules[i - 1].size - 1; + } + *result = ret; + if (i < 0) { + it->module_id = -1; + it->current_id = 0; + return 0; + } + --j; + if (j < 0) { + --i; + if (i >= 0) + j = module[i].size - 1; + } + it->module_id = i; + it->current_id = j; + return 0; +} + +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.next = session_settings_iterator_next; + it->base.free = session_settings_iterator_free; + it->key = decoded_key; + it->iterator_type = type; + it->format = index->format; + it->module_id = iterator_type_is_reverse(type) ? + SESSION_SETTING_MODULE_max - 1 : 0; + it->current_id = iterator_type_is_reverse(type) ? + session_settings_modules[it->module_id].size - 1 : 0; + 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; + const char *tmp = mp_decode_str(&key, &len); + const char *decoded_key = tt_cstr(tmp, len); + struct tuple *ret = NULL; + struct tuple_format *format = index->format; + for (int i = 0; i < SESSION_SETTING_MODULE_max; ++i) { + if (session_settings_modules[i].get(format, 0, decoded_key, + ITER_EQ, NULL, &ret) != 0) + return -1; + if (ret != NULL) + break; + } + *result = ret; + return 0; +} + +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; + const char *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; + } + uint32_t len; + const char *tmp = mp_decode_str(&data, &len); + const char *decoded_key = tt_cstr(tmp, len); + + int id; + struct tuple *old_tuple = NULL; + struct tuple_format *format = space->format; + struct session_settings_modules *module = NULL; + for (int i = 0; i < SESSION_SETTING_MODULE_max; ++i) { + module = &session_settings_modules[i]; + if (module->get(format, 0, decoded_key, ITER_EQ, &id, + &old_tuple) != 0) + return -1; + if (old_tuple != NULL) + break; + } + if (old_tuple == NULL) + return 0; + + uint32_t new_size = 0, bsize; + const char *old_data = tuple_data_range(old_tuple, &bsize); + const char *new_data = + xrow_update_execute(request->tuple,request->tuple_end, + old_data, old_data + bsize, format->dict, + &new_size, request->index_base, NULL); + if (new_data == NULL) + return -1; + + uint32_t array_len = mp_decode_array(&new_data); + if (array_len != 2) + if (format->exact_field_count != array_len) { + diag_set(ClientError, ER_EXACT_FIELD_COUNT, + array_len, format->exact_field_count); + return -1; + } + + if (mp_typeof(*new_data) != MP_STR) { + diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, + space_index(space, 0)->def->name, space_name(space)); + return -1; + } + + const char *value = new_data; + mp_next(&value); + mp_decode_array(&old_data); + uint32_t size_a = mp_sizeof_str(len); + uint32_t size_b = value - new_data; + if (size_a != size_b || memcmp(old_data, new_data, size_a) != 0) { + diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, + space_index(space, 0)->def->name, space_name(space)); + return -1; + } + + return module->set(format, id, value, result); +} + +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..e7ecbdb --- /dev/null +++ b/src/box/session_settings.h @@ -0,0 +1,61 @@ +#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> +#include <stdint.h> +#include "iterator_type.h" + +#if defined(__cplusplus) +extern "C" { +#endif /* defined(__cplusplus) */ + +enum session_setting_module { + SESSION_SETTING_MODULE_max +}; + +struct tuple; +struct tuple_format; + +struct session_settings_modules { + enum session_setting_module id; + uint32_t size; + int (*get)(struct tuple_format *format, int id, const char *key, + enum iterator_type type, int *end_id, struct tuple **result); + int (*set)(struct tuple_format *format, int id, const char *value, + struct tuple **result); +}; + +extern struct session_settings_modules session_settings_modules[]; +extern const struct space_vtab session_settings_space_vtab; + +#if defined(__cplusplus) +} /* extern "C" */ +#endif /* defined(__plusplus) */ diff --git a/src/box/virtual_engine.c b/src/box/virtual_engine.c index 45993b9..8680747 100644 --- a/src/box/virtual_engine.c +++ b/src/box/virtual_engine.c @@ -28,6 +28,7 @@ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ +#include "session_settings.h" #include "virtual_engine.h" #include "schema.h" #include "tuple.h" @@ -42,13 +43,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] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/3] box: introduce _session_settings system space 2019-12-19 8:32 ` [Tarantool-patches] [PATCH 2/3] box: introduce _session_settings system space imeevma @ 2019-12-21 17:59 ` Vladislav Shpilevoy 2019-12-26 18:01 ` Mergen Imeev 0 siblings, 1 reply; 13+ messages in thread From: Vladislav Shpilevoy @ 2019-12-21 17:59 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches Thanks for the patch! I've pushed my review fixes on top of this commit. See it below and on the branch. If you agree, then squash. Otherwise lets discuss. There are lots of fixes, so review carefully. In my commit you can find inlined explanations about some changes. Also see 3 comments below inlined into your commit. During the review I also thought more about how to implement setting modules - in a single big namespace of all session settings, each with individual session settings, or like I proposed and you've done here, with modules approach. Looks like I was wrong, and the only extendible option is to have a single namespace, sorry. This is because not all settings can be split into submodules and not all of them will have the same prefix. For example, setting 'read_only'. I can't find a good prefix for it. Additionally, some SQL settings may migrate down to box in future, such as foreign key- and trigger- related. And they also likely not to have one prefix. Nonetheless, I've decided that we can move settings to one namespace later, as a separate simple ticket. Here we will finish public API. We can add individual settings in the current solution too, but it will look like a module with a single setting. Also we need to optimize search. Currently it is fullscan what is not acceptable when we have a sorted array. But also, a subject for a separate issue. On 19/12/2019 09:32, imeevma@tarantool.org wrote: > This patch creates _session_settings system space. This space is > used to view and change session settings. There are no settings at > the moment, some will be added in the next patch. > > Part of #4511 > --- > src/box/CMakeLists.txt | 1 + > src/box/bootstrap.snap | Bin 5921 -> 5975 bytes > src/box/lua/space.cc | 2 + > src/box/lua/upgrade.lua | 15 + > src/box/schema_def.h | 8 + > src/box/session_settings.c | 409 +++++++++++++++++++++ > src/box/session_settings.h | 61 +++ > src/box/virtual_engine.c | 52 ++- > test/app-tap/tarantoolctl.test.lua | 4 +- > test/box-py/bootstrap.result | 3 + > test/box/access_sysview.result | 6 +- > test/box/alter.result | 5 +- > ...h-4511-access-settings-from-any-frontend.result | 108 +++++- > ...4511-access-settings-from-any-frontend.test.lua | 37 ++ > test/wal_off/alter.result | 2 +- > 15 files changed, 697 insertions(+), 16 deletions(-) > create mode 100644 src/box/session_settings.c > create mode 100644 src/box/session_settings.h > > diff --git a/src/box/session_settings.c b/src/box/session_settings.c > new file mode 100644 > index 0000000..b5b5db8 > --- /dev/null > +++ b/src/box/session_settings.c > @@ -0,0 +1,409 @@ > + > +static int > +session_settings_iterator_next(struct iterator *iterator, struct tuple **result) > +{ > + struct session_settings_iterator *it = > + (struct session_settings_iterator *)iterator; > + if (it->module_id >= SESSION_SETTING_MODULE_max || it->module_id < 0) { > + *result = NULL; > + return 0; > + } > + > + struct tuple *ret = NULL; > + int i = it->module_id; > + int j = it->current_id; > + enum iterator_type type = it->iterator_type; > + struct tuple_format *format = it->format; > + struct session_settings_modules *module; > + const char *key = it->key; > + > + if (!iterator_type_is_reverse(type)) { 1. That condition never changes after iteration creation. You could reuse virtuality of iterator.next to calculate that condition only once. For that I've split that function in two: next and prev separately. > + for (; i < SESSION_SETTING_MODULE_max; ++i) { > + module = &session_settings_modules[i]; > + if (module->get(format, j, key, type, &j, &ret) != 0) > + return -1; > + if (ret != NULL) > + break; > + j = 0; > + } > + *result = ret; > + if (i == SESSION_SETTING_MODULE_max) { > + it->module_id = SESSION_SETTING_MODULE_max; > + it->current_id = 0; > + return 0; > + } > + ++j; > + if (j >= (int)module->size) { > + ++i; > + j = 0; > + } > + it->module_id = i; > + it->current_id = j; > + return 0; > + } > + > + for (; i >= 0; --i) { > + module = &session_settings_modules[i]; > + if (module->get(format, j, key, type, &j, &ret) != 0) > + return -1; > + if (ret != NULL) > + break; > + if (i > 0) > + j = session_settings_modules[i - 1].size - 1; > + } > + *result = ret; > + if (i < 0) { > + it->module_id = -1; > + it->current_id = 0; > + return 0; > + } > + --j; > + if (j < 0) { > + --i; > + if (i >= 0) > + j = module[i].size - 1; > + } > + it->module_id = i; > + it->current_id = j; > + return 0; > +} > diff --git a/src/box/session_settings.h b/src/box/session_settings.h > new file mode 100644 > index 0000000..e7ecbdb > --- /dev/null > +++ b/src/box/session_settings.h > @@ -0,0 +1,61 @@ > +#if defined(__cplusplus) > +extern "C" { > +#endif /* defined(__cplusplus) */ > + > +enum session_setting_module { > + SESSION_SETTING_MODULE_max 2. Usually we do vice versa - lowercase enum names, and uppercase _MAX. That makes possible to use things such as STR2ENUM. > +}; > + > +struct tuple; > +struct tuple_format; > + > +struct session_settings_modules { > + enum session_setting_module id; > + uint32_t size; > + int (*get)(struct tuple_format *format, int id, const char *key, > + enum iterator_type type, int *end_id, struct tuple **result); > + int (*set)(struct tuple_format *format, int id, const char *value, > + struct tuple **result); 3. The thing I mostly didn't like here is that you force submodule to deal with tuple format, to create tuples, to do search by key. That is a space's and index's task, not of SQL or other submodule. Moreover, that logic would be duplicated in each submodule. I simplified this API so now the session_settings space internals do all the search, comparisons, tuple creation, etc. And submodules only provide setting name list, and API to get/set a setting by ID. > +}; > + > +extern struct session_settings_modules session_settings_modules[]; > +extern const struct space_vtab session_settings_space_vtab; > + My commit with explanations: ================================================================== commit 38d1392d20ee90f4bf5418e2402dd5ec589cfb1d Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Date: Sat Dec 21 18:04:14 2019 +0100 Review fixes 2/3 diff --git a/src/box/session_settings.c b/src/box/session_settings.c index b5b5db81e..dd1874b33 100644 --- a/src/box/session_settings.c +++ b/src/box/session_settings.c @@ -31,35 +31,49 @@ #include "session_settings.h" #include "xrow_update.h" #include "virtual_engine.h" +#include "column_mask.h" #include "session.h" #include "schema.h" #include "tuple.h" #include "xrow.h" #include "sql.h" -struct session_settings_modules - session_settings_modules[SESSION_SETTING_MODULE_max] = {}; +struct session_setting_module + session_setting_modules[session_setting_type_MAX] = {}; struct session_settings_index { /** Base index. Must be the first member. */ struct index base; - /** Format to create tuples on the fly. */ + /** + * Format of the tuples iterators of this index return. It + * is stored here so as not to lookup space each time to + * get a format and create an iterator. + */ struct tuple_format *format; }; struct session_settings_iterator { /** Base iterator. Must be the first member. */ struct iterator base; - /** Format of the tuples this iterator returns. */ + /** + * Format of the tuples this iterator returns. It is + * stored here so as not to lookup space each time to get + * a format for selected tuples. + */ struct tuple_format *format; - /** ID of current module in global list of the modules. */ + /** + * ID of the current session settings module in the global + * list of the modules. + */ int module_id; /** ID of the setting in current module. */ - int current_id; + int setting_id; /** Decoded key. */ char *key; - /** Type of iterator. */ - enum iterator_type iterator_type; + /** True if the iterator returns only equal keys. */ + bool is_eq; + /** True if the iterator should include equal keys. */ + bool is_including; ================================================================== As you can see, I removed iterator type in favor of a couple of flags. I've found it easier to use these flags in the code than compare the type with all the known types each time. ================================================================== }; static void @@ -72,72 +86,114 @@ session_settings_iterator_free(struct iterator *ptr) } static int -session_settings_iterator_next(struct iterator *iterator, struct tuple **result) +session_settings_next_in_module(const struct session_setting_module *module, + int *sid, const char *key, bool is_eq, + bool is_including) ================================================================== I've moved search by name to the session_settings space. Now submodules shouldn't care about it. And this is reused by all modules (even though now there is only one) ================================================================== { - struct session_settings_iterator *it = - (struct session_settings_iterator *)iterator; - if (it->module_id >= SESSION_SETTING_MODULE_max || it->module_id < 0) { - *result = NULL; + int i = *sid; + int count = module->setting_count; + if (i >= count) + return -1; + if (key == NULL) return 0; + assert(i >= 0); + const char **name = &module->settings[i]; + for (; i < count; ++i, ++name) { + int cmp = strcmp(*name, key); + if ((cmp == 0 && is_including) || + (cmp > 0 && !is_eq)) { + *sid = i; + return 0; + } } + *sid = count; + return -1; +} - struct tuple *ret = NULL; - int i = it->module_id; - int j = it->current_id; - enum iterator_type type = it->iterator_type; - struct tuple_format *format = it->format; - struct session_settings_modules *module; - const char *key = it->key; - - if (!iterator_type_is_reverse(type)) { - for (; i < SESSION_SETTING_MODULE_max; ++i) { - module = &session_settings_modules[i]; - if (module->get(format, j, key, type, &j, &ret) != 0) - return -1; - if (ret != NULL) - break; - j = 0; - } - *result = ret; - if (i == SESSION_SETTING_MODULE_max) { - it->module_id = SESSION_SETTING_MODULE_max; - it->current_id = 0; +static int +session_settings_prev_in_module(const struct session_setting_module *module, + int *sid, const char *key, bool is_eq, + bool is_including) +{ + int i = *sid; + int count = module->setting_count; + if (i < 0) + return -1; + if (key == NULL) + return 0; + if (i >= count) + i = count - 1; + const char **name = &module->settings[i]; + for (; i >= 0; --i, --name) { + int cmp = strcmp(*name, key); + if ((cmp == 0 && is_including) || + (cmp < 0 && !is_eq)) { + *sid = i; return 0; } - ++j; - if (j >= (int)module->size) { - ++i; - j = 0; + } + *sid = -1; + return -1; +} + +static int +session_settings_iterator_next(struct iterator *iterator, struct tuple **result) +{ + struct session_settings_iterator *it = + (struct session_settings_iterator *)iterator; + int mid = it->module_id, sid = it->setting_id; + struct session_setting_module *module; + const char *key = it->key; + bool is_including = it->is_including, is_eq = it->is_eq; + bool is_found = false; + for (; mid < session_setting_type_MAX; ++mid, sid = 0) { + module = &session_setting_modules[mid]; + if (session_settings_next_in_module(module, &sid, key, is_eq, + is_including) == 0) { + is_found = true; + break; } - it->module_id = i; - it->current_id = j; + } + it->module_id = mid; + it->setting_id = sid + 1; + if (!is_found) { + *result = NULL; return 0; } + const char *mp_pair, *mp_pair_end; + module->get(sid, &mp_pair, &mp_pair_end); + *result = box_tuple_new(it->format, mp_pair, mp_pair_end); ================================================================== Note, I use box_tuple_new(), because iterator API assumes that a tuple is referenced somewhere, and that box can just bless a tuple to the userland. Simple tuple_new() does not work here, because it won't be deleted if anything will go wrong after its creation and before tuple_bless() in box. ================================================================== + return *result != NULL ? 0 : -1; +} - for (; i >= 0; --i) { - module = &session_settings_modules[i]; - if (module->get(format, j, key, type, &j, &ret) != 0) - return -1; - if (ret != NULL) +static int +session_settings_iterator_prev(struct iterator *iterator, struct tuple **result) +{ + struct session_settings_iterator *it = + (struct session_settings_iterator *)iterator; + int mid = it->module_id, sid = it->setting_id; + struct session_setting_module *module; + const char *key = it->key; + bool is_including = it->is_including, is_eq = it->is_eq; + bool is_found = false; + for (; mid >= 0; --mid, sid = INT_MAX) { + module = &session_setting_modules[mid]; + if (session_settings_prev_in_module(module, &sid, key, is_eq, + is_including) == 0) { + is_found = true; break; - if (i > 0) - j = session_settings_modules[i - 1].size - 1; + } } - *result = ret; - if (i < 0) { - it->module_id = -1; - it->current_id = 0; + it->module_id = mid; + it->setting_id = sid - 1; + if (!is_found) { + *result = NULL; return 0; } - --j; - if (j < 0) { - --i; - if (i >= 0) - j = module[i].size - 1; - } - it->module_id = i; - it->current_id = j; - return 0; + const char *mp_pair, *mp_pair_end; + module->get(sid, &mp_pair, &mp_pair_end); + *result = box_tuple_new(it->format, mp_pair, mp_pair_end); + return *result != NULL ? 0 : -1; } static void @@ -175,15 +231,23 @@ session_settings_index_create_iterator(struct index *base, return NULL; } iterator_create(&it->base, base); - it->base.next = session_settings_iterator_next; it->base.free = session_settings_iterator_free; it->key = decoded_key; - it->iterator_type = type; + it->is_eq = type == ITER_EQ || type == ITER_REQ; + it->is_including = it->is_eq || type == ITER_GE || type == ITER_ALL || + type == ITER_LE; it->format = index->format; - it->module_id = iterator_type_is_reverse(type) ? - SESSION_SETTING_MODULE_max - 1 : 0; - it->current_id = iterator_type_is_reverse(type) ? - session_settings_modules[it->module_id].size - 1 : 0; + if (!iterator_type_is_reverse(type)) { + it->base.next = session_settings_iterator_next; + it->module_id = 0; + it->setting_id = 0; + } else { + it->base.next = session_settings_iterator_prev; + it->module_id = session_setting_type_MAX - 1; + struct session_setting_module *module = + &session_setting_modules[it->module_id]; + it->setting_id = module->setting_count - 1; + } return (struct iterator *)it; } @@ -192,23 +256,28 @@ session_settings_index_get(struct index *base, const char *key, uint32_t part_count, struct tuple **result) { struct session_settings_index *index = - (struct session_settings_index *)base; + (struct session_settings_index *) base; assert(part_count == 1); - (void)part_count; + (void) part_count; uint32_t len; - const char *tmp = mp_decode_str(&key, &len); - const char *decoded_key = tt_cstr(tmp, len); - struct tuple *ret = NULL; - struct tuple_format *format = index->format; - for (int i = 0; i < SESSION_SETTING_MODULE_max; ++i) { - if (session_settings_modules[i].get(format, 0, decoded_key, - ITER_EQ, NULL, &ret) != 0) - return -1; - if (ret != NULL) - break; + key = mp_decode_str(&key, &len); + key = tt_cstr(key, len); + struct session_setting_module *module = &session_setting_modules[0]; + struct session_setting_module *end = module + session_setting_type_MAX; + int sid = 0; + for (; module < end; ++module, sid = 0) { + if (session_settings_next_in_module(module, &sid, key, true, + true) == 0) + goto found; } - *result = ret; + *result = NULL; return 0; +found:; + const char *mp_pair; + const char *mp_pair_end; + module->get(sid, &mp_pair, &mp_pair_end); + *result = box_tuple_new(index->format, mp_pair, mp_pair_end); + return *result != NULL ? 0 : -1; } static const struct index_vtab session_settings_index_vtab = { @@ -281,70 +350,64 @@ session_settings_space_execute_update(struct space *space, struct txn *txn, struct tuple **result) { (void)txn; - const char *data = request->key; - uint32_t key_len = mp_decode_array(&data); + struct tuple_format *format = space->format; + const char *old_data, *old_data_end, *new_data; + struct region *region = &fiber()->gc; + size_t used = region_used(region); + int rc = -1, sid = 0; + struct index_def *pk_def = space->index[0]->def; + uint64_t column_mask; + + const char *new_key, *key = request->key; + uint32_t new_size, new_key_len, key_len = mp_decode_array(&key); if (key_len == 0) { diag_set(ClientError, ER_EXACT_MATCH, 1, 0); return -1; } - if (key_len > 1 || mp_typeof(*data) != MP_STR) { + if (key_len > 1 || mp_typeof(*key) != MP_STR) { diag_set(ClientError, ER_KEY_PART_TYPE, 0, "string"); return -1; } - uint32_t len; - const char *tmp = mp_decode_str(&data, &len); - const char *decoded_key = tt_cstr(tmp, len); - - int id; - struct tuple *old_tuple = NULL; - struct tuple_format *format = space->format; - struct session_settings_modules *module = NULL; - for (int i = 0; i < SESSION_SETTING_MODULE_max; ++i) { - module = &session_settings_modules[i]; - if (module->get(format, 0, decoded_key, ITER_EQ, &id, - &old_tuple) != 0) - return -1; - if (old_tuple != NULL) - break; + key = mp_decode_str(&key, &key_len); + key = tt_cstr(key, key_len); + struct session_setting_module *module = &session_setting_modules[0]; + struct session_setting_module *end = module + session_setting_type_MAX; + for (; module < end; ++module, sid = 0) { + if (session_settings_next_in_module(module, &sid, key, true, + true) == 0) + goto found; } - if (old_tuple == NULL) - return 0; - - uint32_t new_size = 0, bsize; - const char *old_data = tuple_data_range(old_tuple, &bsize); - const char *new_data = - xrow_update_execute(request->tuple,request->tuple_end, - old_data, old_data + bsize, format->dict, - &new_size, request->index_base, NULL); + *result = NULL; + return 0; +found: + module->get(sid, &old_data, &old_data_end); + new_data = xrow_update_execute(request->tuple, request->tuple_end, + old_data, old_data_end, format->dict, + &new_size, request->index_base, + &column_mask); if (new_data == NULL) - return -1; + goto finish; + *result = box_tuple_new(format, new_data, new_data + new_size); + if (*result == NULL) + goto finish; - uint32_t array_len = mp_decode_array(&new_data); - if (array_len != 2) - if (format->exact_field_count != array_len) { - diag_set(ClientError, ER_EXACT_FIELD_COUNT, - array_len, format->exact_field_count); - return -1; - } - - if (mp_typeof(*new_data) != MP_STR) { - diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, - space_index(space, 0)->def->name, space_name(space)); - return -1; - } - - const char *value = new_data; - mp_next(&value); - mp_decode_array(&old_data); - uint32_t size_a = mp_sizeof_str(len); - uint32_t size_b = value - new_data; - if (size_a != size_b || memcmp(old_data, new_data, size_a) != 0) { - diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, - space_index(space, 0)->def->name, space_name(space)); - return -1; + mp_decode_array(&new_data); + new_key = mp_decode_str(&new_data, &new_key_len); + if (!key_update_can_be_skipped(pk_def->key_def->column_mask, + column_mask)) { + if (key_len != new_key_len || + memcmp(key, new_key, key_len) != 0) { + diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, + pk_def->name, space_name(space)); + goto finish; + } } - - return module->set(format, id, value, result); + if (module->set(sid, new_data) != 0) + goto finish; + rc = 0; +finish: + region_truncate(region, used); + return rc; } static int diff --git a/src/box/session_settings.h b/src/box/session_settings.h index e7ecbdb08..7415e0eea 100644 --- a/src/box/session_settings.h +++ b/src/box/session_settings.h @@ -29,33 +29,41 @@ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ -#include <stddef.h> -#include <stdint.h> -#include "iterator_type.h" -#if defined(__cplusplus) -extern "C" { -#endif /* defined(__cplusplus) */ - -enum session_setting_module { - SESSION_SETTING_MODULE_max +/** + * Session has settings. Settings belong to different subsystems, + * such as SQL. Each subsystem registers here its session setting + * type and a set of settings with getter and setter functions. + * The self-registration of modules allows session setting code + * not to depend on all the subsystems. + * + * The types should be ordered in alphabetical order, because the + * type list is used by setting iterators. + */ +enum session_setting_type { + session_setting_type_MAX, }; -struct tuple; -struct tuple_format; - -struct session_settings_modules { - enum session_setting_module id; - uint32_t size; - int (*get)(struct tuple_format *format, int id, const char *key, - enum iterator_type type, int *end_id, struct tuple **result); - int (*set)(struct tuple_format *format, int id, const char *value, - struct tuple **result); +struct session_setting_module { + /** + * An array of setting names. All of them should have the + * same prefix. + */ + const char **settings; + /** Count of settings. */ + int setting_count; + /** + * Get a MessagePack encoded pair [name, value] for a + * setting having index @a id. Index is from the settings + * array. + */ + void (*get)(int id, const char **mp_pair, const char **mp_pair_end); + /** + * Set value of a setting by a given @a id from a + * MessagePack encoded buffer. Note, it is not a pair, but + * just value. + */ + int (*set)(int id, const char *mp_value); }; -extern struct session_settings_modules session_settings_modules[]; -extern const struct space_vtab session_settings_space_vtab; - -#if defined(__cplusplus) -} /* extern "C" */ -#endif /* defined(__plusplus) */ +extern struct session_setting_module session_setting_modules[]; diff --git a/src/box/virtual_engine.c b/src/box/virtual_engine.c index 5ec8015c7..36eec8323 100644 --- a/src/box/virtual_engine.c +++ b/src/box/virtual_engine.c @@ -28,10 +28,12 @@ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ -#include "session_settings.h" #include "virtual_engine.h" +#include "tuple.h" #include "schema.h" +extern const struct space_vtab session_settings_space_vtab; + static void virtual_engine_shutdown(struct engine *engine) { ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/3] box: introduce _session_settings system space 2019-12-21 17:59 ` Vladislav Shpilevoy @ 2019-12-26 18:01 ` Mergen Imeev 0 siblings, 0 replies; 13+ messages in thread From: Mergen Imeev @ 2019-12-26 18:01 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches Thank you for review and fixes! I haven't found any issues with them. New patch below. On Sat, Dec 21, 2019 at 06:59:12PM +0100, Vladislav Shpilevoy wrote: > Thanks for the patch! > > I've pushed my review fixes on top of this commit. See it below > and on the branch. If you agree, then squash. Otherwise lets > discuss. There are lots of fixes, so review carefully. In my > commit you can find inlined explanations about some changes. > > Also see 3 comments below inlined into your commit. > > During the review I also thought more about how to implement setting > modules - in a single big namespace of all session settings, each > with individual session settings, or like I proposed and you've done > here, with modules approach. > > Looks like I was wrong, and the only extendible option is to have a > single namespace, sorry. This is because not all settings can be split > into submodules and not all of them will have the same prefix. For > example, setting 'read_only'. I can't find a good prefix for it. > Additionally, some SQL settings may migrate down to box in future, such > as foreign key- and trigger- related. And they also likely not to have > one prefix. > > Nonetheless, I've decided that we can move settings to one namespace > later, as a separate simple ticket. Here we will finish public API. > > We can add individual settings in the current solution too, but it will > look like a module with a single setting. > > Also we need to optimize search. Currently it is fullscan what is > not acceptable when we have a sorted array. But also, a subject for a > separate issue. > > On 19/12/2019 09:32, imeevma@tarantool.org wrote: > > This patch creates _session_settings system space. This space is > > used to view and change session settings. There are no settings at > > the moment, some will be added in the next patch. > > > > Part of #4511 > > --- > > src/box/CMakeLists.txt | 1 + > > src/box/bootstrap.snap | Bin 5921 -> 5975 bytes > > src/box/lua/space.cc | 2 + > > src/box/lua/upgrade.lua | 15 + > > src/box/schema_def.h | 8 + > > src/box/session_settings.c | 409 +++++++++++++++++++++ > > src/box/session_settings.h | 61 +++ > > src/box/virtual_engine.c | 52 ++- > > test/app-tap/tarantoolctl.test.lua | 4 +- > > test/box-py/bootstrap.result | 3 + > > test/box/access_sysview.result | 6 +- > > test/box/alter.result | 5 +- > > ...h-4511-access-settings-from-any-frontend.result | 108 +++++- > > ...4511-access-settings-from-any-frontend.test.lua | 37 ++ > > test/wal_off/alter.result | 2 +- > > 15 files changed, 697 insertions(+), 16 deletions(-) > > create mode 100644 src/box/session_settings.c > > create mode 100644 src/box/session_settings.h > > > > diff --git a/src/box/session_settings.c b/src/box/session_settings.c > > new file mode 100644 > > index 0000000..b5b5db8 > > --- /dev/null > > +++ b/src/box/session_settings.c > > @@ -0,0 +1,409 @@ > > + > > +static int > > +session_settings_iterator_next(struct iterator *iterator, struct tuple **result) > > +{ > > + struct session_settings_iterator *it = > > + (struct session_settings_iterator *)iterator; > > + if (it->module_id >= SESSION_SETTING_MODULE_max || it->module_id < 0) { > > + *result = NULL; > > + return 0; > > + } > > + > > + struct tuple *ret = NULL; > > + int i = it->module_id; > > + int j = it->current_id; > > + enum iterator_type type = it->iterator_type; > > + struct tuple_format *format = it->format; > > + struct session_settings_modules *module; > > + const char *key = it->key; > > + > > + if (!iterator_type_is_reverse(type)) { > > 1. That condition never changes after iteration creation. You could > reuse virtuality of iterator.next to calculate that condition only > once. For that I've split that function in two: next and prev > separately. > > > + for (; i < SESSION_SETTING_MODULE_max; ++i) { > > + module = &session_settings_modules[i]; > > + if (module->get(format, j, key, type, &j, &ret) != 0) > > + return -1; > > + if (ret != NULL) > > + break; > > + j = 0; > > + } > > + *result = ret; > > + if (i == SESSION_SETTING_MODULE_max) { > > + it->module_id = SESSION_SETTING_MODULE_max; > > + it->current_id = 0; > > + return 0; > > + } > > + ++j; > > + if (j >= (int)module->size) { > > + ++i; > > + j = 0; > > + } > > + it->module_id = i; > > + it->current_id = j; > > + return 0; > > + } > > + > > + for (; i >= 0; --i) { > > + module = &session_settings_modules[i]; > > + if (module->get(format, j, key, type, &j, &ret) != 0) > > + return -1; > > + if (ret != NULL) > > + break; > > + if (i > 0) > > + j = session_settings_modules[i - 1].size - 1; > > + } > > + *result = ret; > > + if (i < 0) { > > + it->module_id = -1; > > + it->current_id = 0; > > + return 0; > > + } > > + --j; > > + if (j < 0) { > > + --i; > > + if (i >= 0) > > + j = module[i].size - 1; > > + } > > + it->module_id = i; > > + it->current_id = j; > > + return 0; > > +} > > diff --git a/src/box/session_settings.h b/src/box/session_settings.h > > new file mode 100644 > > index 0000000..e7ecbdb > > --- /dev/null > > +++ b/src/box/session_settings.h > > @@ -0,0 +1,61 @@ > > +#if defined(__cplusplus) > > +extern "C" { > > +#endif /* defined(__cplusplus) */ > > + > > +enum session_setting_module { > > + SESSION_SETTING_MODULE_max > > 2. Usually we do vice versa - lowercase enum names, > and uppercase _MAX. That makes possible to use things > such as STR2ENUM. > > > +}; > > + > > +struct tuple; > > +struct tuple_format; > > + > > +struct session_settings_modules { > > + enum session_setting_module id; > > + uint32_t size; > > + int (*get)(struct tuple_format *format, int id, const char *key, > > + enum iterator_type type, int *end_id, struct tuple **result); > > + int (*set)(struct tuple_format *format, int id, const char *value, > > + struct tuple **result); > > 3. The thing I mostly didn't like here is that you force submodule > to deal with tuple format, to create tuples, to do search by key. > That is a space's and index's task, not of SQL or other submodule. > Moreover, that logic would be duplicated in each submodule. > > I simplified this API so now the session_settings space internals do > all the search, comparisons, tuple creation, etc. And submodules only > provide setting name list, and API to get/set a setting by ID. > > > +}; > > + > > +extern struct session_settings_modules session_settings_modules[]; > > +extern const struct space_vtab session_settings_space_vtab; > > + > > My commit with explanations: > > ================================================================== > > commit 38d1392d20ee90f4bf5418e2402dd5ec589cfb1d > Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> > Date: Sat Dec 21 18:04:14 2019 +0100 > > Review fixes 2/3 > > diff --git a/src/box/session_settings.c b/src/box/session_settings.c > index b5b5db81e..dd1874b33 100644 > --- a/src/box/session_settings.c > +++ b/src/box/session_settings.c > @@ -31,35 +31,49 @@ > #include "session_settings.h" > #include "xrow_update.h" > #include "virtual_engine.h" > +#include "column_mask.h" > #include "session.h" > #include "schema.h" > #include "tuple.h" > #include "xrow.h" > #include "sql.h" > > -struct session_settings_modules > - session_settings_modules[SESSION_SETTING_MODULE_max] = {}; > +struct session_setting_module > + session_setting_modules[session_setting_type_MAX] = {}; > > struct session_settings_index { > /** Base index. Must be the first member. */ > struct index base; > - /** Format to create tuples on the fly. */ > + /** > + * Format of the tuples iterators of this index return. It > + * is stored here so as not to lookup space each time to > + * get a format and create an iterator. > + */ > struct tuple_format *format; > }; > > struct session_settings_iterator { > /** Base iterator. Must be the first member. */ > struct iterator base; > - /** Format of the tuples this iterator returns. */ > + /** > + * Format of the tuples this iterator returns. It is > + * stored here so as not to lookup space each time to get > + * a format for selected tuples. > + */ > struct tuple_format *format; > - /** ID of current module in global list of the modules. */ > + /** > + * ID of the current session settings module in the global > + * list of the modules. > + */ > int module_id; > /** ID of the setting in current module. */ > - int current_id; > + int setting_id; > /** Decoded key. */ > char *key; > - /** Type of iterator. */ > - enum iterator_type iterator_type; > + /** True if the iterator returns only equal keys. */ > + bool is_eq; > + /** True if the iterator should include equal keys. */ > + bool is_including; > ================================================================== > > As you can see, I removed iterator type in favor of a couple of > flags. I've found it easier to use these flags in the code than > compare the type with all the known types each time. > > ================================================================== > }; > > static void > @@ -72,72 +86,114 @@ session_settings_iterator_free(struct iterator *ptr) > } > > static int > -session_settings_iterator_next(struct iterator *iterator, struct tuple **result) > +session_settings_next_in_module(const struct session_setting_module *module, > + int *sid, const char *key, bool is_eq, > + bool is_including) > ================================================================== > > I've moved search by name to the session_settings space. Now > submodules shouldn't care about it. And this is reused by all > modules (even though now there is only one) > > ================================================================== > { > - struct session_settings_iterator *it = > - (struct session_settings_iterator *)iterator; > - if (it->module_id >= SESSION_SETTING_MODULE_max || it->module_id < 0) { > - *result = NULL; > + int i = *sid; > + int count = module->setting_count; > + if (i >= count) > + return -1; > + if (key == NULL) > return 0; > + assert(i >= 0); > + const char **name = &module->settings[i]; > + for (; i < count; ++i, ++name) { > + int cmp = strcmp(*name, key); > + if ((cmp == 0 && is_including) || > + (cmp > 0 && !is_eq)) { > + *sid = i; > + return 0; > + } > } > + *sid = count; > + return -1; > +} > > - struct tuple *ret = NULL; > - int i = it->module_id; > - int j = it->current_id; > - enum iterator_type type = it->iterator_type; > - struct tuple_format *format = it->format; > - struct session_settings_modules *module; > - const char *key = it->key; > - > - if (!iterator_type_is_reverse(type)) { > - for (; i < SESSION_SETTING_MODULE_max; ++i) { > - module = &session_settings_modules[i]; > - if (module->get(format, j, key, type, &j, &ret) != 0) > - return -1; > - if (ret != NULL) > - break; > - j = 0; > - } > - *result = ret; > - if (i == SESSION_SETTING_MODULE_max) { > - it->module_id = SESSION_SETTING_MODULE_max; > - it->current_id = 0; > +static int > +session_settings_prev_in_module(const struct session_setting_module *module, > + int *sid, const char *key, bool is_eq, > + bool is_including) > +{ > + int i = *sid; > + int count = module->setting_count; > + if (i < 0) > + return -1; > + if (key == NULL) > + return 0; > + if (i >= count) > + i = count - 1; > + const char **name = &module->settings[i]; > + for (; i >= 0; --i, --name) { > + int cmp = strcmp(*name, key); > + if ((cmp == 0 && is_including) || > + (cmp < 0 && !is_eq)) { > + *sid = i; > return 0; > } > - ++j; > - if (j >= (int)module->size) { > - ++i; > - j = 0; > + } > + *sid = -1; > + return -1; > +} > + > +static int > +session_settings_iterator_next(struct iterator *iterator, struct tuple **result) > +{ > + struct session_settings_iterator *it = > + (struct session_settings_iterator *)iterator; > + int mid = it->module_id, sid = it->setting_id; > + struct session_setting_module *module; > + const char *key = it->key; > + bool is_including = it->is_including, is_eq = it->is_eq; > + bool is_found = false; > + for (; mid < session_setting_type_MAX; ++mid, sid = 0) { > + module = &session_setting_modules[mid]; > + if (session_settings_next_in_module(module, &sid, key, is_eq, > + is_including) == 0) { > + is_found = true; > + break; > } > - it->module_id = i; > - it->current_id = j; > + } > + it->module_id = mid; > + it->setting_id = sid + 1; > + if (!is_found) { > + *result = NULL; > return 0; > } > + const char *mp_pair, *mp_pair_end; > + module->get(sid, &mp_pair, &mp_pair_end); > + *result = box_tuple_new(it->format, mp_pair, mp_pair_end); > ================================================================== > > Note, I use box_tuple_new(), because iterator API assumes that a > tuple is referenced somewhere, and that box can just bless a tuple > to the userland. Simple tuple_new() does not work here, because it > won't be deleted if anything will go wrong after its creation and > before tuple_bless() in box. > > ================================================================== > + return *result != NULL ? 0 : -1; > +} > > - for (; i >= 0; --i) { > - module = &session_settings_modules[i]; > - if (module->get(format, j, key, type, &j, &ret) != 0) > - return -1; > - if (ret != NULL) > +static int > +session_settings_iterator_prev(struct iterator *iterator, struct tuple **result) > +{ > + struct session_settings_iterator *it = > + (struct session_settings_iterator *)iterator; > + int mid = it->module_id, sid = it->setting_id; > + struct session_setting_module *module; > + const char *key = it->key; > + bool is_including = it->is_including, is_eq = it->is_eq; > + bool is_found = false; > + for (; mid >= 0; --mid, sid = INT_MAX) { > + module = &session_setting_modules[mid]; > + if (session_settings_prev_in_module(module, &sid, key, is_eq, > + is_including) == 0) { > + is_found = true; > break; > - if (i > 0) > - j = session_settings_modules[i - 1].size - 1; > + } > } > - *result = ret; > - if (i < 0) { > - it->module_id = -1; > - it->current_id = 0; > + it->module_id = mid; > + it->setting_id = sid - 1; > + if (!is_found) { > + *result = NULL; > return 0; > } > - --j; > - if (j < 0) { > - --i; > - if (i >= 0) > - j = module[i].size - 1; > - } > - it->module_id = i; > - it->current_id = j; > - return 0; > + const char *mp_pair, *mp_pair_end; > + module->get(sid, &mp_pair, &mp_pair_end); > + *result = box_tuple_new(it->format, mp_pair, mp_pair_end); > + return *result != NULL ? 0 : -1; > } > > static void > @@ -175,15 +231,23 @@ session_settings_index_create_iterator(struct index *base, > return NULL; > } > iterator_create(&it->base, base); > - it->base.next = session_settings_iterator_next; > it->base.free = session_settings_iterator_free; > it->key = decoded_key; > - it->iterator_type = type; > + it->is_eq = type == ITER_EQ || type == ITER_REQ; > + it->is_including = it->is_eq || type == ITER_GE || type == ITER_ALL || > + type == ITER_LE; > it->format = index->format; > - it->module_id = iterator_type_is_reverse(type) ? > - SESSION_SETTING_MODULE_max - 1 : 0; > - it->current_id = iterator_type_is_reverse(type) ? > - session_settings_modules[it->module_id].size - 1 : 0; > + if (!iterator_type_is_reverse(type)) { > + it->base.next = session_settings_iterator_next; > + it->module_id = 0; > + it->setting_id = 0; > + } else { > + it->base.next = session_settings_iterator_prev; > + it->module_id = session_setting_type_MAX - 1; > + struct session_setting_module *module = > + &session_setting_modules[it->module_id]; > + it->setting_id = module->setting_count - 1; > + } > return (struct iterator *)it; > } > > @@ -192,23 +256,28 @@ session_settings_index_get(struct index *base, const char *key, > uint32_t part_count, struct tuple **result) > { > struct session_settings_index *index = > - (struct session_settings_index *)base; > + (struct session_settings_index *) base; > assert(part_count == 1); > - (void)part_count; > + (void) part_count; > uint32_t len; > - const char *tmp = mp_decode_str(&key, &len); > - const char *decoded_key = tt_cstr(tmp, len); > - struct tuple *ret = NULL; > - struct tuple_format *format = index->format; > - for (int i = 0; i < SESSION_SETTING_MODULE_max; ++i) { > - if (session_settings_modules[i].get(format, 0, decoded_key, > - ITER_EQ, NULL, &ret) != 0) > - return -1; > - if (ret != NULL) > - break; > + key = mp_decode_str(&key, &len); > + key = tt_cstr(key, len); > + struct session_setting_module *module = &session_setting_modules[0]; > + struct session_setting_module *end = module + session_setting_type_MAX; > + int sid = 0; > + for (; module < end; ++module, sid = 0) { > + if (session_settings_next_in_module(module, &sid, key, true, > + true) == 0) > + goto found; > } > - *result = ret; > + *result = NULL; > return 0; > +found:; > + const char *mp_pair; > + const char *mp_pair_end; > + module->get(sid, &mp_pair, &mp_pair_end); > + *result = box_tuple_new(index->format, mp_pair, mp_pair_end); > + return *result != NULL ? 0 : -1; > } > > static const struct index_vtab session_settings_index_vtab = { > @@ -281,70 +350,64 @@ session_settings_space_execute_update(struct space *space, struct txn *txn, > struct tuple **result) > { > (void)txn; > - const char *data = request->key; > - uint32_t key_len = mp_decode_array(&data); > + struct tuple_format *format = space->format; > + const char *old_data, *old_data_end, *new_data; > + struct region *region = &fiber()->gc; > + size_t used = region_used(region); > + int rc = -1, sid = 0; > + struct index_def *pk_def = space->index[0]->def; > + uint64_t column_mask; > + > + const char *new_key, *key = request->key; > + uint32_t new_size, new_key_len, key_len = mp_decode_array(&key); > if (key_len == 0) { > diag_set(ClientError, ER_EXACT_MATCH, 1, 0); > return -1; > } > - if (key_len > 1 || mp_typeof(*data) != MP_STR) { > + if (key_len > 1 || mp_typeof(*key) != MP_STR) { > diag_set(ClientError, ER_KEY_PART_TYPE, 0, "string"); > return -1; > } > - uint32_t len; > - const char *tmp = mp_decode_str(&data, &len); > - const char *decoded_key = tt_cstr(tmp, len); > - > - int id; > - struct tuple *old_tuple = NULL; > - struct tuple_format *format = space->format; > - struct session_settings_modules *module = NULL; > - for (int i = 0; i < SESSION_SETTING_MODULE_max; ++i) { > - module = &session_settings_modules[i]; > - if (module->get(format, 0, decoded_key, ITER_EQ, &id, > - &old_tuple) != 0) > - return -1; > - if (old_tuple != NULL) > - break; > + key = mp_decode_str(&key, &key_len); > + key = tt_cstr(key, key_len); > + struct session_setting_module *module = &session_setting_modules[0]; > + struct session_setting_module *end = module + session_setting_type_MAX; > + for (; module < end; ++module, sid = 0) { > + if (session_settings_next_in_module(module, &sid, key, true, > + true) == 0) > + goto found; > } > - if (old_tuple == NULL) > - return 0; > - > - uint32_t new_size = 0, bsize; > - const char *old_data = tuple_data_range(old_tuple, &bsize); > - const char *new_data = > - xrow_update_execute(request->tuple,request->tuple_end, > - old_data, old_data + bsize, format->dict, > - &new_size, request->index_base, NULL); > + *result = NULL; > + return 0; > +found: > + module->get(sid, &old_data, &old_data_end); > + new_data = xrow_update_execute(request->tuple, request->tuple_end, > + old_data, old_data_end, format->dict, > + &new_size, request->index_base, > + &column_mask); > if (new_data == NULL) > - return -1; > + goto finish; > + *result = box_tuple_new(format, new_data, new_data + new_size); > + if (*result == NULL) > + goto finish; > > - uint32_t array_len = mp_decode_array(&new_data); > - if (array_len != 2) > - if (format->exact_field_count != array_len) { > - diag_set(ClientError, ER_EXACT_FIELD_COUNT, > - array_len, format->exact_field_count); > - return -1; > - } > - > - if (mp_typeof(*new_data) != MP_STR) { > - diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, > - space_index(space, 0)->def->name, space_name(space)); > - return -1; > - } > - > - const char *value = new_data; > - mp_next(&value); > - mp_decode_array(&old_data); > - uint32_t size_a = mp_sizeof_str(len); > - uint32_t size_b = value - new_data; > - if (size_a != size_b || memcmp(old_data, new_data, size_a) != 0) { > - diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, > - space_index(space, 0)->def->name, space_name(space)); > - return -1; > + mp_decode_array(&new_data); > + new_key = mp_decode_str(&new_data, &new_key_len); > + if (!key_update_can_be_skipped(pk_def->key_def->column_mask, > + column_mask)) { > + if (key_len != new_key_len || > + memcmp(key, new_key, key_len) != 0) { > + diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, > + pk_def->name, space_name(space)); > + goto finish; > + } > } > - > - return module->set(format, id, value, result); > + if (module->set(sid, new_data) != 0) > + goto finish; > + rc = 0; > +finish: > + region_truncate(region, used); > + return rc; > } > > static int > diff --git a/src/box/session_settings.h b/src/box/session_settings.h > index e7ecbdb08..7415e0eea 100644 > --- a/src/box/session_settings.h > +++ b/src/box/session_settings.h > @@ -29,33 +29,41 @@ > * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > * SUCH DAMAGE. > */ > -#include <stddef.h> > -#include <stdint.h> > -#include "iterator_type.h" > > -#if defined(__cplusplus) > -extern "C" { > -#endif /* defined(__cplusplus) */ > - > -enum session_setting_module { > - SESSION_SETTING_MODULE_max > +/** > + * Session has settings. Settings belong to different subsystems, > + * such as SQL. Each subsystem registers here its session setting > + * type and a set of settings with getter and setter functions. > + * The self-registration of modules allows session setting code > + * not to depend on all the subsystems. > + * > + * The types should be ordered in alphabetical order, because the > + * type list is used by setting iterators. > + */ > +enum session_setting_type { > + session_setting_type_MAX, > }; > > -struct tuple; > -struct tuple_format; > - > -struct session_settings_modules { > - enum session_setting_module id; > - uint32_t size; > - int (*get)(struct tuple_format *format, int id, const char *key, > - enum iterator_type type, int *end_id, struct tuple **result); > - int (*set)(struct tuple_format *format, int id, const char *value, > - struct tuple **result); > +struct session_setting_module { > + /** > + * An array of setting names. All of them should have the > + * same prefix. > + */ > + const char **settings; > + /** Count of settings. */ > + int setting_count; > + /** > + * Get a MessagePack encoded pair [name, value] for a > + * setting having index @a id. Index is from the settings > + * array. > + */ > + void (*get)(int id, const char **mp_pair, const char **mp_pair_end); > + /** > + * Set value of a setting by a given @a id from a > + * MessagePack encoded buffer. Note, it is not a pair, but > + * just value. > + */ > + int (*set)(int id, const char *mp_value); > }; > > -extern struct session_settings_modules session_settings_modules[]; > -extern const struct space_vtab session_settings_space_vtab; > - > -#if defined(__cplusplus) > -} /* extern "C" */ > -#endif /* defined(__plusplus) */ > +extern struct session_setting_module session_setting_modules[]; > diff --git a/src/box/virtual_engine.c b/src/box/virtual_engine.c > index 5ec8015c7..36eec8323 100644 > --- a/src/box/virtual_engine.c > +++ b/src/box/virtual_engine.c > @@ -28,10 +28,12 @@ > * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > * SUCH DAMAGE. > */ > -#include "session_settings.h" > #include "virtual_engine.h" > +#include "tuple.h" > #include "schema.h" > > +extern const struct space_vtab session_settings_space_vtab; > + > static void > virtual_engine_shutdown(struct engine *engine) > { > From 7efcc8500ae9c58ad12ee7eadc8833be70a590c1 Mon Sep 17 00:00:00 2001 From: Mergen Imeev <imeevma@gmail.com> Date: Sat, 30 Nov 2019 12:59:45 +0300 Subject: [PATCH] box: introduce _session_settings system space This patch creates _session_settings system space. This space is used to view and change session settings. There are no settings at the moment, some will be added in the next patch. Part of #4511 diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt index d79d52c..b57e90b 100644 --- a/src/box/CMakeLists.txt +++ b/src/box/CMakeLists.txt @@ -80,6 +80,7 @@ add_library(box STATIC sysview.c blackhole.c virtual_engine.c + session_settings.c vinyl.c vy_stmt.c vy_mem.c diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap index 7ff3aa5..c573eec 100644 Binary files a/src/box/bootstrap.snap and b/src/box/bootstrap.snap differ diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc index f6e96f0..01b58af 100644 --- a/src/box/lua/space.cc +++ b/src/box/lua/space.cc @@ -653,6 +653,8 @@ box_lua_space_init(struct lua_State *L) lua_setfield(L, -2, "SPACE_SEQUENCE_ID"); lua_pushnumber(L, BOX_FUNC_INDEX_ID); lua_setfield(L, -2, "FUNC_INDEX_ID"); + lua_pushnumber(L, BOX_SESSION_SETTINGS_ID); + lua_setfield(L, -2, "SESSION_SETTINGS_ID"); lua_pushnumber(L, BOX_SYSTEM_ID_MIN); lua_setfield(L, -2, "SYSTEM_ID_MIN"); lua_pushnumber(L, BOX_SYSTEM_ID_MAX); diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua index 07f1e03..4dfd571 100644 --- a/src/box/lua/upgrade.lua +++ b/src/box/lua/upgrade.lua @@ -951,8 +951,23 @@ local function drop_func_collation() _func.index.name:alter({parts = {{'name', 'string'}}}) end +local function create_session_settings_space() + local _space = box.space[box.schema.SPACE_ID] + local _index = box.space[box.schema.INDEX_ID] + local format = {} + format[1] = {name='name', type='string'} + format[2] = {name='value', type='any'} + log.info("create space _session_settings") + _space:insert{box.schema.SESSION_SETTINGS_ID, ADMIN, '_session_settings', + 'virtual', 2, {temporary = true}, format} + log.info("create index _session_settings:primary") + _index:insert{box.schema.SESSION_SETTINGS_ID, 0, 'primary', 'tree', + {unique = true}, {{0, 'string'}}} +end + local function upgrade_to_2_3_1() drop_func_collation() + create_session_settings_space() end -------------------------------------------------------------------------------- diff --git a/src/box/schema_def.h b/src/box/schema_def.h index ba870ff..f86cd42 100644 --- a/src/box/schema_def.h +++ b/src/box/schema_def.h @@ -114,6 +114,8 @@ enum { BOX_CK_CONSTRAINT_ID = 364, /** Space id of _func_index. */ BOX_FUNC_INDEX_ID = 372, + /** Space id of _session_settings. */ + BOX_SESSION_SETTINGS_ID = 380, /** End of the reserved range of system spaces. */ BOX_SYSTEM_ID_MAX = 511, BOX_ID_NIL = 2147483647 @@ -277,6 +279,12 @@ enum { BOX_FUNC_INDEX_FUNCTION_ID = 2, }; +/** _session_settings fields. */ +enum { + BOX_SESSION_SETTINGS_FIELD_NAME = 0, + BOX_SESSION_SETTINGS_FIELD_VALUE = 1, +}; + /* * Different objects which can be subject to access * control. diff --git a/src/box/session_settings.c b/src/box/session_settings.c new file mode 100644 index 0000000..dd1874b --- /dev/null +++ b/src/box/session_settings.c @@ -0,0 +1,472 @@ +/* + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY <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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Tarantool-patches] [PATCH 3/3] box: add SQL settings to _session_settings 2019-12-19 8:32 [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view imeevma 2019-12-19 8:32 ` [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine imeevma 2019-12-19 8:32 ` [Tarantool-patches] [PATCH 2/3] box: introduce _session_settings system space imeevma @ 2019-12-19 8:33 ` imeevma 2019-12-21 17:59 ` Vladislav Shpilevoy 2019-12-27 13:45 ` [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view Vladislav Shpilevoy 3 siblings, 1 reply; 13+ messages in thread From: imeevma @ 2019-12-19 8:33 UTC (permalink / raw) To: v.shpilevoy; +Cc: tarantool-patches Hi! Thank you for review! I divided the patch into three parts: creation of engine, creation of system space and addition SQL settings to system space. My answers and third part below. On 12/12/19 2:47 AM, Vladislav Shpilevoy wrote: > 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. > Thanks, fixed. >> --- >> - ['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'. > Thanks, done. >> 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. > Done. >> +}; >> + >> +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. > Thanks, I think I fixed this. >> + >> +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. > Fixed. >> + 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. > I asked this question, but haven't find an answer. If the issue won't be fixed in this patch-set, I will fill an issue. >> + uint32_t len; >> + const char *tmp; >> + const char *data; > > 7. Lets avoid C89 declaration style. > Fixed. >> + >> + 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); > Thanks a lot! Fixed. >> +} >> + >> +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. > Fixed, I think. >> 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. Fixed, I think. New patch: From 71b3e0f017780fe6efcf2d1ccb5e4d46f0d666cd Mon Sep 17 00:00:00 2001 From: Mergen Imeev <imeevma@gmail.com> Date: Sat, 30 Nov 2019 12:59:45 +0300 Subject: [PATCH] box: add SQL settings to _session_settings Part of #4511 @TarantoolBot document Title: _session_settings system space The _session_settings system space used to view or change session settings. This space uses a new engine. This allows us to create tuples on the fly when the get() or select() methods are called. This engine does not support the insert(), replace(), and delete() methods. The only way to change the setting value is update(), which can only be used with the "=" operation. Because space creates a tuple on the fly, it allows us to get a tuple without saving it anywhere. But this means that every time we get a tuple from this system space, it is a new tuple, even if they look the same: tarantool> s = box.space._session_settings tarantool> name = 'sql_default_engine' tarantool> s:get({name}) == s:get({name}) --- - false ... Currently, this space contains only SQL settings, since the only session settings are SQL settings. List of currently available session settings: sql_default_engine sql_defer_foreign_keys sql_full_column_names sql_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'] ... diff --git a/src/box/session_settings.h b/src/box/session_settings.h index e7ecbdb..34d1a25 100644 --- a/src/box/session_settings.h +++ b/src/box/session_settings.h @@ -38,6 +38,7 @@ extern "C" { #endif /* defined(__cplusplus) */ enum session_setting_module { + SESSION_SETTING_MODULE_SQL, SESSION_SETTING_MODULE_max }; diff --git a/src/box/sql.h b/src/box/sql.h index 0fa52fc..3f86568 100644 --- a/src/box/sql.h +++ b/src/box/sql.h @@ -33,6 +33,7 @@ #include <stdbool.h> #include <stdint.h> +#include "iterator_type.h" #if defined(__cplusplus) extern "C" { @@ -70,6 +71,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 +422,51 @@ void vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref, struct tuple *tuple); + +/** + * Return number of SQL session settings. + * + * @retval Number of SQL session settings. + */ +int +sql_session_settings_count(); + +/** + * Matches given key with SQL settings names and returns tuple + * that contains name and value of the setting. + * + * @param format The format of result tuple. + * @param id The ID to start search setting from. + * @param key The key to find the setting. + * @param type Type of iterator to match key with settings name. + * @param end_id The ID of returned setting. + * @param result Result tuple. + * + * @retval 0 on success. + * @retval -1 on error. + */ +int +sql_session_setting_get(struct tuple_format *format, int id, const char *key, + enum iterator_type type, int *end_id, + struct tuple **result); + +/** + * Set new value to SQL session setting. Value given in MsgPack + * format. Returns tuple that contains name and new value of the + * setting. + * + * @param format Format of result tuple. + * @param id ID of SQL setting to set new value. + * @param value MsgPack contains value to set. + * @param result Result tuple. + * + * @retval 0 on success. + * @retval -1 on error. + */ +int +sql_session_setting_set(struct tuple_format *format, int id, const char *value, + struct tuple **result); + #if defined(__cplusplus) } /* extern "C" { */ #endif diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 51cd7ce..995a32e 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,275 @@ 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, +#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, +}; + +int +sql_session_settings_count() +{ + return SQL_SESSION_SETTING_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_SETTING_DEFAULT_ENGINE */ + {"sql_default_engine", FIELD_TYPE_STRING, 0}, + /** SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS */ + {"sql_defer_foreign_keys", FIELD_TYPE_BOOLEAN, SQL_DeferFKs}, + /** SQL_SESSION_SETTING_FULL_COLUMN_NAMES */ + {"sql_full_column_names", FIELD_TYPE_BOOLEAN, SQL_FullColNames}, +#ifndef NDEBUG + /** SQL_SESSION_SETTING_PARSER_TRACE */ + {"sql_parser_trace", FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG}, +#endif + /** SQL_SESSION_SETTING_RECURSIVE_TRIGGERS */ + {"sql_recursive_triggers", FIELD_TYPE_BOOLEAN, SQL_RecTriggers}, + /** SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS */ + {"sql_reverse_unordered_selects", FIELD_TYPE_BOOLEAN, SQL_ReverseOrder}, +#ifndef NDEBUG + /** SQL_SESSION_SETTING_SELECT_TRACE */ + {"sql_select_trace", FIELD_TYPE_BOOLEAN, SQL_SelectTrace}, + /** SQL_SESSION_SETTING_TRACE */ + {"sql_trace", FIELD_TYPE_BOOLEAN, SQL_SqlTrace}, + /** SQL_SESSION_SETTING_VDBE_ADDOPTRACE */ + {"sql_vdbe_addoptrace", FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace}, + /** SQL_SESSION_SETTING_VDBE_DEBUG */ + {"sql_vdbe_debug", FIELD_TYPE_BOOLEAN, + SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace}, + /** SQL_SESSION_SETTING_VDBE_EQP */ + {"sql_vdbe_eqp", FIELD_TYPE_BOOLEAN, SQL_VdbeEQP}, + /** SQL_SESSION_SETTING_VDBE_LISTING */ + {"sql_vdbe_listing", FIELD_TYPE_BOOLEAN, SQL_VdbeListing}, + /** SQL_SESSION_SETTING_VDBE_TRACE */ + {"sql_vdbe_trace", FIELD_TYPE_BOOLEAN, SQL_VdbeTrace}, + /** SQL_SESSION_SETTING_WHERE_TRACE */ + {"sql_where_trace", FIELD_TYPE_BOOLEAN, SQL_WhereTrace}, +#endif +}; + +/** + * Return SQL setting ID that matches the given key and iterator + * type. Search begins from next_id. + * + * @param key Key to match settings name to. + * @param next_id ID to start search from. + * @param type Type of iterator. + * + * @retval setting ID. + */ +static int +sql_session_setting_id_by_name(const char *key, int next_id, + enum iterator_type type) +{ + int id = next_id; + if (key == NULL) + return id; + if (iterator_type_is_reverse(type)) { + for (; id >= 0; --id) { + int compare = strcmp(sql_session_opts[id].name, key); + if (compare == 0 && (type == ITER_REQ || + type == ITER_LE)) + break; + if (compare < 0 && (type == ITER_LT || type == ITER_LE)) + break; + } + } else { + for (; id < SQL_SESSION_SETTING_max; ++id) { + int compare = strcmp(sql_session_opts[id].name, key); + if (compare == 0 && (type == ITER_EQ || + type == ITER_GE || + type == ITER_ALL)) + break; + if (compare > 0 && (type == ITER_GT || + type == ITER_GE || + type == ITER_ALL)) + break; + } + } + return id; +} + +/** + * Return a tuple with the specified format, which contains the + * name and value of the SQL setting with the specified ID. + * + * @param format Format of tuple to return. + * @param id ID of tuple to return. + * @param result[out] Returned tuple. + * + * @retval 0 on success. + * @retval -1 on error. + */ +static int +sql_session_setting_tuple(struct tuple_format *format, int id, + struct tuple **result) +{ + if (id < 0 || id >= SQL_SESSION_SETTING_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)); + /* + * Currently, SQL session settings are of a boolean or + * string type. + */ + bool is_bool = sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN; + if (is_bool) { + size += mp_sizeof_bool(true); + } else { + assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE); + engine = sql_storage_engine_strs[session->sql_default_engine]; + size += mp_sizeof_str(strlen(engine)); + } + + 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 (is_bool) + 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_get(struct tuple_format *format, int id, const char *key, + enum iterator_type type, int *end_id, + struct tuple **result) +{ + int new_id = sql_session_setting_id_by_name(key, id, type); + if (end_id != NULL) + *end_id = new_id; + return sql_session_setting_tuple(format, new_id, result); +} + +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; +} + +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_set(struct tuple_format *format, int id, const char *value, + struct tuple **result) +{ + if (id < 0 || id >= SQL_SESSION_SETTING_max) + return 0; + if (!is_value_type_correct(*value, sql_session_opts[id].field_type)) { + diag_set(ClientError, ER_FIELD_TYPE, "2", + field_type_strs[sql_session_opts[id].field_type]); + return -1; + } + if (sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN) { + if (sql_set_boolean_option(id, mp_decode_bool(&value)) != 0) + return -1; + } else { + assert(sql_session_opts[id].field_type == FIELD_TYPE_STRING); + uint32_t len; + const char *tmp = mp_decode_str(&value, &len); + const char *decoded_value = tt_cstr(tmp, len); + if (sql_set_string_option(id, decoded_value) != 0) + return -1; + } + return sql_session_setting_tuple(format, id, result); +} diff --git a/src/box/sql/main.c b/src/box/sql/main.c index 0b20f21..826cd1c 100644 --- a/src/box/sql/main.c +++ b/src/box/sql/main.c @@ -39,6 +39,7 @@ #include "vdbeInt.h" #include "version.h" #include "box/session.h" +#include "box/session_settings.h" /* * If the following global variable points to a string which is the @@ -398,6 +399,13 @@ sql_init_db(sql **out_db) /* Enable the lookaside-malloc subsystem */ setupLookaside(db, 0, LOOKASIDE_SLOT_SIZE, LOOKASIDE_SLOT_NUMBER); + struct session_settings_modules *sql_settings = + &session_settings_modules[SESSION_SETTING_MODULE_SQL]; + sql_settings->id = SESSION_SETTING_MODULE_SQL; + sql_settings->size = sql_session_settings_count(); + sql_settings->get = sql_session_setting_get; + sql_settings->set = sql_session_setting_set; + *out_db = db; return 0; } diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result index 75d53cf..579a5a1 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,231 @@ 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 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; | --- | ... -s:select() +test_run:cmd('setopt delimiter ""'); | --- - | - [] + | - true | ... -s:select({}, {iterator='EQ'}) + +check_sorting(s, t) | --- - | - [] + | - true | ... -s:select({}, {iterator='ALL'}) +check_sorting(s, t, 'abcde') | --- - | - [] + | - true | ... -s:select({}, {iterator='GE'}) +check_sorting(s, t, 'sql_d') | --- - | - [] + | - true | ... -s:select({}, {iterator='GT'}) +check_sorting(s, t, 'sql_v') | --- - | - [] + | - true | ... -s:select({}, {iterator='REQ'}) +check_sorting(s, t, 'sql_defer_foreign_keys') | --- - | - [] + | - true | ... -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 2 does not match field type: expected + | a number' + | ... +s:update('sql_defer_foreign_keys', {{'-', 'value', 2}}) + | --- + | - error: 'Argument type in operation ''-'' on field 2 does not match field type: expected + | a number' + | ... +s:update('sql_defer_foreign_keys', {{'&', 'value', 2}}) + | --- + | - error: 'Argument type in operation ''&'' on field 2 does not match field type: expected + | a positive integer' + | ... +s:update('sql_defer_foreign_keys', {{'|', 'value', 2}}) + | --- + | - error: 'Argument type in operation ''|'' on field 2 does not match field type: expected + | a positive integer' + | ... +s:update('sql_defer_foreign_keys', {{'^', 'value', 2}}) + | --- + | - error: 'Argument type in operation ''^'' on field 2 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, 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/gh-4511-access-settings-from-any-frontend.test.lua b/test/box/gh-4511-access-settings-from-any-frontend.test.lua index 3304454..0699aec 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,85 @@ 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 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 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, 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'}}) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 3/3] box: add SQL settings to _session_settings 2019-12-19 8:33 ` [Tarantool-patches] [PATCH 3/3] box: add SQL settings to _session_settings imeevma @ 2019-12-21 17:59 ` Vladislav Shpilevoy 2019-12-26 18:07 ` Mergen Imeev 0 siblings, 1 reply; 13+ messages in thread From: Vladislav Shpilevoy @ 2019-12-21 17:59 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches Thanks for the patch! I've pushed my review fixes on top of this commit. See it below and on the branch. If you agree, then squash. Otherwise lets discuss. In my commit you can find inlined explanations about some changes. ================================================================================ commit 0cb8d1f6a337a1eb912ada031a6393bc22616e2e Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Date: Sat Dec 21 18:08:06 2019 +0100 Review fixes 3/3 diff --git a/src/box/errcode.h b/src/box/errcode.h index c660b1c70..11894fccc 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -258,6 +258,7 @@ struct errcode_record { /*203 */_(ER_BOOTSTRAP_READONLY, "Trying to bootstrap a local read-only instance as master") \ /*204 */_(ER_SQL_FUNC_WRONG_RET_COUNT, "SQL expects exactly one argument returned from %s, got %d")\ /*205 */_(ER_FUNC_INVALID_RETURN_TYPE, "Function '%s' returned value of invalid type: expected %s got %s") \ + /*206 */_(ER_SESSION_SETTING_INVALID_VALUE, "Session setting %s expected a value of type %s") \ ================================================================================ I've decided that ER_FIELD_TYPE, used before, does not really fit here. Because it produces a not user-friendly message, and because formally speaking in the space format we said that type of the field is 'any'. So it should have accepted everything. Via that new error I've tried to explain to a user the problem a bit deeper. ================================================================================ /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/session_settings.h b/src/box/session_settings.h index 8166f17f8..25490a7e3 100644 --- a/src/box/session_settings.h +++ b/src/box/session_settings.h @@ -30,9 +30,19 @@ * SUCH DAMAGE. */ -enum session_setting_module { - SESSION_SETTING_MODULE_SQL, - SESSION_SETTING_MODULE_max +/** + * Session has settings. Settings belong to different subsystems, + * such as SQL. Each subsystem registers here its session setting + * type and a set of settings with getter and setter functions. + * The self-registration of modules allows session setting code + * not to depend on all the subsystems. + * + * The types should be ordered in alphabetical order, because the + * type list is used by setting iterators. + */ +enum session_setting_type { + SESSION_SETTING_SQL, + session_setting_type_MAX, }; struct session_setting_module { diff --git a/src/box/sql.c b/src/box/sql.c index f1df55571..cc826177b 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -64,9 +64,14 @@ static const uint32_t default_sql_flags = SQL_ShortColNames | SQL_AutoIndex | SQL_RecTriggers; +extern void +sql_session_settings_init(); + void sql_init() { + sql_session_settings_init(); + default_flags |= default_sql_flags; current_session()->sql_flags |= default_sql_flags; diff --git a/src/box/sql.h b/src/box/sql.h index 3f86568a6..0fa52fc0b 100644 --- a/src/box/sql.h +++ b/src/box/sql.h @@ -33,7 +33,6 @@ #include <stdbool.h> #include <stdint.h> -#include "iterator_type.h" #if defined(__cplusplus) extern "C" { @@ -71,7 +70,6 @@ struct Select; struct Table; struct sql_trigger; struct space_def; -struct tuple_format; /** * Perform parsing of provided expression. This is done by @@ -422,51 +420,6 @@ void vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref, struct tuple *tuple); - -/** - * Return number of SQL session settings. - * - * @retval Number of SQL session settings. - */ -int -sql_session_settings_count(); - -/** - * Matches given key with SQL settings names and returns tuple - * that contains name and value of the setting. - * - * @param format The format of result tuple. - * @param id The ID to start search setting from. - * @param key The key to find the setting. - * @param type Type of iterator to match key with settings name. - * @param end_id The ID of returned setting. - * @param result Result tuple. - * - * @retval 0 on success. - * @retval -1 on error. - */ -int -sql_session_setting_get(struct tuple_format *format, int id, const char *key, - enum iterator_type type, int *end_id, - struct tuple **result); - -/** - * Set new value to SQL session setting. Value given in MsgPack - * format. Returns tuple that contains name and new value of the - * setting. - * - * @param format Format of result tuple. - * @param id ID of SQL setting to set new value. - * @param value MsgPack contains value to set. - * @param result Result tuple. - * - * @retval 0 on success. - * @retval -1 on error. - */ -int -sql_session_setting_set(struct tuple_format *format, int id, const char *value, - struct tuple **result); - #if defined(__cplusplus) } /* extern "C" { */ #endif diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 995a32e8d..a6e3752fc 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -46,7 +46,6 @@ #include <ctype.h> #include "sqlInt.h" #include "vdbeInt.h" -#include "box/tuple.h" #include "tarantoolInt.h" #include "box/box.h" #include "box/ck_constraint.h" @@ -57,6 +56,7 @@ #include "box/schema.h" #include "box/tuple_format.h" #include "box/coll_id_cache.h" +#include "box/session_settings.h" void sql_finish_coding(struct Parse *parse_context) @@ -3272,177 +3272,119 @@ enum { SQL_SESSION_SETTING_VDBE_TRACE, SQL_SESSION_SETTING_WHERE_TRACE, #endif - SQL_SESSION_SETTING_max, + sql_session_setting_MAX, }; -int -sql_session_settings_count() -{ - return SQL_SESSION_SETTING_max; -} +static const char *sql_session_setting_strs[sql_session_setting_MAX] = { + "sql_default_engine", + "sql_defer_foreign_keys", + "sql_full_column_names", +#ifndef NDEBUG + "sql_parser_trace", +#endif + "sql_recursive_triggers", + "sql_reverse_unordered_selects", +#ifndef NDEBUG + "sql_select_trace", + "sql_trace", + "sql_vdbe_addoptrace", + "sql_vdbe_debug", + "sql_vdbe_eqp", + "sql_vdbe_listing", + "sql_vdbe_trace", + "sql_where_trace", +#endif +}; /** * A local structure that allows to establish a connection between - * the name of the parameter, its field type and mask, if it have - * one. + * parameter and its field type and mask, if it has one. */ struct sql_option_metadata { - const char *name; uint32_t field_type; uint32_t mask; }; /** - * Variable that contains names of the SQL session options, their - * field types and mask if they have one or 0 if don't have. + * Variable that contains SQL session option field types and masks + * if they have one or 0 if don't have. * * It is IMPORTANT that these options sorted by name. */ static struct sql_option_metadata sql_session_opts[] = { /** SQL_SESSION_SETTING_DEFAULT_ENGINE */ - {"sql_default_engine", FIELD_TYPE_STRING, 0}, + {FIELD_TYPE_STRING, 0}, /** SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS */ - {"sql_defer_foreign_keys", FIELD_TYPE_BOOLEAN, SQL_DeferFKs}, + {FIELD_TYPE_BOOLEAN, SQL_DeferFKs}, /** SQL_SESSION_SETTING_FULL_COLUMN_NAMES */ - {"sql_full_column_names", FIELD_TYPE_BOOLEAN, SQL_FullColNames}, + {FIELD_TYPE_BOOLEAN, SQL_FullColNames}, #ifndef NDEBUG /** SQL_SESSION_SETTING_PARSER_TRACE */ - {"sql_parser_trace", FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG}, + {FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG}, #endif /** SQL_SESSION_SETTING_RECURSIVE_TRIGGERS */ - {"sql_recursive_triggers", FIELD_TYPE_BOOLEAN, SQL_RecTriggers}, + {FIELD_TYPE_BOOLEAN, SQL_RecTriggers}, /** SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS */ - {"sql_reverse_unordered_selects", FIELD_TYPE_BOOLEAN, SQL_ReverseOrder}, + {FIELD_TYPE_BOOLEAN, SQL_ReverseOrder}, #ifndef NDEBUG /** SQL_SESSION_SETTING_SELECT_TRACE */ - {"sql_select_trace", FIELD_TYPE_BOOLEAN, SQL_SelectTrace}, + {FIELD_TYPE_BOOLEAN, SQL_SelectTrace}, /** SQL_SESSION_SETTING_TRACE */ - {"sql_trace", FIELD_TYPE_BOOLEAN, SQL_SqlTrace}, + {FIELD_TYPE_BOOLEAN, SQL_SqlTrace}, /** SQL_SESSION_SETTING_VDBE_ADDOPTRACE */ - {"sql_vdbe_addoptrace", FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace}, + {FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace}, /** SQL_SESSION_SETTING_VDBE_DEBUG */ - {"sql_vdbe_debug", FIELD_TYPE_BOOLEAN, + {FIELD_TYPE_BOOLEAN, SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace}, /** SQL_SESSION_SETTING_VDBE_EQP */ - {"sql_vdbe_eqp", FIELD_TYPE_BOOLEAN, SQL_VdbeEQP}, + {FIELD_TYPE_BOOLEAN, SQL_VdbeEQP}, /** SQL_SESSION_SETTING_VDBE_LISTING */ - {"sql_vdbe_listing", FIELD_TYPE_BOOLEAN, SQL_VdbeListing}, + {FIELD_TYPE_BOOLEAN, SQL_VdbeListing}, /** SQL_SESSION_SETTING_VDBE_TRACE */ - {"sql_vdbe_trace", FIELD_TYPE_BOOLEAN, SQL_VdbeTrace}, + {FIELD_TYPE_BOOLEAN, SQL_VdbeTrace}, /** SQL_SESSION_SETTING_WHERE_TRACE */ - {"sql_where_trace", FIELD_TYPE_BOOLEAN, SQL_WhereTrace}, + {FIELD_TYPE_BOOLEAN, SQL_WhereTrace}, #endif }; -/** - * Return SQL setting ID that matches the given key and iterator - * type. Search begins from next_id. - * - * @param key Key to match settings name to. - * @param next_id ID to start search from. - * @param type Type of iterator. - * - * @retval setting ID. - */ -static int -sql_session_setting_id_by_name(const char *key, int next_id, - enum iterator_type type) -{ - int id = next_id; - if (key == NULL) - return id; - if (iterator_type_is_reverse(type)) { - for (; id >= 0; --id) { - int compare = strcmp(sql_session_opts[id].name, key); - if (compare == 0 && (type == ITER_REQ || - type == ITER_LE)) - break; - if (compare < 0 && (type == ITER_LT || type == ITER_LE)) - break; - } - } else { - for (; id < SQL_SESSION_SETTING_max; ++id) { - int compare = strcmp(sql_session_opts[id].name, key); - if (compare == 0 && (type == ITER_EQ || - type == ITER_GE || - type == ITER_ALL)) - break; - if (compare > 0 && (type == ITER_GT || - type == ITER_GE || - type == ITER_ALL)) - break; - } - } - return id; -} - -/** - * Return a tuple with the specified format, which contains the - * name and value of the SQL setting with the specified ID. - * - * @param format Format of tuple to return. - * @param id ID of tuple to return. - * @param result[out] Returned tuple. - * - * @retval 0 on success. - * @retval -1 on error. - */ -static int -sql_session_setting_tuple(struct tuple_format *format, int id, - struct tuple **result) +static void +sql_session_setting_get(int id, const char **mp_pair, const char **mp_pair_end) { - if (id < 0 || id >= SQL_SESSION_SETTING_max) { - *result = NULL; - return 0; - } + assert(id >= 0 && id < sql_session_setting_MAX); struct session *session = current_session(); uint32_t flags = session->sql_flags; - uint32_t mask = sql_session_opts[id].mask; - const char *engine = NULL; - /* Tuple format contains two fields - name and value. */ - uint32_t column_count = format->min_field_count; - assert(column_count == 2); - size_t size = mp_sizeof_array(column_count) + - mp_sizeof_str(strlen(sql_session_opts[id].name)); + struct sql_option_metadata *opt = &sql_session_opts[id]; + uint32_t mask = opt->mask; + const char *name = sql_session_setting_strs[id]; + size_t name_len = strlen(name); + size_t engine_len; + const char *engine; + size_t size = mp_sizeof_array(2) + mp_sizeof_str(name_len); /* * Currently, SQL session settings are of a boolean or * string type. */ - bool is_bool = sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN; + bool is_bool = opt->field_type == FIELD_TYPE_BOOLEAN; if (is_bool) { size += mp_sizeof_bool(true); } else { assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE); engine = sql_storage_engine_strs[session->sql_default_engine]; - size += mp_sizeof_str(strlen(engine)); + engine_len = strlen(engine); + size += mp_sizeof_str(engine_len); } - char *pos_ret = static_alloc(size); - assert(pos_ret != NULL); - char *pos = mp_encode_array(pos_ret, column_count); - pos = mp_encode_str(pos, sql_session_opts[id].name, - strlen(sql_session_opts[id].name)); + char *pos = static_alloc(size); + assert(pos != NULL); + char *pos_end = mp_encode_array(pos, 2); + pos_end = mp_encode_str(pos_end, name, name_len); if (is_bool) - pos = mp_encode_bool(pos, (flags & mask) == mask); + pos_end = mp_encode_bool(pos_end, (flags & mask) == mask); else - pos = mp_encode_str(pos, engine, strlen(engine)); - struct tuple *tuple = tuple_new(format, pos_ret, pos_ret + size); - if (tuple == NULL) - return -1; - *result = tuple; - return 0; -} - -int -sql_session_setting_get(struct tuple_format *format, int id, const char *key, - enum iterator_type type, int *end_id, - struct tuple **result) -{ - int new_id = sql_session_setting_id_by_name(key, id, type); - if (end_id != NULL) - *end_id = new_id; - return sql_session_setting_tuple(format, new_id, result); + pos_end = mp_encode_str(pos_end, engine, engine_len); + *mp_pair = pos; + *mp_pair_end = pos_end; } static int @@ -3481,37 +3423,40 @@ sql_set_string_option(int id, const char *value) return 0; } -bool -is_value_type_correct(char mp_type, enum field_type field_type) -{ - if (mp_typeof(mp_type) == MP_BOOL && field_type == FIELD_TYPE_BOOLEAN) - return true; - if (mp_typeof(mp_type) == MP_STR && field_type == FIELD_TYPE_STRING) - return true; - return false; +static int +sql_session_setting_set(int id, const char *mp_value) +{ + assert(id >= 0 && id < sql_session_setting_MAX); + enum mp_type mtype = mp_typeof(*mp_value); + enum field_type stype = sql_session_opts[id].field_type; + uint32_t len; + const char *tmp; + switch(stype) { + case FIELD_TYPE_BOOLEAN: + if (mtype != MP_BOOL) + break; + return sql_set_boolean_option(id, mp_decode_bool(&mp_value)); + case FIELD_TYPE_STRING: + if (mtype != MP_STR) + break; + tmp = mp_decode_str(&mp_value, &len); + tmp = tt_cstr(tmp, len); + return sql_set_string_option(id, tmp); + default: + unreachable(); + } + diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE, + sql_session_setting_strs[id], field_type_strs[stype]); + return -1; } -int -sql_session_setting_set(struct tuple_format *format, int id, const char *value, - struct tuple **result) -{ - if (id < 0 || id >= SQL_SESSION_SETTING_max) - return 0; - if (!is_value_type_correct(*value, sql_session_opts[id].field_type)) { - diag_set(ClientError, ER_FIELD_TYPE, "2", - field_type_strs[sql_session_opts[id].field_type]); - return -1; - } - if (sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN) { - if (sql_set_boolean_option(id, mp_decode_bool(&value)) != 0) - return -1; - } else { - assert(sql_session_opts[id].field_type == FIELD_TYPE_STRING); - uint32_t len; - const char *tmp = mp_decode_str(&value, &len); - const char *decoded_value = tt_cstr(tmp, len); - if (sql_set_string_option(id, decoded_value) != 0) - return -1; - } - return sql_session_setting_tuple(format, id, result); +void +sql_session_settings_init() +{ + struct session_setting_module *module = + &session_setting_modules[SESSION_SETTING_SQL]; + module->settings = sql_session_setting_strs; + module->setting_count = sql_session_setting_MAX; + module->get = sql_session_setting_get; + module->set = sql_session_setting_set; } diff --git a/src/box/sql/main.c b/src/box/sql/main.c index 826cd1c90..0b20f2132 100644 --- a/src/box/sql/main.c +++ b/src/box/sql/main.c @@ -39,7 +39,6 @@ #include "vdbeInt.h" #include "version.h" #include "box/session.h" -#include "box/session_settings.h" /* * If the following global variable points to a string which is the @@ -399,13 +398,6 @@ sql_init_db(sql **out_db) /* Enable the lookaside-malloc subsystem */ setupLookaside(db, 0, LOOKASIDE_SLOT_SIZE, LOOKASIDE_SLOT_NUMBER); - struct session_settings_modules *sql_settings = - &session_settings_modules[SESSION_SETTING_MODULE_SQL]; - sql_settings->id = SESSION_SETTING_MODULE_SQL; - sql_settings->size = sql_session_settings_count(); - sql_settings->get = sql_session_setting_get; - sql_settings->set = sql_session_setting_set; - *out_db = db; return 0; } diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result index 579a5a18f..e86efedb4 100644 --- a/test/box/gh-4511-access-settings-from-any-frontend.result +++ b/test/box/gh-4511-access-settings-from-any-frontend.result @@ -65,16 +65,19 @@ test_run:cmd('setopt delimiter ";"') | - true | ... function check_sorting(ss, ts, key) - local is_right = true local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'} for _, it in pairs(iterators_list) do local view_space = ss:select({key}, {iterator = it}) local test_space = ts:select({key}, {iterator = it}) for key, value in pairs(view_space) do - is_right = is_right and (test_space[key].name == value.name) + if test_space[key].name ~= value.name then + return { + err = 'bad sorting', type = it, + exp = test_space[key].name, got = value.name + } + end end ================================================================================ This change simplifies debug. Because a failed test now prints where exactly it has failed. ================================================================================ end - return is_right end; | --- | ... @@ -85,23 +88,18 @@ test_run:cmd('setopt delimiter ""'); check_sorting(s, t) | --- - | - true | ... check_sorting(s, t, 'abcde') | --- - | - true | ... check_sorting(s, t, 'sql_d') | --- - | - true | ... check_sorting(s, t, 'sql_v') | --- - | - true | ... check_sorting(s, t, 'sql_defer_foreign_keys') | --- - | - true | ... t:drop() @@ -242,12 +240,12 @@ s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}}) | --- | - error: Illegal parameters, field id must be a number or a string | ... -s:update('sql_defer_foreign_keys', {{'=', 1, true}}) +s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}}) | --- | - error: Attempt to modify a tuple field which is part of index 'primary' in space | '_session_settings' | ... -s:update('sql_defer_foreign_keys', {{'=', 'name', true}}) +s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}}) | --- | - error: Attempt to modify a tuple field which is part of index 'primary' in space | '_session_settings' @@ -263,13 +261,13 @@ s:update('sql_defer_foreign_keys', {{'=', 'some text', true}}) s:update('sql_defer_foreign_keys', {{'=', 'value', 1}}) | --- - | - error: 'Tuple field 2 type does not match one required by operation: expected boolean' + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean | ... s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}}) | --- - | - error: 'Tuple field 2 type does not match one required by operation: expected boolean' + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean | ... s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}}) | --- - | - error: 'Tuple field 2 type does not match one required by operation: expected boolean' + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean | ... diff --git a/test/box/gh-4511-access-settings-from-any-frontend.test.lua b/test/box/gh-4511-access-settings-from-any-frontend.test.lua index 0699aec31..9642f681c 100644 --- a/test/box/gh-4511-access-settings-from-any-frontend.test.lua +++ b/test/box/gh-4511-access-settings-from-any-frontend.test.lua @@ -30,16 +30,19 @@ for _,value in s:pairs() do t:insert(value) end test_run:cmd('setopt delimiter ";"') function check_sorting(ss, ts, key) - local is_right = true local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'} for _, it in pairs(iterators_list) do local view_space = ss:select({key}, {iterator = it}) local test_space = ts:select({key}, {iterator = it}) for key, value in pairs(view_space) do - is_right = is_right and (test_space[key].name == value.name) + if test_space[key].name ~= value.name then + return { + err = 'bad sorting', type = it, + exp = test_space[key].name, got = value.name + } + end end end - return is_right end; test_run:cmd('setopt delimiter ""'); @@ -93,8 +96,8 @@ s:update('sql_defer_foreign_keys', {{1, 'value', true}}) s:update('sql_defer_foreign_keys', {{{1}, 'value', true}}) s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}}) -s:update('sql_defer_foreign_keys', {{'=', 1, true}}) -s:update('sql_defer_foreign_keys', {{'=', 'name', true}}) +s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}}) +s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}}) s:update('sql_defer_foreign_keys', {{'=', 3, true}}) s:update('sql_defer_foreign_keys', {{'=', 'some text', true}}) diff --git a/test/box/misc.result b/test/box/misc.result index d2a20307a..004faaaad 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -554,6 +554,7 @@ t; 203: box.error.BOOTSTRAP_READONLY 204: box.error.SQL_FUNC_WRONG_RET_COUNT 205: box.error.FUNC_INVALID_RETURN_TYPE + 206: box.error.SESSION_SETTING_INVALID_VALUE ... test_run:cmd("setopt delimiter ''"); --- ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 3/3] box: add SQL settings to _session_settings 2019-12-21 17:59 ` Vladislav Shpilevoy @ 2019-12-26 18:07 ` Mergen Imeev 0 siblings, 0 replies; 13+ messages in thread From: Mergen Imeev @ 2019-12-26 18:07 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches Thank you for review and fixes! I haven't found any issues with them. New patch below. Also, I added new session setting: sql_full_metadata. On Sat, Dec 21, 2019 at 06:59:10PM +0100, Vladislav Shpilevoy wrote: > Thanks for the patch! > > I've pushed my review fixes on top of this commit. See it below > and on the branch. If you agree, then squash. Otherwise lets > discuss. In my commit you can find inlined explanations about some > changes. > > ================================================================================ > > commit 0cb8d1f6a337a1eb912ada031a6393bc22616e2e > Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> > Date: Sat Dec 21 18:08:06 2019 +0100 > > Review fixes 3/3 > > diff --git a/src/box/errcode.h b/src/box/errcode.h > index c660b1c70..11894fccc 100644 > --- a/src/box/errcode.h > +++ b/src/box/errcode.h > @@ -258,6 +258,7 @@ struct errcode_record { > /*203 */_(ER_BOOTSTRAP_READONLY, "Trying to bootstrap a local read-only instance as master") \ > /*204 */_(ER_SQL_FUNC_WRONG_RET_COUNT, "SQL expects exactly one argument returned from %s, got %d")\ > /*205 */_(ER_FUNC_INVALID_RETURN_TYPE, "Function '%s' returned value of invalid type: expected %s got %s") \ > + /*206 */_(ER_SESSION_SETTING_INVALID_VALUE, "Session setting %s expected a value of type %s") \ > ================================================================================ > > I've decided that ER_FIELD_TYPE, used before, does not > really fit here. Because it produces a not user-friendly > message, and because formally speaking in the space format > we said that type of the field is 'any'. So it should have > accepted everything. Via that new error I've tried to > explain to a user the problem a bit deeper. > > ================================================================================ > > /* > * !IMPORTANT! Please follow instructions at start of the file > diff --git a/src/box/session_settings.h b/src/box/session_settings.h > index 8166f17f8..25490a7e3 100644 > --- a/src/box/session_settings.h > +++ b/src/box/session_settings.h > @@ -30,9 +30,19 @@ > * SUCH DAMAGE. > */ > > -enum session_setting_module { > - SESSION_SETTING_MODULE_SQL, > - SESSION_SETTING_MODULE_max > +/** > + * Session has settings. Settings belong to different subsystems, > + * such as SQL. Each subsystem registers here its session setting > + * type and a set of settings with getter and setter functions. > + * The self-registration of modules allows session setting code > + * not to depend on all the subsystems. > + * > + * The types should be ordered in alphabetical order, because the > + * type list is used by setting iterators. > + */ > +enum session_setting_type { > + SESSION_SETTING_SQL, > + session_setting_type_MAX, > }; > > struct session_setting_module { > diff --git a/src/box/sql.c b/src/box/sql.c > index f1df55571..cc826177b 100644 > --- a/src/box/sql.c > +++ b/src/box/sql.c > @@ -64,9 +64,14 @@ static const uint32_t default_sql_flags = SQL_ShortColNames > | SQL_AutoIndex > | SQL_RecTriggers; > > +extern void > +sql_session_settings_init(); > + > void > sql_init() > { > + sql_session_settings_init(); > + > default_flags |= default_sql_flags; > > current_session()->sql_flags |= default_sql_flags; > diff --git a/src/box/sql.h b/src/box/sql.h > index 3f86568a6..0fa52fc0b 100644 > --- a/src/box/sql.h > +++ b/src/box/sql.h > @@ -33,7 +33,6 @@ > > #include <stdbool.h> > #include <stdint.h> > -#include "iterator_type.h" > > #if defined(__cplusplus) > extern "C" { > @@ -71,7 +70,6 @@ struct Select; > struct Table; > struct sql_trigger; > struct space_def; > -struct tuple_format; > > /** > * Perform parsing of provided expression. This is done by > @@ -422,51 +420,6 @@ void > vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref, > struct tuple *tuple); > > - > -/** > - * Return number of SQL session settings. > - * > - * @retval Number of SQL session settings. > - */ > -int > -sql_session_settings_count(); > - > -/** > - * Matches given key with SQL settings names and returns tuple > - * that contains name and value of the setting. > - * > - * @param format The format of result tuple. > - * @param id The ID to start search setting from. > - * @param key The key to find the setting. > - * @param type Type of iterator to match key with settings name. > - * @param end_id The ID of returned setting. > - * @param result Result tuple. > - * > - * @retval 0 on success. > - * @retval -1 on error. > - */ > -int > -sql_session_setting_get(struct tuple_format *format, int id, const char *key, > - enum iterator_type type, int *end_id, > - struct tuple **result); > - > -/** > - * Set new value to SQL session setting. Value given in MsgPack > - * format. Returns tuple that contains name and new value of the > - * setting. > - * > - * @param format Format of result tuple. > - * @param id ID of SQL setting to set new value. > - * @param value MsgPack contains value to set. > - * @param result Result tuple. > - * > - * @retval 0 on success. > - * @retval -1 on error. > - */ > -int > -sql_session_setting_set(struct tuple_format *format, int id, const char *value, > - struct tuple **result); > - > #if defined(__cplusplus) > } /* extern "C" { */ > #endif > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > index 995a32e8d..a6e3752fc 100644 > --- a/src/box/sql/build.c > +++ b/src/box/sql/build.c > @@ -46,7 +46,6 @@ > #include <ctype.h> > #include "sqlInt.h" > #include "vdbeInt.h" > -#include "box/tuple.h" > #include "tarantoolInt.h" > #include "box/box.h" > #include "box/ck_constraint.h" > @@ -57,6 +56,7 @@ > #include "box/schema.h" > #include "box/tuple_format.h" > #include "box/coll_id_cache.h" > +#include "box/session_settings.h" > > void > sql_finish_coding(struct Parse *parse_context) > @@ -3272,177 +3272,119 @@ enum { > SQL_SESSION_SETTING_VDBE_TRACE, > SQL_SESSION_SETTING_WHERE_TRACE, > #endif > - SQL_SESSION_SETTING_max, > + sql_session_setting_MAX, > }; > > -int > -sql_session_settings_count() > -{ > - return SQL_SESSION_SETTING_max; > -} > +static const char *sql_session_setting_strs[sql_session_setting_MAX] = { > + "sql_default_engine", > + "sql_defer_foreign_keys", > + "sql_full_column_names", > +#ifndef NDEBUG > + "sql_parser_trace", > +#endif > + "sql_recursive_triggers", > + "sql_reverse_unordered_selects", > +#ifndef NDEBUG > + "sql_select_trace", > + "sql_trace", > + "sql_vdbe_addoptrace", > + "sql_vdbe_debug", > + "sql_vdbe_eqp", > + "sql_vdbe_listing", > + "sql_vdbe_trace", > + "sql_where_trace", > +#endif > +}; > > /** > * A local structure that allows to establish a connection between > - * the name of the parameter, its field type and mask, if it have > - * one. > + * parameter and its field type and mask, if it has one. > */ > struct sql_option_metadata > { > - const char *name; > uint32_t field_type; > uint32_t mask; > }; > > /** > - * Variable that contains names of the SQL session options, their > - * field types and mask if they have one or 0 if don't have. > + * Variable that contains SQL session option field types and masks > + * if they have one or 0 if don't have. > * > * It is IMPORTANT that these options sorted by name. > */ > static struct sql_option_metadata sql_session_opts[] = { > /** SQL_SESSION_SETTING_DEFAULT_ENGINE */ > - {"sql_default_engine", FIELD_TYPE_STRING, 0}, > + {FIELD_TYPE_STRING, 0}, > /** SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS */ > - {"sql_defer_foreign_keys", FIELD_TYPE_BOOLEAN, SQL_DeferFKs}, > + {FIELD_TYPE_BOOLEAN, SQL_DeferFKs}, > /** SQL_SESSION_SETTING_FULL_COLUMN_NAMES */ > - {"sql_full_column_names", FIELD_TYPE_BOOLEAN, SQL_FullColNames}, > + {FIELD_TYPE_BOOLEAN, SQL_FullColNames}, > #ifndef NDEBUG > /** SQL_SESSION_SETTING_PARSER_TRACE */ > - {"sql_parser_trace", FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG}, > + {FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG}, > #endif > /** SQL_SESSION_SETTING_RECURSIVE_TRIGGERS */ > - {"sql_recursive_triggers", FIELD_TYPE_BOOLEAN, SQL_RecTriggers}, > + {FIELD_TYPE_BOOLEAN, SQL_RecTriggers}, > /** SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS */ > - {"sql_reverse_unordered_selects", FIELD_TYPE_BOOLEAN, SQL_ReverseOrder}, > + {FIELD_TYPE_BOOLEAN, SQL_ReverseOrder}, > #ifndef NDEBUG > /** SQL_SESSION_SETTING_SELECT_TRACE */ > - {"sql_select_trace", FIELD_TYPE_BOOLEAN, SQL_SelectTrace}, > + {FIELD_TYPE_BOOLEAN, SQL_SelectTrace}, > /** SQL_SESSION_SETTING_TRACE */ > - {"sql_trace", FIELD_TYPE_BOOLEAN, SQL_SqlTrace}, > + {FIELD_TYPE_BOOLEAN, SQL_SqlTrace}, > /** SQL_SESSION_SETTING_VDBE_ADDOPTRACE */ > - {"sql_vdbe_addoptrace", FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace}, > + {FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace}, > /** SQL_SESSION_SETTING_VDBE_DEBUG */ > - {"sql_vdbe_debug", FIELD_TYPE_BOOLEAN, > + {FIELD_TYPE_BOOLEAN, > SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace}, > /** SQL_SESSION_SETTING_VDBE_EQP */ > - {"sql_vdbe_eqp", FIELD_TYPE_BOOLEAN, SQL_VdbeEQP}, > + {FIELD_TYPE_BOOLEAN, SQL_VdbeEQP}, > /** SQL_SESSION_SETTING_VDBE_LISTING */ > - {"sql_vdbe_listing", FIELD_TYPE_BOOLEAN, SQL_VdbeListing}, > + {FIELD_TYPE_BOOLEAN, SQL_VdbeListing}, > /** SQL_SESSION_SETTING_VDBE_TRACE */ > - {"sql_vdbe_trace", FIELD_TYPE_BOOLEAN, SQL_VdbeTrace}, > + {FIELD_TYPE_BOOLEAN, SQL_VdbeTrace}, > /** SQL_SESSION_SETTING_WHERE_TRACE */ > - {"sql_where_trace", FIELD_TYPE_BOOLEAN, SQL_WhereTrace}, > + {FIELD_TYPE_BOOLEAN, SQL_WhereTrace}, > #endif > }; > > -/** > - * Return SQL setting ID that matches the given key and iterator > - * type. Search begins from next_id. > - * > - * @param key Key to match settings name to. > - * @param next_id ID to start search from. > - * @param type Type of iterator. > - * > - * @retval setting ID. > - */ > -static int > -sql_session_setting_id_by_name(const char *key, int next_id, > - enum iterator_type type) > -{ > - int id = next_id; > - if (key == NULL) > - return id; > - if (iterator_type_is_reverse(type)) { > - for (; id >= 0; --id) { > - int compare = strcmp(sql_session_opts[id].name, key); > - if (compare == 0 && (type == ITER_REQ || > - type == ITER_LE)) > - break; > - if (compare < 0 && (type == ITER_LT || type == ITER_LE)) > - break; > - } > - } else { > - for (; id < SQL_SESSION_SETTING_max; ++id) { > - int compare = strcmp(sql_session_opts[id].name, key); > - if (compare == 0 && (type == ITER_EQ || > - type == ITER_GE || > - type == ITER_ALL)) > - break; > - if (compare > 0 && (type == ITER_GT || > - type == ITER_GE || > - type == ITER_ALL)) > - break; > - } > - } > - return id; > -} > - > -/** > - * Return a tuple with the specified format, which contains the > - * name and value of the SQL setting with the specified ID. > - * > - * @param format Format of tuple to return. > - * @param id ID of tuple to return. > - * @param result[out] Returned tuple. > - * > - * @retval 0 on success. > - * @retval -1 on error. > - */ > -static int > -sql_session_setting_tuple(struct tuple_format *format, int id, > - struct tuple **result) > +static void > +sql_session_setting_get(int id, const char **mp_pair, const char **mp_pair_end) > { > - if (id < 0 || id >= SQL_SESSION_SETTING_max) { > - *result = NULL; > - return 0; > - } > + assert(id >= 0 && id < sql_session_setting_MAX); > struct session *session = current_session(); > uint32_t flags = session->sql_flags; > - uint32_t mask = sql_session_opts[id].mask; > - const char *engine = NULL; > - /* Tuple format contains two fields - name and value. */ > - uint32_t column_count = format->min_field_count; > - assert(column_count == 2); > - size_t size = mp_sizeof_array(column_count) + > - mp_sizeof_str(strlen(sql_session_opts[id].name)); > + struct sql_option_metadata *opt = &sql_session_opts[id]; > + uint32_t mask = opt->mask; > + const char *name = sql_session_setting_strs[id]; > + size_t name_len = strlen(name); > + size_t engine_len; > + const char *engine; > + size_t size = mp_sizeof_array(2) + mp_sizeof_str(name_len); > /* > * Currently, SQL session settings are of a boolean or > * string type. > */ > - bool is_bool = sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN; > + bool is_bool = opt->field_type == FIELD_TYPE_BOOLEAN; > if (is_bool) { > size += mp_sizeof_bool(true); > } else { > assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE); > engine = sql_storage_engine_strs[session->sql_default_engine]; > - size += mp_sizeof_str(strlen(engine)); > + engine_len = strlen(engine); > + size += mp_sizeof_str(engine_len); > } > > - char *pos_ret = static_alloc(size); > - assert(pos_ret != NULL); > - char *pos = mp_encode_array(pos_ret, column_count); > - pos = mp_encode_str(pos, sql_session_opts[id].name, > - strlen(sql_session_opts[id].name)); > + char *pos = static_alloc(size); > + assert(pos != NULL); > + char *pos_end = mp_encode_array(pos, 2); > + pos_end = mp_encode_str(pos_end, name, name_len); > if (is_bool) > - pos = mp_encode_bool(pos, (flags & mask) == mask); > + pos_end = mp_encode_bool(pos_end, (flags & mask) == mask); > else > - pos = mp_encode_str(pos, engine, strlen(engine)); > - struct tuple *tuple = tuple_new(format, pos_ret, pos_ret + size); > - if (tuple == NULL) > - return -1; > - *result = tuple; > - return 0; > -} > - > -int > -sql_session_setting_get(struct tuple_format *format, int id, const char *key, > - enum iterator_type type, int *end_id, > - struct tuple **result) > -{ > - int new_id = sql_session_setting_id_by_name(key, id, type); > - if (end_id != NULL) > - *end_id = new_id; > - return sql_session_setting_tuple(format, new_id, result); > + pos_end = mp_encode_str(pos_end, engine, engine_len); > + *mp_pair = pos; > + *mp_pair_end = pos_end; > } > > static int > @@ -3481,37 +3423,40 @@ sql_set_string_option(int id, const char *value) > return 0; > } > > -bool > -is_value_type_correct(char mp_type, enum field_type field_type) > -{ > - if (mp_typeof(mp_type) == MP_BOOL && field_type == FIELD_TYPE_BOOLEAN) > - return true; > - if (mp_typeof(mp_type) == MP_STR && field_type == FIELD_TYPE_STRING) > - return true; > - return false; > +static int > +sql_session_setting_set(int id, const char *mp_value) > +{ > + assert(id >= 0 && id < sql_session_setting_MAX); > + enum mp_type mtype = mp_typeof(*mp_value); > + enum field_type stype = sql_session_opts[id].field_type; > + uint32_t len; > + const char *tmp; > + switch(stype) { > + case FIELD_TYPE_BOOLEAN: > + if (mtype != MP_BOOL) > + break; > + return sql_set_boolean_option(id, mp_decode_bool(&mp_value)); > + case FIELD_TYPE_STRING: > + if (mtype != MP_STR) > + break; > + tmp = mp_decode_str(&mp_value, &len); > + tmp = tt_cstr(tmp, len); > + return sql_set_string_option(id, tmp); > + default: > + unreachable(); > + } > + diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE, > + sql_session_setting_strs[id], field_type_strs[stype]); > + return -1; > } > > -int > -sql_session_setting_set(struct tuple_format *format, int id, const char *value, > - struct tuple **result) > -{ > - if (id < 0 || id >= SQL_SESSION_SETTING_max) > - return 0; > - if (!is_value_type_correct(*value, sql_session_opts[id].field_type)) { > - diag_set(ClientError, ER_FIELD_TYPE, "2", > - field_type_strs[sql_session_opts[id].field_type]); > - return -1; > - } > - if (sql_session_opts[id].field_type == FIELD_TYPE_BOOLEAN) { > - if (sql_set_boolean_option(id, mp_decode_bool(&value)) != 0) > - return -1; > - } else { > - assert(sql_session_opts[id].field_type == FIELD_TYPE_STRING); > - uint32_t len; > - const char *tmp = mp_decode_str(&value, &len); > - const char *decoded_value = tt_cstr(tmp, len); > - if (sql_set_string_option(id, decoded_value) != 0) > - return -1; > - } > - return sql_session_setting_tuple(format, id, result); > +void > +sql_session_settings_init() > +{ > + struct session_setting_module *module = > + &session_setting_modules[SESSION_SETTING_SQL]; > + module->settings = sql_session_setting_strs; > + module->setting_count = sql_session_setting_MAX; > + module->get = sql_session_setting_get; > + module->set = sql_session_setting_set; > } > diff --git a/src/box/sql/main.c b/src/box/sql/main.c > index 826cd1c90..0b20f2132 100644 > --- a/src/box/sql/main.c > +++ b/src/box/sql/main.c > @@ -39,7 +39,6 @@ > #include "vdbeInt.h" > #include "version.h" > #include "box/session.h" > -#include "box/session_settings.h" > > /* > * If the following global variable points to a string which is the > @@ -399,13 +398,6 @@ sql_init_db(sql **out_db) > /* Enable the lookaside-malloc subsystem */ > setupLookaside(db, 0, LOOKASIDE_SLOT_SIZE, LOOKASIDE_SLOT_NUMBER); > > - struct session_settings_modules *sql_settings = > - &session_settings_modules[SESSION_SETTING_MODULE_SQL]; > - sql_settings->id = SESSION_SETTING_MODULE_SQL; > - sql_settings->size = sql_session_settings_count(); > - sql_settings->get = sql_session_setting_get; > - sql_settings->set = sql_session_setting_set; > - > *out_db = db; > return 0; > } > diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result > index 579a5a18f..e86efedb4 100644 > --- a/test/box/gh-4511-access-settings-from-any-frontend.result > +++ b/test/box/gh-4511-access-settings-from-any-frontend.result > @@ -65,16 +65,19 @@ test_run:cmd('setopt delimiter ";"') > | - true > | ... > function check_sorting(ss, ts, key) > - local is_right = true > local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'} > for _, it in pairs(iterators_list) do > local view_space = ss:select({key}, {iterator = it}) > local test_space = ts:select({key}, {iterator = it}) > for key, value in pairs(view_space) do > - is_right = is_right and (test_space[key].name == value.name) > + if test_space[key].name ~= value.name then > + return { > + err = 'bad sorting', type = it, > + exp = test_space[key].name, got = value.name > + } > + end > end > ================================================================================ > > This change simplifies debug. Because a failed test now prints where exactly > it has failed. > > ================================================================================ > end > - return is_right > end; > | --- > | ... > @@ -85,23 +88,18 @@ test_run:cmd('setopt delimiter ""'); > > check_sorting(s, t) > | --- > - | - true > | ... > check_sorting(s, t, 'abcde') > | --- > - | - true > | ... > check_sorting(s, t, 'sql_d') > | --- > - | - true > | ... > check_sorting(s, t, 'sql_v') > | --- > - | - true > | ... > check_sorting(s, t, 'sql_defer_foreign_keys') > | --- > - | - true > | ... > > t:drop() > @@ -242,12 +240,12 @@ s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}}) > | --- > | - error: Illegal parameters, field id must be a number or a string > | ... > -s:update('sql_defer_foreign_keys', {{'=', 1, true}}) > +s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}}) > | --- > | - error: Attempt to modify a tuple field which is part of index 'primary' in space > | '_session_settings' > | ... > -s:update('sql_defer_foreign_keys', {{'=', 'name', true}}) > +s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}}) > | --- > | - error: Attempt to modify a tuple field which is part of index 'primary' in space > | '_session_settings' > @@ -263,13 +261,13 @@ s:update('sql_defer_foreign_keys', {{'=', 'some text', true}}) > > s:update('sql_defer_foreign_keys', {{'=', 'value', 1}}) > | --- > - | - error: 'Tuple field 2 type does not match one required by operation: expected boolean' > + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean > | ... > s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}}) > | --- > - | - error: 'Tuple field 2 type does not match one required by operation: expected boolean' > + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean > | ... > s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}}) > | --- > - | - error: 'Tuple field 2 type does not match one required by operation: expected boolean' > + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean > | ... > diff --git a/test/box/gh-4511-access-settings-from-any-frontend.test.lua b/test/box/gh-4511-access-settings-from-any-frontend.test.lua > index 0699aec31..9642f681c 100644 > --- a/test/box/gh-4511-access-settings-from-any-frontend.test.lua > +++ b/test/box/gh-4511-access-settings-from-any-frontend.test.lua > @@ -30,16 +30,19 @@ for _,value in s:pairs() do t:insert(value) end > > test_run:cmd('setopt delimiter ";"') > function check_sorting(ss, ts, key) > - local is_right = true > local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'} > for _, it in pairs(iterators_list) do > local view_space = ss:select({key}, {iterator = it}) > local test_space = ts:select({key}, {iterator = it}) > for key, value in pairs(view_space) do > - is_right = is_right and (test_space[key].name == value.name) > + if test_space[key].name ~= value.name then > + return { > + err = 'bad sorting', type = it, > + exp = test_space[key].name, got = value.name > + } > + end > end > end > - return is_right > end; > test_run:cmd('setopt delimiter ""'); > > @@ -93,8 +96,8 @@ s:update('sql_defer_foreign_keys', {{1, 'value', true}}) > s:update('sql_defer_foreign_keys', {{{1}, 'value', true}}) > > s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}}) > -s:update('sql_defer_foreign_keys', {{'=', 1, true}}) > -s:update('sql_defer_foreign_keys', {{'=', 'name', true}}) > +s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}}) > +s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}}) > s:update('sql_defer_foreign_keys', {{'=', 3, true}}) > s:update('sql_defer_foreign_keys', {{'=', 'some text', true}}) > > diff --git a/test/box/misc.result b/test/box/misc.result > index d2a20307a..004faaaad 100644 > --- a/test/box/misc.result > +++ b/test/box/misc.result > @@ -554,6 +554,7 @@ t; > 203: box.error.BOOTSTRAP_READONLY > 204: box.error.SQL_FUNC_WRONG_RET_COUNT > 205: box.error.FUNC_INVALID_RETURN_TYPE > + 206: box.error.SESSION_SETTING_INVALID_VALUE > ... > test_run:cmd("setopt delimiter ''"); > --- From 2a3ed37c777c771725f9d29b03483aa372f829d6 Mon Sep 17 00:00:00 2001 From: Mergen Imeev <imeevma@gmail.com> Date: Sat, 30 Nov 2019 12:59:45 +0300 Subject: [PATCH] box: add SQL settings to _session_settings Part of #4511 @TarantoolBot document Title: _session_settings system space The _session_settings system space used to view or change session settings. This space uses a new engine. This allows us to create tuples on the fly when the get() or select() methods are called. This engine does not support the insert(), replace(), and delete() methods. The only way to change the setting value is update(), which can only be used with the "=" operation. Because space creates a tuple on the fly, it allows us to get a tuple without saving it anywhere. But this means that every time we get a tuple from this system space, it is a new tuple, even if they look the same: tarantool> s = box.space._session_settings tarantool> name = 'sql_default_engine' tarantool> s:get({name}) == s:get({name}) --- - false ... Currently, this space contains only SQL settings, since the only session settings are SQL settings. List of currently available session settings: sql_default_engine sql_defer_foreign_keys sql_full_column_names sql_full_metadata sql_recursive_triggers sql_reverse_unordered_selects Debug build also have debug settings that could be obtained from this sysview: sql_parser_trace sql_select_trace sql_trace sql_vdbe_addoptrace sql_vdbe_debug sql_vdbe_eqp sql_vdbe_listing sql_vdbe_trace sql_where_trace Example of usage: tarantool> s = box.space._session_settings -- View session settings values. tarantool> s:get({'sql_default_engine'}) --- - ['sql_default_engine', 'memtx'] ... tarantool> s:select() --- - - ['sql_default_engine', 'memtx'] - ['sql_defer_foreign_keys', false] - ['sql_full_column_names', false] - ['sql_full_metadata', false] - ['sql_recursive_triggers', true] - ['sql_reverse_unordered_selects', false] ... tarantool> s:select('sql_g', {iterator='LE'}) --- - - ['sql_full_column_names', false] - ['sql_full_metadata', false] - ['sql_defer_foreign_keys', false] - ['sql_default_engine', 'memtx'] ... -- Change session setting value. tarantool> s:update('sql_default_engine', {{'=', 'value', 'vinyl'}}) --- - ['sql_default_engine', 'vinyl'] ... diff --git a/src/box/errcode.h b/src/box/errcode.h index c660b1c..11894fc 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -258,6 +258,7 @@ struct errcode_record { /*203 */_(ER_BOOTSTRAP_READONLY, "Trying to bootstrap a local read-only instance as master") \ /*204 */_(ER_SQL_FUNC_WRONG_RET_COUNT, "SQL expects exactly one argument returned from %s, got %d")\ /*205 */_(ER_FUNC_INVALID_RETURN_TYPE, "Function '%s' returned value of invalid type: expected %s got %s") \ + /*206 */_(ER_SESSION_SETTING_INVALID_VALUE, "Session setting %s expected a value of type %s") \ /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/session_settings.h b/src/box/session_settings.h index 7415e0e..25490a7 100644 --- a/src/box/session_settings.h +++ b/src/box/session_settings.h @@ -41,6 +41,7 @@ * type list is used by setting iterators. */ enum session_setting_type { + SESSION_SETTING_SQL, session_setting_type_MAX, }; diff --git a/src/box/sql.c b/src/box/sql.c index f1df555..cc82617 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -64,9 +64,14 @@ static const uint32_t default_sql_flags = SQL_ShortColNames | SQL_AutoIndex | SQL_RecTriggers; +extern void +sql_session_settings_init(); + void sql_init() { + sql_session_settings_init(); + default_flags |= default_sql_flags; current_session()->sql_flags |= default_sql_flags; diff --git a/src/box/sql/build.c b/src/box/sql/build.c index a106dc3..c7c4901 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -57,6 +57,7 @@ #include "box/tuple_format.h" #include "box/coll_id_cache.h" #include "box/user.h" +#include "box/session_settings.h" void sql_finish_coding(struct Parse *parse_context) @@ -3295,3 +3296,224 @@ sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name, *fieldno = i; return 0; } + +/** + * Identifiers of all SQL session setings. The identifier of the + * option is equal to its place in the sorted list of session + * options of current module. + * + * It is IMPORTANT that these options are sorted by name. If this + * is not the case, the result returned by the _session_settings + * space iterator will not be sorted properly. + */ +enum { + SQL_SESSION_SETTING_DEFAULT_ENGINE = 0, + SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS, + SQL_SESSION_SETTING_FULL_COLUMN_NAMES, + SQL_SESSION_SETTING_FULL_METADATA, +#ifndef NDEBUG + SQL_SESSION_SETTING_PARSER_TRACE, +#endif + SQL_SESSION_SETTING_RECURSIVE_TRIGGERS, + SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS, +#ifndef NDEBUG + SQL_SESSION_SETTING_SELECT_TRACE, + SQL_SESSION_SETTING_TRACE, + SQL_SESSION_SETTING_VDBE_ADDOPTRACE, + SQL_SESSION_SETTING_VDBE_DEBUG, + SQL_SESSION_SETTING_VDBE_EQP, + SQL_SESSION_SETTING_VDBE_LISTING, + SQL_SESSION_SETTING_VDBE_TRACE, + SQL_SESSION_SETTING_WHERE_TRACE, +#endif + sql_session_setting_MAX, +}; + +static const char *sql_session_setting_strs[sql_session_setting_MAX] = { + "sql_default_engine", + "sql_defer_foreign_keys", + "sql_full_column_names", + "sql_full_metadata", +#ifndef NDEBUG + "sql_parser_trace", +#endif + "sql_recursive_triggers", + "sql_reverse_unordered_selects", +#ifndef NDEBUG + "sql_select_trace", + "sql_trace", + "sql_vdbe_addoptrace", + "sql_vdbe_debug", + "sql_vdbe_eqp", + "sql_vdbe_listing", + "sql_vdbe_trace", + "sql_where_trace", +#endif +}; + +/** + * A local structure that allows to establish a connection between + * parameter and its field type and mask, if it has one. + */ +struct sql_option_metadata +{ + uint32_t field_type; + uint32_t mask; +}; + +/** + * Variable that contains SQL session option field types and masks + * if they have one or 0 if don't have. + * + * It is IMPORTANT that these options sorted by name. + */ +static struct sql_option_metadata sql_session_opts[] = { + /** SQL_SESSION_SETTING_DEFAULT_ENGINE */ + {FIELD_TYPE_STRING, 0}, + /** SQL_SESSION_SETTING_DEFER_FOREIGN_KEYS */ + {FIELD_TYPE_BOOLEAN, SQL_DeferFKs}, + /** SQL_SESSION_SETTING_FULL_COLUMN_NAMES */ + {FIELD_TYPE_BOOLEAN, SQL_FullColNames}, + /** SQL_SESSION_SETTING_FULL_METADATA */ + {FIELD_TYPE_BOOLEAN, SQL_FullMetadata}, +#ifndef NDEBUG + /** SQL_SESSION_SETTING_PARSER_TRACE */ + {FIELD_TYPE_BOOLEAN, PARSER_TRACE_FLAG}, +#endif + /** SQL_SESSION_SETTING_RECURSIVE_TRIGGERS */ + {FIELD_TYPE_BOOLEAN, SQL_RecTriggers}, + /** SQL_SESSION_SETTING_REVERSE_UNORDERED_SELECTS */ + {FIELD_TYPE_BOOLEAN, SQL_ReverseOrder}, +#ifndef NDEBUG + /** SQL_SESSION_SETTING_SELECT_TRACE */ + {FIELD_TYPE_BOOLEAN, SQL_SelectTrace}, + /** SQL_SESSION_SETTING_TRACE */ + {FIELD_TYPE_BOOLEAN, SQL_SqlTrace}, + /** SQL_SESSION_SETTING_VDBE_ADDOPTRACE */ + {FIELD_TYPE_BOOLEAN, SQL_VdbeAddopTrace}, + /** SQL_SESSION_SETTING_VDBE_DEBUG */ + {FIELD_TYPE_BOOLEAN, + SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace}, + /** SQL_SESSION_SETTING_VDBE_EQP */ + {FIELD_TYPE_BOOLEAN, SQL_VdbeEQP}, + /** SQL_SESSION_SETTING_VDBE_LISTING */ + {FIELD_TYPE_BOOLEAN, SQL_VdbeListing}, + /** SQL_SESSION_SETTING_VDBE_TRACE */ + {FIELD_TYPE_BOOLEAN, SQL_VdbeTrace}, + /** SQL_SESSION_SETTING_WHERE_TRACE */ + {FIELD_TYPE_BOOLEAN, SQL_WhereTrace}, +#endif +}; + +static void +sql_session_setting_get(int id, const char **mp_pair, const char **mp_pair_end) +{ + assert(id >= 0 && id < sql_session_setting_MAX); + struct session *session = current_session(); + uint32_t flags = session->sql_flags; + struct sql_option_metadata *opt = &sql_session_opts[id]; + uint32_t mask = opt->mask; + const char *name = sql_session_setting_strs[id]; + size_t name_len = strlen(name); + size_t engine_len; + const char *engine; + size_t size = mp_sizeof_array(2) + mp_sizeof_str(name_len); + /* + * Currently, SQL session settings are of a boolean or + * string type. + */ + bool is_bool = opt->field_type == FIELD_TYPE_BOOLEAN; + if (is_bool) { + size += mp_sizeof_bool(true); + } else { + assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE); + engine = sql_storage_engine_strs[session->sql_default_engine]; + engine_len = strlen(engine); + size += mp_sizeof_str(engine_len); + } + + char *pos = static_alloc(size); + assert(pos != NULL); + char *pos_end = mp_encode_array(pos, 2); + pos_end = mp_encode_str(pos_end, name, name_len); + if (is_bool) + pos_end = mp_encode_bool(pos_end, (flags & mask) == mask); + else + pos_end = mp_encode_str(pos_end, engine, engine_len); + *mp_pair = pos; + *mp_pair_end = pos_end; +} + +static int +sql_set_boolean_option(int id, bool value) +{ + struct session *session = current_session(); + struct sql_option_metadata *option = &sql_session_opts[id]; + assert(option->field_type == FIELD_TYPE_BOOLEAN); + if (value) + session->sql_flags |= option->mask; + else + session->sql_flags &= ~option->mask; +#ifndef NDEBUG + if (id == SQL_SESSION_SETTING_PARSER_TRACE) { + if (value) + sqlParserTrace(stdout, "parser: "); + else + sqlParserTrace(NULL, NULL); + } +#endif + return 0; +} + +static int +sql_set_string_option(int id, const char *value) +{ + assert(sql_session_opts[id].field_type = FIELD_TYPE_STRING); + assert(id == SQL_SESSION_SETTING_DEFAULT_ENGINE); + (void)id; + enum sql_storage_engine engine = STR2ENUM(sql_storage_engine, value); + if (engine == sql_storage_engine_MAX) { + diag_set(ClientError, ER_NO_SUCH_ENGINE, value); + return -1; + } + current_session()->sql_default_engine = engine; + return 0; +} + +static int +sql_session_setting_set(int id, const char *mp_value) +{ + assert(id >= 0 && id < sql_session_setting_MAX); + enum mp_type mtype = mp_typeof(*mp_value); + enum field_type stype = sql_session_opts[id].field_type; + uint32_t len; + const char *tmp; + switch(stype) { + case FIELD_TYPE_BOOLEAN: + if (mtype != MP_BOOL) + break; + return sql_set_boolean_option(id, mp_decode_bool(&mp_value)); + case FIELD_TYPE_STRING: + if (mtype != MP_STR) + break; + tmp = mp_decode_str(&mp_value, &len); + tmp = tt_cstr(tmp, len); + return sql_set_string_option(id, tmp); + default: + unreachable(); + } + diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE, + sql_session_setting_strs[id], field_type_strs[stype]); + return -1; +} + +void +sql_session_settings_init() +{ + struct session_setting_module *module = + &session_setting_modules[SESSION_SETTING_SQL]; + module->settings = sql_session_setting_strs; + module->setting_count = sql_session_setting_MAX; + module->get = sql_session_setting_get; + module->set = sql_session_setting_set; +} diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result index 75d53cf..4e68129 100644 --- a/test/box/gh-4511-access-settings-from-any-frontend.result +++ b/test/box/gh-4511-access-settings-from-any-frontend.result @@ -45,72 +45,229 @@ s:replace({'sql_defer_foreign_keys', true}) | - error: Session_settings space does not support replace() | ... --- Check get() and select(). They should return nothing for now. -s:get({'a'}) +-- +-- Check select() method of session_settings space. Should work +-- the same way as an ordinary space with an index of the type +-- "TREE". +-- +t = box.schema.space.create('settings', {format = s:format()}) + | --- + | ... +_ = t:create_index('primary') + | --- + | ... +for _,value in s:pairs() do t:insert(value) end + | --- + | ... + +test_run:cmd('setopt delimiter ";"') + | --- + | - true + | ... +function check_sorting(ss, ts, key) + local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'} + for _, it in pairs(iterators_list) do + local view_space = ss:select({key}, {iterator = it}) + local test_space = ts:select({key}, {iterator = it}) + for key, value in pairs(view_space) do + if test_space[key].name ~= value.name then + return { + err = 'bad sorting', type = it, + exp = test_space[key].name, got = value.name + } + end + end + end +end; | --- | ... -s:select() +test_run:cmd('setopt delimiter ""'); | --- - | - [] + | - true | ... -s:select({}, {iterator='EQ'}) + +check_sorting(s, t) | --- - | - [] | ... -s:select({}, {iterator='ALL'}) +check_sorting(s, t, 'abcde') | --- - | - [] | ... -s:select({}, {iterator='GE'}) +check_sorting(s, t, 'sql_d') | --- - | - [] | ... -s:select({}, {iterator='GT'}) +check_sorting(s, t, 'sql_v') | --- - | - [] | ... -s:select({}, {iterator='REQ'}) +check_sorting(s, t, 'sql_defer_foreign_keys') | --- - | - [] | ... -s:select({}, {iterator='LE'}) + +t:drop() | --- - | - [] | ... -s:select({}, {iterator='LT'}) + +-- Check get() method of session_settings space. +s:get({'sql_defer_foreign_keys'}) | --- - | - [] + | - ['sql_defer_foreign_keys', false] | ... -s:select({'a'}, {iterator='EQ'}) +s:get({'sql_recursive_triggers'}) | --- - | - [] + | - ['sql_recursive_triggers', true] | ... -s:select({'a'}, {iterator='ALL'}) +s:get({'sql_reverse_unordered_selects'}) | --- - | - [] + | - ['sql_reverse_unordered_selects', false] | ... -s:select({'a'}, {iterator='GE'}) +s:get({'sql_default_engine'}) | --- - | - [] + | - ['sql_default_engine', 'memtx'] | ... -s:select({'a'}, {iterator='GT'}) +s:get({'abcd'}) | --- - | - [] | ... -s:select({'a'}, {iterator='REQ'}) + +-- Check pairs() method of session_settings space. +t = {} | --- - | - [] | ... -s:select({'a'}, {iterator='LE'}) +for key, value in s:pairs() do table.insert(t, {key, value}) end | --- - | - [] | ... -s:select({'a'}, {iterator='LT'}) +#t == s:count() | --- - | - [] + | - true | ... --- Currently there is nothing to update, but update() should work. -s:update('some_option', {{'=', 'value', true}}) +-- Check update() method of session_settings space. + +-- Correct updates. +s:update('sql_defer_foreign_keys', {{'=', 'value', true}}) + | --- + | - ['sql_defer_foreign_keys', true] + | ... +s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}}) + | --- + | - ['sql_defer_foreign_keys', false] + | ... +s:update('sql_default_engine', {{'=', 2, 'vinyl'}}) + | --- + | - ['sql_default_engine', 'vinyl'] + | ... +s:update('sql_default_engine', {{':', 'value', 1, 5, 'memtx'}}) + | --- + | - ['sql_default_engine', 'memtx'] + | ... +s:update('a', {{'=', 2, 1}}) + | --- + | ... + +-- Inorrect updates. +s:update({{'sql_defer_foreign_keys'}}, {{'=', 'value', true}}) + | --- + | - error: 'Supplied key type of part 0 does not match index part type: expected string' + | ... + +s:update('sql_defer_foreign_keys', {'=', 'value', true}) + | --- + | - error: Illegal parameters, update operation must be an array {op,..} + | ... +s:update('sql_defer_foreign_keys', {{'=', 'value', true}, {'=', 2, true}}) + | --- + | - ['sql_defer_foreign_keys', true] + | ... +s:update('sql_defer_foreign_keys', {{}}) + | --- + | - error: Illegal parameters, update operation must be an array {op,..}, got empty + | array + | ... +s:update('sql_defer_foreign_keys', {{'='}}) + | --- + | - error: Unknown UPDATE operation + | ... +s:update('sql_defer_foreign_keys', {{'=', 'value'}}) + | --- + | - error: Unknown UPDATE operation + | ... +s:update('sql_defer_foreign_keys', {{'=', 'value', true, 1}}) + | --- + | - error: Unknown UPDATE operation + | ... + +s:update('sql_defer_foreign_keys', {{'+', 'value', 2}}) + | --- + | - error: 'Argument type in operation ''+'' on field ''value'' does not match field + | type: expected a number' + | ... +s:update('sql_defer_foreign_keys', {{'-', 'value', 2}}) + | --- + | - error: 'Argument type in operation ''-'' on field ''value'' does not match field + | type: expected a number' + | ... +s:update('sql_defer_foreign_keys', {{'&', 'value', 2}}) + | --- + | - error: 'Argument type in operation ''&'' on field ''value'' does not match field + | type: expected a positive integer' + | ... +s:update('sql_defer_foreign_keys', {{'|', 'value', 2}}) + | --- + | - error: 'Argument type in operation ''|'' on field ''value'' does not match field + | type: expected a positive integer' + | ... +s:update('sql_defer_foreign_keys', {{'^', 'value', 2}}) + | --- + | - error: 'Argument type in operation ''^'' on field ''value'' does not match field + | type: expected a positive integer' + | ... +s:update('sql_defer_foreign_keys', {{'!', 'value', 2}}) + | --- + | - error: Tuple field count 3 does not match space field count 2 + | ... +s:update('sql_defer_foreign_keys', {{'#', 'value', 2}}) + | --- + | - error: Tuple field count 1 does not match space field count 2 + | ... +s:update('sql_defer_foreign_keys', {{1, 'value', true}}) + | --- + | - error: Illegal parameters, update operation name must be a string + | ... +s:update('sql_defer_foreign_keys', {{{1}, 'value', true}}) + | --- + | - error: Illegal parameters, update operation name must be a string + | ... + +s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}}) + | --- + | - error: Illegal parameters, field id must be a number or a string + | ... +s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}}) + | --- + | - error: Attempt to modify a tuple field which is part of index 'primary' in space + | '_session_settings' + | ... +s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}}) + | --- + | - error: Attempt to modify a tuple field which is part of index 'primary' in space + | '_session_settings' + | ... +s:update('sql_defer_foreign_keys', {{'=', 3, true}}) + | --- + | - error: Tuple field count 3 does not match space field count 2 + | ... +s:update('sql_defer_foreign_keys', {{'=', 'some text', true}}) + | --- + | - error: Field 'some text' was not found in the tuple + | ... + +s:update('sql_defer_foreign_keys', {{'=', 'value', 1}}) + | --- + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean + | ... +s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}}) + | --- + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean + | ... +s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}}) | --- + | - error: Session setting sql_defer_foreign_keys expected a value of type boolean | ... diff --git a/test/box/gh-4511-access-settings-from-any-frontend.test.lua b/test/box/gh-4511-access-settings-from-any-frontend.test.lua index 3304454..9642f68 100644 --- a/test/box/gh-4511-access-settings-from-any-frontend.test.lua +++ b/test/box/gh-4511-access-settings-from-any-frontend.test.lua @@ -19,23 +19,88 @@ s:insert({'a', 1}) s:delete({'b'}) s:replace({'sql_defer_foreign_keys', true}) --- Check get() and select(). They should return nothing for now. -s:get({'a'}) -s:select() -s:select({}, {iterator='EQ'}) -s:select({}, {iterator='ALL'}) -s:select({}, {iterator='GE'}) -s:select({}, {iterator='GT'}) -s:select({}, {iterator='REQ'}) -s:select({}, {iterator='LE'}) -s:select({}, {iterator='LT'}) -s:select({'a'}, {iterator='EQ'}) -s:select({'a'}, {iterator='ALL'}) -s:select({'a'}, {iterator='GE'}) -s:select({'a'}, {iterator='GT'}) -s:select({'a'}, {iterator='REQ'}) -s:select({'a'}, {iterator='LE'}) -s:select({'a'}, {iterator='LT'}) - --- Currently there is nothing to update, but update() should work. -s:update('some_option', {{'=', 'value', true}}) +-- +-- Check select() method of session_settings space. Should work +-- the same way as an ordinary space with an index of the type +-- "TREE". +-- +t = box.schema.space.create('settings', {format = s:format()}) +_ = t:create_index('primary') +for _,value in s:pairs() do t:insert(value) end + +test_run:cmd('setopt delimiter ";"') +function check_sorting(ss, ts, key) + local iterators_list = {'ALL', 'REQ', 'EQ', 'GE', 'GT', 'LE', 'LT'} + for _, it in pairs(iterators_list) do + local view_space = ss:select({key}, {iterator = it}) + local test_space = ts:select({key}, {iterator = it}) + for key, value in pairs(view_space) do + if test_space[key].name ~= value.name then + return { + err = 'bad sorting', type = it, + exp = test_space[key].name, got = value.name + } + end + end + end +end; +test_run:cmd('setopt delimiter ""'); + +check_sorting(s, t) +check_sorting(s, t, 'abcde') +check_sorting(s, t, 'sql_d') +check_sorting(s, t, 'sql_v') +check_sorting(s, t, 'sql_defer_foreign_keys') + +t:drop() + +-- Check get() method of session_settings space. +s:get({'sql_defer_foreign_keys'}) +s:get({'sql_recursive_triggers'}) +s:get({'sql_reverse_unordered_selects'}) +s:get({'sql_default_engine'}) +s:get({'abcd'}) + +-- Check pairs() method of session_settings space. +t = {} +for key, value in s:pairs() do table.insert(t, {key, value}) end +#t == s:count() + +-- Check update() method of session_settings space. + +-- Correct updates. +s:update('sql_defer_foreign_keys', {{'=', 'value', true}}) +s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}}) +s:update('sql_default_engine', {{'=', 2, 'vinyl'}}) +s:update('sql_default_engine', {{':', 'value', 1, 5, 'memtx'}}) +s:update('a', {{'=', 2, 1}}) + +-- Inorrect updates. +s:update({{'sql_defer_foreign_keys'}}, {{'=', 'value', true}}) + +s:update('sql_defer_foreign_keys', {'=', 'value', true}) +s:update('sql_defer_foreign_keys', {{'=', 'value', true}, {'=', 2, true}}) +s:update('sql_defer_foreign_keys', {{}}) +s:update('sql_defer_foreign_keys', {{'='}}) +s:update('sql_defer_foreign_keys', {{'=', 'value'}}) +s:update('sql_defer_foreign_keys', {{'=', 'value', true, 1}}) + +s:update('sql_defer_foreign_keys', {{'+', 'value', 2}}) +s:update('sql_defer_foreign_keys', {{'-', 'value', 2}}) +s:update('sql_defer_foreign_keys', {{'&', 'value', 2}}) +s:update('sql_defer_foreign_keys', {{'|', 'value', 2}}) +s:update('sql_defer_foreign_keys', {{'^', 'value', 2}}) +s:update('sql_defer_foreign_keys', {{'!', 'value', 2}}) +s:update('sql_defer_foreign_keys', {{'#', 'value', 2}}) +s:update('sql_defer_foreign_keys', {{1, 'value', true}}) +s:update('sql_defer_foreign_keys', {{{1}, 'value', true}}) + +s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}}) +s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}}) +s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}}) +s:update('sql_defer_foreign_keys', {{'=', 3, true}}) +s:update('sql_defer_foreign_keys', {{'=', 'some text', true}}) + +s:update('sql_defer_foreign_keys', {{'=', 'value', 1}}) +s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}}) +s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}}) diff --git a/test/box/misc.result b/test/box/misc.result index d2a2030..004faaa 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -554,6 +554,7 @@ t; 203: box.error.BOOTSTRAP_READONLY 204: box.error.SQL_FUNC_WRONG_RET_COUNT 205: box.error.FUNC_INVALID_RETURN_TYPE + 206: box.error.SESSION_SETTING_INVALID_VALUE ... test_run:cmd("setopt delimiter ''"); --- ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view 2019-12-19 8:32 [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view imeevma ` (2 preceding siblings ...) 2019-12-19 8:33 ` [Tarantool-patches] [PATCH 3/3] box: add SQL settings to _session_settings imeevma @ 2019-12-27 13:45 ` Vladislav Shpilevoy 3 siblings, 0 replies; 13+ messages in thread From: Vladislav Shpilevoy @ 2019-12-27 13:45 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches LGTM. On 19/12/2019 09:32, 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 > > src/box/CMakeLists.txt | 2 + > src/box/bootstrap.snap | Bin 5921 -> 5975 bytes > src/box/box.cc | 4 + > src/box/lua/space.cc | 2 + > src/box/lua/upgrade.lua | 15 + > src/box/schema_def.h | 8 + > src/box/session_settings.c | 409 +++++++++++++++++++++ > src/box/session_settings.h | 62 ++++ > src/box/sql.h | 47 +++ > src/box/sql/build.c | 273 ++++++++++++++ > src/box/sql/main.c | 8 + > src/box/virtual_engine.c | 134 +++++++ > src/box/virtual_engine.h | 55 +++ > 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 | 275 ++++++++++++++ > ...4511-access-settings-from-any-frontend.test.lua | 103 ++++++ > test/wal_off/alter.result | 2 +- > 20 files changed, 1409 insertions(+), 8 deletions(-) > create mode 100644 src/box/session_settings.c > create mode 100644 src/box/session_settings.h > 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-12-27 13:45 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-19 8:32 [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view imeevma 2019-12-19 8:32 ` [Tarantool-patches] [PATCH 1/3] box: introduce 'virtual' engine imeevma 2019-12-21 17:59 ` Vladislav Shpilevoy 2019-12-26 17:59 ` Mergen Imeev 2019-12-26 4:45 ` Konstantin Osipov 2019-12-27 13:45 ` Vladislav Shpilevoy 2019-12-19 8:32 ` [Tarantool-patches] [PATCH 2/3] box: introduce _session_settings system space imeevma 2019-12-21 17:59 ` Vladislav Shpilevoy 2019-12-26 18:01 ` Mergen Imeev 2019-12-19 8:33 ` [Tarantool-patches] [PATCH 3/3] box: add SQL settings to _session_settings imeevma 2019-12-21 17:59 ` Vladislav Shpilevoy 2019-12-26 18:07 ` Mergen Imeev 2019-12-27 13:45 ` [Tarantool-patches] [PATCH 0/3] Introduce _session_setting system view Vladislav Shpilevoy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox