From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (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 151AA469719 for ; Tue, 17 Mar 2020 20:26:56 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\)) From: Chris Sosnin In-Reply-To: <20200316170212.GH14628@tarantool.org> Date: Tue, 17 Mar 2020 20:26:54 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <6B7EB5E1-B936-46F4-AB4F-28EEDB72408A@tarantool.org> References: <36341694915f89597f2ac938077d0d10bcad0448.1581940900.git.k.sosnin@tarantool.org> <20200316170212.GH14628@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 4/4] sql: provide a user friendly frontend for accessing session settings List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org > On 16 Mar 2020, at 20:02, Nikita Pettik = wrote: >=20 > 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. >=20 > 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? >=20 >> Closes #4711 >>=20 >> @TarantoolBot document >> Title: API for accessing _session_settings space. >> There are two ways of updating values of session settings: >> via Lua and SQL. >>=20 >> SQL: >> Instead of typing long UPDATE query one can use the SET statement: >> `box.execute([[SET "" =3D ]])`. >> Note, that this query is case sensitive so the name must be quoted. >>=20 >> Example: >> ``` >> tarantool> box.execute([[set "sql_default_engine" =3D 'memtx']]) >> --- >> - row_count: 1 >> ... >>=20 >> tarantool> box.execute([[set "sql_defer_foreign_keys" =3D true]]) >=20 > 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? >=20 >> 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); >>=20 >> 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" >>=20 >> /* >> * Invoke this macro on memory cells just prior to changing the >> @@ -5248,6 +5249,55 @@ case OP_IncMaxid: { >> break; >> } >>=20 >> +/* 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: { >=20 > Please, use more specific opcode name. Like OP_SettingSet. Didn=E2=80=99t we accept =E2=80=99set=E2=80=99 syntax? >=20