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

[Nikita Pettik <korablev@tarantool.org>]

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.
 
> 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>" = <new_value>]])`.
> Note, that this query is case sensitive so the name must be quoted.
> 
> Example:
> ```
> tarantool> box.execute([[set "sql_default_engine" = 'memtx']])
> ---
> - row_count: 1
> ...
> 
> tarantool> box.execute([[set "sql_defer_foreign_keys" = 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.

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