From: Kirill Yukhin <kyukhin@tarantool.org> To: Alexander Turenko <alexander.turenko@tarantool.org> Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org, tarantool-discussions@dev.tarantool.org, Peter Gulutzan <pgulutzan@ocelot.ca> Subject: Re: [Tarantool-patches] [PATCH v3 0/5] Replace control pragmas by SET Date: Wed, 18 Dec 2019 13:20:51 +0300 [thread overview] Message-ID: <20191218102051.qgfa6nskqgrsqh4j@tarantool.org> (raw) In-Reply-To: <20191217221143.parwzoyb3e327sw5@tkn_work_nb> Hello, On 18 дек 01:11, Alexander Turenko wrote: > Mergen, > > I don't have enough context here, but have questions about this > proposal. Sorry if they were already discussed and I missed it. > > Most important question is that I don't understand why we don't want to > provide language specific APIs for sessions settings. It looks both more > convenient and more performant. > > See this and other questions and notes below. > > (CCed Peter, because he may have some opinion how SQL API should look.) > > WBR, Alexader Turenko. > > ---- > > AFAIR discussions around a space / view for session settings arose from > Kostya O. proposal to move toward support of standard information schema > views (please, correct me, if I remember it wrongly). Then it becomes > out of scope somehow. But okay, let it being out. > > (BTW, I looked over SQL/Schemata 2011 and don't found anything like > MySQL's GLOBAL_VARIABLES and SESSION_VARIABLES tables. It seems there is > no standard table for session variables. I don't sure however.) > > ---- > > We have two basic variants: > > * Implement an API for session settings for each supported language > (C, Lua, SQL) and a protocol for connectors. > * Provide a system view / space (this is proposed by Mergen). > > First, a space / view is not most convenient way to operate on session > settings from a language. Let's compare. > > Lua: > > | box.space._vsession_settings:get({'sql_default_engine'}).value > | box.space._vsession_settings:update({'sql_default_engine'}, > | {{'=', 'value', 'vinyl'}}) > | > | box.session.settings:get('sql_default_engine') > | box.session.settings:set('sql_default_engine', 'vinyl') Frankly, to me this is not of big difference. Especially when we are talking about settings, references to which are rare. After all, one might want to implement setter to avoid such updates. > SQL: > > | SELECT "value" FROM "_vsession_settings" WHERE "name" = 'sql_default_engine' > | UPDATE "_vsession_settings" SET "value" = 'vinyl' \ > | WHERE "name" = 'sql_default_engine' > | > | SESSION GET 'sql_default_engine' > | SESSION SET 'sql_default_engine' = 'vinyl' > > C (sketchy): > > | /* Read from a _vsession_settings. */ > | > | enum { > | BOX_VSESSION_SETTINGS_VALUE_ID = 2 > | }; > | > | char key[32]; > | char *key_end = key; > | key_end = mp_encode_array(key_end, 1); > | key_end = mp_encode_str(key_end, "sql_default_engine", > | sizeof("sql_default_engine") - 1); > | > | box_tuple_t *tuple; > | box_iterator_t *it = box_index_iterator(BOX_VSESSION_SETTINGS_ID, 0, ITER_EQ, > | key, key_end); > | box_iterator_next(it, &tuple); > | const char *buf = box_tuple_field(tuple, BOX_VSESSION_SETTINGS_VALUE_ID); > | > | uint32_t engine_len; > | const char *engine = mp_decode_str(&buf, &engine_len); > | > | box_iterator_free(it); > | > | /* Update a value in _vsession_settings. */ > | > | <I'll skip it.> > | > | /* Get and set with a language aware API. */ > | > | uint32_t engine_len; > | const char *engine = box_session_get_str(SESSION_SQL_DEFAULT_ENGINE, > | &engine_len); > | > | box_session_set_str(SESSION_SQL_DEFAULT_ENGINE, "vinyl", > | sizeof("vinyl") - 1); > > Languare-aware APIs above are just examples. I propose to implement such > APIs, but not how they should look exactly. This might have sense, but I'd treat it as a follow up activity. > To sum the examples up: it seems for me that language aware APIs are a > way more simple for a user. > > Second, all tuples are msgpack encoded (at least now). So any get/set > operation on _vspace_settings will require to encode and decode msgpack > (yep, both encode and decode at once). It will be surely less performant > then a hashmap lookup (session id -> struct session_settings) plus a > field access. > > So, language aware API can be implemented in more performant way then > general space-like one. I think performance out of scope here at all. > It seems that we anyway need an API for connectors. So we can provide > the proposed view, but don't use it internally to implement language > specific APIs (for performance). > > Console session settings (like statements delimiter, input language, > output format) are out of scope here? Yes. To sum up. We spent too much time here. I think we can improve approaches in future. But I see no serious reasons for that: 1. To make it easier to use we might whant to implement some stored routines or something/ 2. Performance of encode/decode is out of intereset here. I propose you to file a feature request as follow up of the patchset. -- Regards, Kirill Yukhin
next prev parent reply other threads:[~2019-12-18 10:20 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-07 10:36 imeevma 2019-11-07 10:36 ` [Tarantool-patches] [PATCH v3 1/5] sysview: make get() and create_iterator() methods virtual imeevma 2019-11-07 10:36 ` [Tarantool-patches] [PATCH v3 2/5] box: introdice _vsession_settings sysview imeevma 2019-11-07 10:36 ` [Tarantool-patches] [PATCH v3 3/5] sql: introduce SET statement imeevma 2019-11-07 12:40 ` Vladislav Shpilevoy 2019-11-07 14:12 ` Mergen Imeev 2019-11-11 21:56 ` Vladislav Shpilevoy 2019-11-15 14:06 ` Mergen Imeev 2019-11-17 17:26 ` Vladislav Shpilevoy 2019-11-17 20:32 ` Vladislav Shpilevoy 2019-11-27 10:33 ` Mergen Imeev 2019-11-27 23:03 ` Vladislav Shpilevoy 2019-11-27 23:07 ` Vladislav Shpilevoy 2019-11-27 23:09 ` Vladislav Shpilevoy 2019-11-28 8:59 ` Mergen Imeev 2019-11-28 8:56 ` Mergen Imeev 2019-11-07 10:36 ` [Tarantool-patches] [PATCH v3 4/5] temporary: disable boolean.test.sql imeevma 2019-11-07 10:37 ` [Tarantool-patches] [PATCH v3 5/5] sql: replace control pragmas imeevma 2019-12-06 11:37 ` [Tarantool-patches] [PATCH v3 0/5] Replace control pragmas by SET Kirill Yukhin 2019-12-06 13:50 ` Mergen Imeev 2019-12-06 14:06 ` Sergey Ostanevich 2019-12-17 22:11 ` Alexander Turenko 2019-12-18 2:39 ` Peter Gulutzan 2019-12-18 17:39 ` Peter Gulutzan 2019-12-19 9:59 ` Mergen Imeev 2019-12-19 17:35 ` Peter Gulutzan 2019-12-19 17:51 ` Mergen Imeev 2019-12-19 21:09 ` Vladislav Shpilevoy 2019-12-18 10:20 ` Kirill Yukhin [this message] 2019-12-18 10:53 ` Alexander Turenko
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=20191218102051.qgfa6nskqgrsqh4j@tarantool.org \ --to=kyukhin@tarantool.org \ --cc=alexander.turenko@tarantool.org \ --cc=pgulutzan@ocelot.ca \ --cc=tarantool-discussions@dev.tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v3 0/5] Replace control pragmas by SET' \ /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