From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp15.mail.ru (smtp15.mail.ru [94.100.176.133]) (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 3EA97469719 for ; Tue, 17 Mar 2020 17:36:45 +0300 (MSK) From: Chris Sosnin Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_3E88CD99-C1B7-4B10-BD71-955CB56BF718" Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\)) Date: Tue, 17 Mar 2020 17:36:43 +0300 In-Reply-To: <20200317142759.GC3221@tarantool.org> References: <80558927f63a1ad49a4b7c699bfff0bedd727a11.1581940900.git.k.sosnin@tarantool.org> <20200316160842.GG14628@tarantool.org> <10819f60-76f1-988c-79be-eb70c2667af5@tarantool.org> <20200317142759.GC3221@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 3/4] box: 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 --Apple-Mail=_3E88CD99-C1B7-4B10-BD71-955CB56BF718 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On 17 Mar 2020, at 17:27, Nikita Pettik = wrote: >=20 > On 16 Mar 23:53, Vladislav Shpilevoy wrote: >> On 16/03/2020 17:08, Nikita Pettik wrote: >>> On 17 Feb 15:12, Chris Sosnin wrote: >>>> - space_object:update() is hard to use for configuring session = settings, >>>> so we provide box.session.setting table, which can be used in a = much more >>>> native way. >>>>=20 >>>> - Prior to this patch sql settings were not accessible before = box.cfg() >>>> call, even though these flags can be set right after session = creation. >>>>=20 >>>> Part of #4711 >>>> --- >>>=20 >>> tarantool> box.session.settings.sql_vdbe_debug >>> --- >>> - false >>> ... >>>=20 >>> tarantool> box.session.settings.sql_vdbe_debug =3D true >>> --- >>> ... >>>=20 >>> tarantool> box.session.settings.sql_vdbe_debug >>> --- >>> - true >>> ... >>=20 >> Yeah, we can ban this. To avoid confusion. >>=20 >>>=20 >>> tarantool> box.execute("select 1") >>> --- >>> - metadata: >>> - name: '1' >>> type: integer >>> rows: >>> - [1] >>> ... >>>=20 >>> Looks inconsistent. Can we use instead of :set() method simple >>> table value assignment? Otherwise accessing row table values >>=20 >> Assignment would require not to store settings in = box.session.settings, >> to be able to redefine __newindex metamethod. If we don't store them, = we >> kill autocompletion, which was asked explicitly by somebody. >>=20 >> But I am on your side here - I don't think autocompletion worth this >> complication. Who wants to look at existing settings can just print >> box.sessions.settings table. >>=20 >>> should be disallowed. Same concerns :get() method. Why ever >>> anyone should bother with :get() when one can access table value >>> via simple indexing? >>=20 >> Hm. But there is no :get() method. We didn't implement getting, = because >> no one asked for this. >=20 > As far as I remember no one either asked for :set() method (except for = me) :) This method is a workaround to allow console autocompletion, Lua = doesn=E2=80=99t allow you to overload indexing for the keys already present in the table. But here I agree = that this implementation is rather misleading. I will rework the patch to allow settings. =3D = syntax. >=20 >> And indeed, usually you just set settings, without >> checking if set really worked. I was thinking we could introduce get = if >> someone asks for that. But up to you. Can be added now as well. >=20 > I do not insist since do not consider this to be prio1 task (as well > as any other issue connected with settings machinery). --Apple-Mail=_3E88CD99-C1B7-4B10-BD71-955CB56BF718 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On 17 Mar 2020, at 17:27, Nikita Pettik <korablev@tarantool.org> wrote:

On 16 Mar 23:53, Vladislav = Shpilevoy wrote:
On 16/03/2020 17:08, Nikita Pettik = wrote:
On 17 Feb = 15:12, Chris Sosnin wrote:
- space_object:update() is hard to use for configuring = session settings,
so we provide box.session.setting table, = which can be used in a much more
native way.

- Prior to this patch sql settings were not = accessible before box.cfg()
call, even though these flags = can be set right after session creation.

Part= of #4711
---

tarantool> box.session.settings.sql_vdbe_debug
---
- false
...

tarantool> box.session.settings.sql_vdbe_debug =3D true
---
...

tarantool> box.session.settings.sql_vdbe_debug
---
- true
...

Yeah, we can ban this. To avoid = confusion.


tarantool> box.execute("select 1")
---
- metadata:
 - name: = '1'
   type: integer
 rows:
 - [1]
...

Looks inconsistent. Can we use instead of = :set() method simple
table value assignment? Otherwise = accessing row table values

Assignment would require not to store settings in = box.session.settings,
to be able to redefine __newindex = metamethod. If we don't store them, we
kill = autocompletion, which was asked explicitly by somebody.

But I am on your side here - I don't think autocompletion = worth this
complication. Who wants to look at existing = settings can just print
box.sessions.settings table.

should be = disallowed. Same concerns :get() method. Why ever
anyone = should bother with :get() when one can access table value
via simple indexing?

Hm. But there is no :get() method. We didn't implement = getting, because
no one asked for this.

As far as I remember no one either asked for :set() method = (except for me) :)

This method is a workaround to allow console = autocompletion, Lua doesn=E2=80=99t allow you to = overload
indexing for the keys already present in the table. = But here I agree that this implementation is = rather
misleading. I will rework the patch to allow = settings.<name> =3D <value> syntax.


And = indeed, usually you just set settings, without
checking if = set really worked. I was thinking we could introduce get if
someone asks for that. But up to you. Can be added now as = well.

I do not insist since do not consider this to be prio1 task = (as well
as any other = issue connected with settings = machinery).

= --Apple-Mail=_3E88CD99-C1B7-4B10-BD71-955CB56BF718--