Tarantool development patches archive
 help / color / mirror / Atom feed
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

  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