From: Chris Sosnin <k.sosnin@tarantool.org>
To: Nikita Pettik <korablev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings
Date: Wed, 18 Mar 2020 00:00:36 +0300 [thread overview]
Message-ID: <892E6B07-11CA-4A55-BC55-08B9E34F6C27@tarantool.org> (raw)
In-Reply-To: <20200317201232.GA4294@tarantool.org>
[-- Attachment #1: Type: text/plain, Size: 4231 bytes --]
Thank you for your answers!
> 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" = true WHERE "name" = '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> = <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>" = <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.
>>
>> SELECT * FROM “_session_settings” is simple enough, isn’t 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" = '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’t we accept ’set’ 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.
I don’t mind this changes, I will work on them tomorrow.
[-- Attachment #2: Type: text/html, Size: 26502 bytes --]
next prev parent reply other threads:[~2020-03-17 21:00 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-17 12:12 [Tarantool-patches] [PATCH 0/4] box: session settings fixes Chris Sosnin
2020-02-17 12:12 ` [Tarantool-patches] [PATCH 1/4] box: replace session_settings modules with a single array Chris Sosnin
2020-02-17 12:12 ` [Tarantool-patches] [PATCH 2/4] box: add binary search for _session_settings space Chris Sosnin
2020-03-16 14:16 ` Nikita Pettik
2020-03-16 22:53 ` Vladislav Shpilevoy
2020-03-17 17:24 ` Nikita Pettik
2020-02-17 12:12 ` [Tarantool-patches] [PATCH 3/4] box: provide a user friendly frontend for accessing session settings Chris Sosnin
2020-03-16 16:08 ` Nikita Pettik
2020-03-16 22:53 ` Vladislav Shpilevoy
2020-03-17 14:27 ` Nikita Pettik
2020-03-17 14:36 ` Chris Sosnin
2020-02-17 12:12 ` [Tarantool-patches] [PATCH 4/4] sql: " Chris Sosnin
2020-03-16 17:02 ` Nikita Pettik
2020-03-16 22:53 ` Vladislav Shpilevoy
2020-03-17 17:26 ` Chris Sosnin
2020-03-17 20:12 ` Nikita Pettik
2020-03-17 21:00 ` Chris Sosnin [this message]
2020-03-18 10:00 ` Chris Sosnin
-- strict thread matches above, loose matches on Subject: below --
2020-03-30 9:13 [Tarantool-patches] [PATCH 0/4] session settings fixes Chris Sosnin
2020-03-30 9:13 ` [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings Chris Sosnin
2020-04-03 15:19 ` Nikita Pettik
2020-04-04 21:56 ` Vladislav Shpilevoy
2020-04-10 15:40 ` Chris Sosnin
2020-04-11 17:18 ` Vladislav Shpilevoy
2020-04-13 7:50 ` Timur Safin
2020-02-03 22:17 [Tarantool-patches] [PATCH] " Vladislav Shpilevoy
2020-02-04 19:32 ` [Tarantool-patches] [PATCH 4/4] " Chris Sosnin
2020-02-06 22:16 ` Vladislav Shpilevoy
2020-02-07 9:40 ` Chris Sosnin
2020-02-10 22:09 ` Vladislav Shpilevoy
2020-02-17 11:46 ` Chris Sosnin
2020-02-17 11:56 ` Nikita Pettik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=892E6B07-11CA-4A55-BC55-08B9E34F6C27@tarantool.org \
--to=k.sosnin@tarantool.org \
--cc=korablev@tarantool.org \
--cc=tarantool-patches@dev.tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox