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

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

[-- Attachment #1: Type: text/plain, Size: 3823 bytes --]



> 11 февр. 2020 г., в 01:09, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
> 
> Hi! Thanks for the fixes!
> 
> On the branch you said this commit is 'Part of #4711'. Why
> not 'Closes #4711'? What else is left?
> 
Replaced with ‘Closes’.

>>    sql: provide a user friendly frontend for accessing session settings
>> 
>>    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.
>> 
>>    Part of #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 accessible right after
>>    session creation. The syntax is the following:
> 
> Lets say it is available always. Because a user can't live without
> a session anyway. It always exists for a user.
Commited the following change:

which is accessible right after session creation —> which is always accessible
to user


>>    `box.session.settings.<setting_name>:set(<new_value>)`.
>> 
>>    Example of usage:
>>    ```
>>    tarantool> box.session.settings.sql_default_engine
>>    ---
>>    - memtx
>>    ...
>> 
>>    tarantool> box.session.settings.sql_default_engine:set('vinyl')
>>    ---
>>    ...
>> 
>>    ```
>> 
>>    The table itself represents the (unordered) result of select
>>    from _session_settings space. Every setting is implemented as
>>    a table, so there is no way to retrieve an actual value and use
>>    it until :get() method is introduced.
>> 
>>    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]])
>>    ---
>>    - row_count: 1
>>    ...
>> 
>>    ```
> 
> I am not so sure about binary search anymore. I just found another
> probably much faster way - perfect hash function. There is a GNU
> tool 'gperf' to generate C code for a given keyset with hash
> calculation and search functions.
> 
> I fed our settings to it and obtained not so scary result.
> 
> File: test.gperf
> 
>    %%
>    sql_default_engine
>    sql_defer_foreign_keys
>    sql_full_column_names
>    sql_full_metadata
>    sql_parser_debug
>    sql_recursive_triggers
>    sql_reverse_unordered_selects
>    sql_select_debug
>    sql_vdbe_debug
>    %%
> 
> Command: gperf test.gperf > test.c
> 
> The test.c file is 122 lines long including some {}, useless
> checks, macros, and comments. It could easily be compacted a lot.
I like the idea of speeding search up, but it would make adding new settings
even more complicated (+regenerate file with gpref, +mannualy format it).

> 
> This is probably overkill though. It needs to be completely
> regenerated each time when the keyset is changed, consumes more
> static memory.
I agree.

> 
> The patchset LGTM. I propose to send it to Nikita on a second
> review.

Nikita, please do a second review.

branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4712-search-settings <https://github.com/tarantool/tarantool/tree/ksosnin/gh-4712-search-settings>
issue #1: https://github.com/tarantool/tarantool/issues/4711 <https://github.com/tarantool/tarantool/issues/4711>
issue #2: https://github.com/tarantool/tarantool/issues/4712 <https://github.com/tarantool/tarantool/issues/4712>

[-- Attachment #2: Type: text/html, Size: 6686 bytes --]