From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 5A65D469719
 for <tarantool-patches@dev.tarantool.org>;
 Tue, 11 Feb 2020 01:09:13 +0300 (MSK)
References: <e35a47d4-7bd9-66a3-6598-d2be3881e845@tarantool.org>
 <20200204193200.61510-1-k.sosnin@tarantool.org>
 <ac542812-4be5-a979-ad5e-290a946fca7e@tarantool.org>
 <FBA2C2CA-0314-45FF-BD7D-BC2DBEEA322C@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <d01a2df7-249a-1bba-20a8-22e06998d5f0@tarantool.org>
Date: Mon, 10 Feb 2020 23:09:11 +0100
MIME-Version: 1.0
In-Reply-To: <FBA2C2CA-0314-45FF-BD7D-BC2DBEEA322C@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly
 frontend for accessing session settings
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Chris Sosnin <k.sosnin@tarantool.org>
Cc: tarantool-patches@dev.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?

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

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

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

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