[Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings
Chris Sosnin
k.sosnin at tarantool.org
Fri Apr 10 18:40:03 MSK 2020
> > diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> > index a00da31f9..cdcf8b6d8 100644
> > --- a/src/box/sql/build.c
> > +++ b/src/box/sql/build.c
> > @@ -3481,3 +3481,28 @@ sql_session_settings_init()
> > setting->set = sql_session_setting_set;
> > }
> > }
> > +
> > +void
> > +sql_setting_set(struct Parse *parse_context, struct Token *name,
> > + struct Expr *expr)
> > +{
> > + struct Vdbe *vdbe = sqlGetVdbe(parse_context);
> > + if (vdbe == NULL)
> > + goto abort;
> > + sqlVdbeCountChanges(vdbe);
> > + char *key = sql_name_from_token(parse_context->db, name);
> > + if (key == NULL)
> > + goto abort;
> > + int index = session_setting_find(key);
>
> I agree with Nikita. Better make this call part of OP_SetSetting.
I agree, fixed:
+
+void
+sql_setting_set(struct Parse *parse_context, struct Token *name,
+ struct Expr *expr)
+{
+ struct Vdbe *vdbe = sqlGetVdbe(parse_context);
+ if (vdbe == NULL)
+ goto abort;
+ sqlVdbeCountChanges(vdbe);
+ char *key = sql_name_from_token(parse_context->db, name);
+ if (key == NULL)
+ goto abort;
+ int target = ++parse_context->nMem;
+ sqlExprCode(parse_context, expr, target);
+ sqlVdbeAddOp4(vdbe, OP_SetSession, target, 0, 0, key, P4_DYNAMIC);
+ return;
+abort:
+ parse_context->is_aborted = true;
+ return;
+}
+/* Opcode: SetSession P1 * * P4 *
+ *
+ * Set new value of the session setting. P4 is the name of the
+ * setting being updated, P1 is the register holding a value.
+ */
+case OP_SetSession: {
+ assert(pOp->p4type == P4_DYNAMIC);
+ const char *setting_name = pOp->p4.z;
+ int sid = session_setting_find(setting_name);
+ if (sid < 0) {
+ diag_set(ClientError, ER_NO_SUCH_SESSION_SETTING, setting_name);
+ goto abort_due_to_error;
+ }
+ pIn1 = &aMem[pOp->p1];
...
> Talking of the syntax, I don't have a strong opinion here
> regarding any particular option. I only want the new statement
> be short, and not involving _session_settings manipulations.
I changed it to the following (the main problem with reserved word persists):
+///////////////////////////// The SET SESSION command ////////////////////////
+//
+cmd ::= SET SESSION nm(X) EQ term(Y). {
+ sql_setting_set(pParse,&X,Y.pExpr);
+}
+
I also changed the 3rd commit (Lua frontend) in the series to introduce new error
code (it's present in the diff above):
+ /*213 */_(ER_NO_SUCH_SESSION_SETTING, "Session setting %s doesn't exist") \
as long as searching is moved to vdbe.c and raising parsing error is not correct.
You can see the whole patch below:
===============================================================================
Currently if a user wants to change session setting with SQL, one has
to execute UPDATE query like:
[[UPDATE "_session_settings" SET "value" = true WHERE "name" = 'name']]
However, direct access to system spaces isn't considered to be a good practice.
To avoid that and a bit simplify user's life, we introduce SQL shortcut command
SET SESSION.
Closes #4711
@TarantoolBot document
Title: API for accessing _session_settings space.
There are two ways of updating values of session settings:
via Lua and SQL.
Lua:
box.session.settings is a table, which is always accessible
to user. The syntax is the following:
`box.session.settings.<setting_name> = <new_value>`.
Example of usage:
```
tarantool> box.session.settings.sql_default_engine
---
- memtx
...
tarantool> box.session.settings.sql_default_engine = 'vinyl'
---
...
```
The table itself represents the (unordered) result of select
from _session_settings space.
SQL:
Instead of typing long UPDATE query one can use the SET SESSION command:
`box.execute([[SET SESSION "<setting_name>" = <new_value>]])`.
Note, that this query is case sensitive so the name must be quoted.
Also, SET SESSION doesn't provide any implicit casts, so <new_value> must
be of the type corresponding to the setting being updated.
Example:
```
tarantool> box.execute([[set session "sql_default_engine" = 'memtx']])
---
- row_count: 1
...
tarantool> box.execute([[set session "sql_defer_foreign_keys" = true]])
---
- row_count: 1
...
```
---
extra/mkkeywordhash.c | 1 +
src/box/sql/build.c | 20 +++++++++++
src/box/sql/parse.y | 6 ++++
src/box/sql/sqlInt.h | 11 ++++++
src/box/sql/vdbe.c | 54 ++++++++++++++++++++++++++++++
test/box/session_settings.result | 49 +++++++++++++++++++++++++++
test/box/session_settings.test.lua | 13 +++++++
7 files changed, 154 insertions(+)
diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
index 006285622..dd42c8f5f 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -158,6 +158,7 @@ static Keyword aKeywordTable[] = {
{ "SAVEPOINT", "TK_SAVEPOINT", true },
{ "SCALAR", "TK_SCALAR", true },
{ "SELECT", "TK_SELECT", true },
+ { "SESSION", "TK_SESSION", false },
{ "SET", "TK_SET", true },
{ "SIMPLE", "TK_SIMPLE", true },
{ "START", "TK_START", true },
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index a00da31f9..aecba4177 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3481,3 +3481,23 @@ sql_session_settings_init()
setting->set = sql_session_setting_set;
}
}
+
+void
+sql_setting_set(struct Parse *parse_context, struct Token *name,
+ struct Expr *expr)
+{
+ struct Vdbe *vdbe = sqlGetVdbe(parse_context);
+ if (vdbe == NULL)
+ goto abort;
+ sqlVdbeCountChanges(vdbe);
+ char *key = sql_name_from_token(parse_context->db, name);
+ if (key == NULL)
+ goto abort;
+ int target = ++parse_context->nMem;
+ sqlExprCode(parse_context, expr, target);
+ sqlVdbeAddOp4(vdbe, OP_SetSession, target, 0, 0, key, P4_DYNAMIC);
+ return;
+abort:
+ parse_context->is_aborted = true;
+ return;
+}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 1a0e89703..995875566 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1541,6 +1541,12 @@ cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y). {
sql_drop_index(pParse);
}
+///////////////////////////// The SET SESSION command ////////////////////////
+//
+cmd ::= SET SESSION nm(X) EQ term(Y). {
+ sql_setting_set(pParse,&X,Y.pExpr);
+}
+
///////////////////////////// The PRAGMA command /////////////////////////////
//
cmd ::= PRAGMA nm(X). {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 2d6bad424..e45a7671d 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4517,4 +4517,15 @@ int
sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
uint32_t *fieldno);
+/**
+ * Create VDBE instructions to set the new value of the session setting.
+ *
+ * @param parse_context Parsing context.
+ * @param name Name of the session setting.
+ * @param value New value of the session setting.
+ */
+void
+sql_setting_set(struct Parse *parse_context, struct Token *name,
+ struct Expr *value);
+
#endif /* sqlINT_H */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index e8a029a8a..c3256c15a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -55,6 +55,7 @@
#include "box/schema.h"
#include "box/space.h"
#include "box/sequence.h"
+#include "box/session_settings.h"
/*
* Invoke this macro on memory cells just prior to changing the
@@ -5252,6 +5253,59 @@ case OP_IncMaxid: {
break;
}
+/* Opcode: SetSession P1 * * P4 *
+ *
+ * Set new value of the session setting. P4 is the name of the
+ * setting being updated, P1 is the register holding a value.
+ */
+case OP_SetSession: {
+ assert(pOp->p4type == P4_DYNAMIC);
+ const char *setting_name = pOp->p4.z;
+ int sid = session_setting_find(setting_name);
+ if (sid < 0) {
+ diag_set(ClientError, ER_NO_SUCH_SESSION_SETTING, setting_name);
+ goto abort_due_to_error;
+ }
+ pIn1 = &aMem[pOp->p1];
+ struct session_setting *setting = &session_settings[sid];
+ switch (setting->field_type) {
+ case FIELD_TYPE_BOOLEAN: {
+ if ((pIn1->flags & MEM_Bool) == 0)
+ goto invalid_type;
+ bool value = pIn1->u.b;
+ size_t size = mp_sizeof_bool(value);
+ char *mp_value = (char *) static_alloc(size);
+ mp_encode_bool(mp_value, value);
+ if (setting->set(sid, mp_value) != 0)
+ goto abort_due_to_error;
+ break;
+ }
+ case FIELD_TYPE_STRING: {
+ if ((pIn1->flags & MEM_Str) == 0)
+ goto invalid_type;
+ const char *str = pIn1->z;
+ uint32_t size = mp_sizeof_str(pIn1->n);
+ char *mp_value = (char *) static_alloc(size);
+ if (mp_value == NULL) {
+ diag_set(OutOfMemory, size, "static_alloc", "mp_value");
+ goto abort_due_to_error;
+ }
+ mp_encode_str(mp_value, str, pIn1->n);
+ if (setting->set(sid, mp_value) != 0)
+ goto abort_due_to_error;
+ break;
+ }
+ default:
+ invalid_type:
+ diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE,
+ session_setting_strs[sid],
+ field_type_strs[setting->field_type]);
+ goto abort_due_to_error;
+ }
+ p->nChange++;
+ break;
+}
+
/* Opcode: Noop * * * * *
*
* Do nothing. This instruction is often useful as a jump
diff --git a/test/box/session_settings.result b/test/box/session_settings.result
index b32a0becb..b82bdda94 100644
--- a/test/box/session_settings.result
+++ b/test/box/session_settings.result
@@ -339,6 +339,39 @@ settings.sql_defer_foreign_keys
| - false
| ...
+box.execute([[set session "sql_default_engine" = 'vinyl']])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_default_engine').value
+ | ---
+ | - vinyl
+ | ...
+box.execute([[set session "sql_default_engine" = 'memtx']])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_default_engine').value
+ | ---
+ | - memtx
+ | ...
+box.execute([[set session "sql_defer_foreign_keys" = true]])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_defer_foreign_keys').value
+ | ---
+ | - true
+ | ...
+box.execute([[set session "sql_defer_foreign_keys" = false]])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_defer_foreign_keys').value
+ | ---
+ | - false
+ | ...
+
settings.sql_default_engine = true
| ---
| - error: Session setting sql_default_engine expected a value of type string
@@ -359,3 +392,19 @@ box.session.settings.sql_default_engine = str
| ---
| - error: Failed to allocate 20483 bytes in static_alloc for mp_value
| ...
+
+box.execute([[set session "sql_def_engine" = true]])
+ | ---
+ | - null
+ | - Session setting sql_def_engine doesn't exist
+ | ...
+box.execute([[set session "sql_default_engine" = true]])
+ | ---
+ | - null
+ | - Session setting sql_default_engine expected a value of type string
+ | ...
+box.execute([[set session "sql_defer_foreign_keys" = 'true']])
+ | ---
+ | - null
+ | - Session setting sql_defer_foreign_keys expected a value of type boolean
+ | ...
diff --git a/test/box/session_settings.test.lua b/test/box/session_settings.test.lua
index 440bef7ce..d4aac0286 100644
--- a/test/box/session_settings.test.lua
+++ b/test/box/session_settings.test.lua
@@ -132,9 +132,22 @@ s:get('sql_defer_foreign_keys').value
s:update('sql_defer_foreign_keys', {{'=', 2, false}})
settings.sql_defer_foreign_keys
+box.execute([[set session "sql_default_engine" = 'vinyl']])
+s:get('sql_default_engine').value
+box.execute([[set session "sql_default_engine" = 'memtx']])
+s:get('sql_default_engine').value
+box.execute([[set session "sql_defer_foreign_keys" = true]])
+s:get('sql_defer_foreign_keys').value
+box.execute([[set session "sql_defer_foreign_keys" = false]])
+s:get('sql_defer_foreign_keys').value
+
settings.sql_default_engine = true
settings.sql_defer_foreign_keys = 'false'
settings.sql_parser_debug = 'string'
str = string.rep('a', 20 * 1024)
box.session.settings.sql_default_engine = str
+
+box.execute([[set session "sql_def_engine" = true]])
+box.execute([[set session "sql_default_engine" = true]])
+box.execute([[set session "sql_defer_foreign_keys" = 'true']])
--
2.21.1 (Apple Git-122.3)
More information about the Tarantool-patches
mailing list