On 17 Mar 2020, at 23:12, = Nikita Pettik <korablev@tarantool.org> wrote:

On 17 Mar 20:26, Chris Sosnin = wrote:

On 16 Mar = 2020, at 20:02, Nikita Pettik <korablev@tarantool.org> wrote:

On 17 Feb 15:12, Chris Sosnin wrote:
Currently if a user wants to change session = setting with sql, he has
to execute non-obvious query, = thus, we introduce a more native way to
do this.

It is not about non-obvious = queries, but it is all about documentation:
the better = feature is described, the clearer its usage turns out to be
for user.

Should I = change the commit message? I though this patchset is about = simplifying
the way session settings are updated?

I would say:

Currently if a user wants to change session settings via SQL, = one has
to execute = UPDATE query like:
[[UPDATE "_session_settings" SET "value" =3D true WHERE = "name" =3D 'name']]
However, direct access to system spaces isn't considered to = be good practice.
To avoid that and a bit simplify user's life, we introduce = SQL shortcut command
SET with following syntax: SET <setting_name> =3D = <value>. <setting_name> is
supposed to be name of setting (note that it is uppercased as = any other
non-quoted = indentifiers in SQL) and <value> - value of corresponding setting = to
be set; = <value> should be either literal or binding marker. Also it is = worth
noting that = SET doesn't provide any implicit casts, so <value> must be of = the
type = corresponding to the setting being updated.

Example:
...

^ It is up to you.

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.

SQL:
Instead of typing long = UPDATE query one can use the SET statement:
`box.execute([[SET "<setting_name>" =3D = <new_value>]])`.
Note, that this query is case = sensitive so the name must be quoted.

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

tarantool> box.execute([[set "sql_defer_foreign_keys" =3D = true]])

Why didn't you = consider show/get command to retrieve setting value?
Otherwise, you simplify way to set session local options, but = doesn't
provide the same way to extract current values.

SELECT * FROM = =E2=80=9C_session_settings=E2=80=9D is simple enough, isn=E2=80=99t = it?

In this case you'll get list of all settings. To get specific = one
we should use = this query:

SELECT value = FROM "_session_settings" WHERE "name" =3D 'xxx'

I do not insist on implementing = GET/SHOW SQL syntax now tho.

diff --git = a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index = d1fcf4761..3ffae5970 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4510,4 +4510,15 = @@ int
sql_fieldno_by_name(struct Parse *parse_context, = struct Expr *field_name,
uint32_t = *fieldno);

diff --git a/src/box/sql/vdbe.c = b/src/box/sql/vdbe.c
index 620d74e66..c81486fa6 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
@@ -5248,6 +5249,55 @@ case = OP_IncMaxid: {
break;
}

+/* Opcode: Set 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_Set: {

Please, use more specific opcode name. Like OP_SettingSet.

Didn=E2=80=99t we accept = =E2=80=99set=E2=80=99 syntax?

SET is okay, but I don't mind = SET SETTING as well. OP_Set is too general
name - SET keyword is also part of UPDATE syntax, so it may = confuse other
developers. = Except for this nit patch LGTM.