[Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings

[Chris Sosnin <k.sosnin@tarantool.org>]

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
SETTING SET.

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 SETTING SET command:
`box.execute([[SETTING SET "<setting_name>" = <new_value>]])`.
Note, that this query is case sensitive so the name must be quoted.
Also, SETTING SET doesn't prove any implicit casts, so <new_value> must
be of the type corresponding to the setting being updated.

Example:
```
tarantool> box.execute([[setting set "sql_default_engine" = 'memtx']])
---
- row_count: 1
...

tarantool> box.execute([[setting set "sql_defer_foreign_keys" = true]])
---
- row_count: 1
...

```
---
 extra/mkkeywordhash.c              |  1 +
 src/box/sql/build.c                | 25 +++++++++++++++
 src/box/sql/parse.y                |  5 +++
 src/box/sql/sqlInt.h               | 11 +++++++
 src/box/sql/vdbe.c                 | 50 ++++++++++++++++++++++++++++++
 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..887853529 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -159,6 +159,7 @@ static Keyword aKeywordTable[] = {
   { "SCALAR",                 "TK_SCALAR",      true  },
   { "SELECT",                 "TK_SELECT",      true  },
   { "SET",                    "TK_SET",         true  },
+  { "SETTING",                "TK_SETTING",     false },
   { "SIMPLE",                 "TK_SIMPLE",      true  },
   { "START",                  "TK_START",       true  },
   { "STRING",                 "TK_STRING_KW",   true  },
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);
+	if (index >= 0) {
+		int target = ++parse_context->nMem;
+		sqlExprCode(parse_context, expr, target);
+		sqlVdbeAddOp2(vdbe, OP_SetSetting, index, target);
+		return;
+	}
+	diag_set(ClientError, ER_SQL_PARSER_GENERIC,
+		 tt_sprintf("Session setting %s doesn't exist", key));
+abort:
+	parse_context->is_aborted = true;
+	return;
+}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 1a0e89703..56977b003 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1541,6 +1541,11 @@ cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y).   {
   sql_drop_index(pParse);
 }
 
+///////////////////////////// The SETTING SET command ////////////////////////
+cmd ::= SETTING SET nm(X) EQ expr(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 1579cc92e..a700dd9c9 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4513,4 +4513,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..d6b753a51 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,55 @@ case OP_IncMaxid: {
 	break;
 }
 
+/* Opcode: SetSetting P1 P2 * * *
+ *
+ * Set the new value of the session setting. P1 is the id of the
+ * setting in the session_settings array, P2 is the register
+ * holding a value.
+ */
+case OP_SetSetting: {
+	int sid = pOp->p1;
+	pIn1 = &aMem[pOp->p2];
+	assert(sid >= 0 && sid < SESSION_SETTING_COUNT);
+	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..28c79379b 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([[setting set "sql_default_engine" = 'vinyl']])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_default_engine').value
+ | ---
+ | - vinyl
+ | ...
+box.execute([[setting set "sql_default_engine" = 'memtx']])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_default_engine').value
+ | ---
+ | - memtx
+ | ...
+box.execute([[setting set "sql_defer_foreign_keys" = true]])
+ | ---
+ | - row_count: 1
+ | ...
+s:get('sql_defer_foreign_keys').value
+ | ---
+ | - true
+ | ...
+box.execute([[setting set "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([[setting set "sql_def_engine" = true]])
+ | ---
+ | - null
+ | - Session setting sql_def_engine doesn't exist
+ | ...
+box.execute([[setting set "sql_default_engine" = true]])
+ | ---
+ | - null
+ | - Session setting sql_default_engine expected a value of type string
+ | ...
+box.execute([[setting set "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..c477139f9 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([[setting set "sql_default_engine" = 'vinyl']])
+s:get('sql_default_engine').value
+box.execute([[setting set "sql_default_engine" = 'memtx']])
+s:get('sql_default_engine').value
+box.execute([[setting set "sql_defer_foreign_keys" = true]])
+s:get('sql_defer_foreign_keys').value
+box.execute([[setting set "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([[setting set "sql_def_engine" = true]])
+box.execute([[setting set "sql_default_engine" = true]])
+box.execute([[setting set "sql_defer_foreign_keys" = 'true']])
-- 
2.21.1 (Apple Git-122.3)