From: Peter Gulutzan <pgulutzan@ocelot.ca> To: Mergen Imeev <imeevma@tarantool.org> Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org, tarantool-discussions@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v3 0/5] Replace control pragmas by SET Date: Wed, 18 Dec 2019 10:39:58 -0700 [thread overview] Message-ID: <67f8badf-2c98-83de-b2aa-781953be5a21@ocelot.ca> (raw) In-Reply-To: <77dd60db-ba26-da21-9502-19f089f1559c@ocelot.ca> Hi, I think that there still might be a few small SQL-specific issues. I pulled today from branch imeevma/gh-4511-pragma-replaced-by-set. 1. The type of "value" is 'any', which is shown in metadata as 'string': " tarantool> box.execute([[SELECT typeof("value") FROM "_vsession_settings" LIMIT 1;]]) --- - metadata: - name: typeof("value") type: string rows: - ['any'] ... " But that means that searches of "value" will usually fail, thus: " tarantool> box.execute([[SELECT "name", "value" FROM "_vsession_settings" where "value" = 'vinyl';]]) --- - null - 'Type mismatch: can not convert text to boolean' ... " Why not scalar? 2. You wrote this comment on issue#4511: " @pgulutzan @kostja It seems that we are going to solve the problem in a different way: we will create a special space that will contain the settings. This space will only allow updates. So, to change the setting value, you should do something like this: box.execute([[UPDATE "_session_settings" SET "value" = 'vinyl' WHERE "name" = 'sql_default_engine']]) or box.space._session_settings:update('sql_default_engine', {{'=', 'value', 'vinyl'}}) This will allow us to use the same method to change any settings in any front-end. Also after that we can simply remove the control p " But at this moment _session_settings does not exist. The only thing that works is box.execute([[SET sql_default_engine = 'vinyl';]]) which is what I and Konstantin Osipov were raising questions about. In fact I can do it without 'write' privileges on anything. This is just temporary, right? 3. VALUE happens to be a reserved word in DB2 and Oracle. Although we have no plans to reserve it, well, it's a micro-issue for compatibility. You might of course dismiss this because our column name is "value" not value. But suppose that some user agrees totally with Konstantin Osipov (alas), and decides to use a method that allows access to the data without quote marks: " tarantool> box.execute([[CREATE VIEW vsession_settings AS SELECT "value" AS value, "name" AS name FROM "_vsession_settings";]]) --- - row_count: 1 ... tarantool> box.execute([[SELECT * FROM vsession_settings WHERE name = 'sql_default_engine';]]) --- - metadata: - name: VALUE type: any - name: NAME type: string rows: - ['vinyl', 'sql_default_engine'] ... " I think this means that some users will have columns named VALUE, so we will cause trouble if someday we reserve it. 4. This suggestion was accepted according to Issue#4511: _session_settings has columns 'name' and 'value', so: UPDATE "session_settings" SET "value" = 'vinyl' WHERE "name" = 'sql_default_engine'; This suggestion (made by N. Pettik in the thread) was rejected: _session_settings has many columns including 'sql_default_engine' and 'value', so UPDATE "session_settings" SET "sql_default_engine" = 'vinyl'; Okay, but ... If we someday support GRANT, and sql_default_engine was a column, we could say: GRANT UPDATE ON "_session_settings" ("sql_default_engine"); that is, we could be very specific about what you can update. But there is no equivalent GRANT statement, with #4511, unless one adds a non-standard extension like GRANT UPDATE on "_session_settings" ("value") WHERE "name" = 'sql_default_engine'; I think that a few other things are simpler if sql_default_engine is a column: CHECK ("sql_default_engine" IN ('memtx','vinyl')) versus CHECK ("name" <> 'sql_default_engine' OR "value" IN ('memtx','vinyl')) and SELECT "sql_default_engine", "parser_trace" FROM "_vsession_settings"; versus SELECT "value" AS sql_default_engine FROM "_vsession_settings" WHERE "name" = 'sql_default_engine' UNION SELECT "value" AS parser_trace FROM "_vsession_settings" WHERE "name" = 'parser_trace'; (I say "I think" and admit that there may be better solutions I didn't think about.) 5. The name _session_settings hints that, if I change the setting, I only affect my own session. However, as you know, if I change sql_compound_select_limit, I affect all sessions. So perhaps it should be in a new different space, _vglobal_settings? Peter Gulutzan
next prev parent reply other threads:[~2019-12-18 17:40 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 [this message] 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 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=67f8badf-2c98-83de-b2aa-781953be5a21@ocelot.ca \ --to=pgulutzan@ocelot.ca \ --cc=imeevma@tarantool.org \ --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