Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mergen Imeev <imeevma@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v3 3/5] sql: introduce SET statement
Date: Thu, 28 Nov 2019 11:59:25 +0300	[thread overview]
Message-ID: <20191128085925.GB8088@tarantool.org> (raw)
In-Reply-To: <1695bfaa-b250-ad14-848d-1a55fd13508c@tarantool.org>

On Thu, Nov 28, 2019 at 12:07:34AM +0100, Vladislav Shpilevoy wrote:
> No, stop. It is not LGTM.
> 
> 1) Why on the branch I see a commit 
> 
>     "sql: refactor PRAGMA-related code"
> 
> But don't see it in this thread?
> 
> 2) In a previous email I wrote this:
> 
> 	- Parts of the message before '@Tarantoolbot document' and
> 	  after are in some ideas different, in some they are totally
> 	  the same. I propose you to remove the part before doc
> 	  request, and write the doc request more accurate. And check
> 	  again that all deleted PRAGMAs really don't work (in the last
> 	  patch), all new SETs really work, with exactly the same names
> 	  as in the commit message;
> 
> This is still actual. You wrote lots of text before docrequest,
> and just a few lines after. The doc team will see the small part.
> Once again, I propose you to drop the part before doc request,
> and write everything after @Tarantoolbot document.

Thank you for review! I fixed mentioned problems. New
commit-message and diff below.


New commit-message:

sql: replace control pragmas by SET

This patch replaces the control pragmas with SET. After this patch
there will be no control pragmas.

Closes #4511
Part of #4621

@TarantoolBot document
Title: SQL SET statement

The SQL SET statement is used to change SQL settings. It should be
used as a substitute for PRAGMA. Pragmas replaced by SET are
deleted.

To change the value of an SQL setting, use the following syntax:
SET <setting name> = <setting value>;

Currently available SQL settings:
'sql_defer_foreign_keys'
'sql_full_column_names'
'sql_recursive_triggers'
'sql_reverse_unordered_selects'
'sql_compound_select_limit'
'sql_default_engine'

In addition, SQL debugging settings can also be changed using this
statement in debug build:
'sql_parser_trace'
'sql_select_trace'
'sql_trace'
'sql_vdbe_addoptrace'
'sql_vdbe_debug'
'sql_vdbe_eqp'
'sql_vdbe_listing'
'sql_vdbe_trace'
'sql_where_trace'

List of replaced control pragmas and their SET analogues:
	Control pragmas			SET parameters
defer_foreign_keys		sql_defer_foreign_keys
full_column_names		sql_full_column_names
recursive_triggers		sql_recursive_triggers
reverse_unordered_selects	sql_reverse_unordered_selects
sql_compound_select_limit	sql_compound_select_limit
sql_default_engine		sql_default_engine

Also, in debug build, these control pragmas are replaced by SET
analogues:
	Control pragmas			SET parameters
parser_trace			sql_parser_trace
select_trace			sql_select_trace
sql_trace			sql_trace
vdbe_addoptrace			sql_vdbe_addoptrace
vdbe_debug			sql_vdbe_debug
vdbe_eqp			sql_vdbe_eqp
vdbe_listing			sql_vdbe_listing
vdbe_trace			sql_vdbe_trace
where_trace			sql_where_trace

Difference between SET and control pragma:
1) SET have more definite syntax:
SET <setting name> = <setting value>;
In PRAGMA, we could set the settings using these methods:
PRAGMA <setting name> = <setting value>;
PRAGMA <setting name>(<setting value>);

2) SET allows only a specific type of value for each setting. In
PRAGMA, we could use almost everything to set up any setting.
Although the rules by which the settings were set in PRAGMA were
pretty easy to understand.

3) SET cannot display setting values. PRAGMA showed the setting
values using the syntax "PRAGMA <setting name>;". With SET, we
must use other means to get the current setting values. For
session settings, we could use the sysview "_vsession_settings"
to get these values. It is worth noting that all current SQL
settings are session settings, with the exception of
'sql_reverse_unordered_selects'.

Example of usage:
SET sql_default_engine = 'memtx';
SET sql_reverse_unordered_selects = false;



Diff:

diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
index e2d60a9..2f21cc1 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -132,3 +132,79 @@ box.execute("SET sql_reverse_unordered_selects = "..tostring(order)..";")
 ---
 - row_count: 0
 ...
+-- Make sure that pragmas doesn't work anymore
+box.execute("PRAGMA defer_foreign_keys = 'true';")
+---
+- null
+- Pragma 'DEFER_FOREIGN_KEYS' does not exist
+...
+box.execute("PRAGMA full_column_names = 1;")
+---
+- null
+- Pragma 'FULL_COLUMN_NAMES' does not exist
+...
+box.execute("PRAGMA recursive_triggers;")
+---
+- null
+- Pragma 'RECURSIVE_TRIGGERS' does not exist
+...
+box.execute("PRAGMA reverse_unordered_selects = 0;")
+---
+- null
+- Pragma 'REVERSE_UNORDERED_SELECTS' does not exist
+...
+box.execute("PRAGMA sql_compound_select_limit = 15;")
+---
+- null
+- Pragma 'SQL_COMPOUND_SELECT_LIMIT' does not exist
+...
+box.execute("PRAGMA sql_default_engine = 'some_engine';")
+---
+- null
+- Pragma 'SQL_DEFAULT_ENGINE' does not exist
+...
+box.execute("PRAGMA parser_trace = true;")
+---
+- null
+- Pragma 'PARSER_TRACE' does not exist
+...
+box.execute("PRAGMA select_trace = 0;")
+---
+- null
+- Pragma 'SELECT_TRACE' does not exist
+...
+box.execute("PRAGMA sql_trace = 1;")
+---
+- null
+- Pragma 'SQL_TRACE' does not exist
+...
+box.execute("PRAGMA vdbe_addoptrace = 'OFF'")
+---
+- null
+- Pragma 'VDBE_ADDOPTRACE' does not exist
+...
+box.execute("PRAGMA vdbe_debug = 'ON'")
+---
+- null
+- Pragma 'VDBE_DEBUG' does not exist
+...
+box.execute("PRAGMA vdbe_eqp;")
+---
+- null
+- Pragma 'VDBE_EQP' does not exist
+...
+box.execute("PRAGMA vdbe_listing = 'false'")
+---
+- null
+- Pragma 'VDBE_LISTING' does not exist
+...
+box.execute("PRAGMA vdbe_trace;")
+---
+- null
+- Pragma 'VDBE_TRACE' does not exist
+...
+box.execute("PRAGMA where_trace = false")
+---
+- null
+- Pragma 'WHERE_TRACE' does not exist
+...
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
index b15deee..033acd1 100644
--- a/test/sql/sql-debug.test.lua
+++ b/test/sql/sql-debug.test.lua
@@ -26,3 +26,21 @@ box.execute('SELECT * FROM "_vsession_settings";')
 
 box.execute("SET sql_default_engine = '"..engine.."';")
 box.execute("SET sql_reverse_unordered_selects = "..tostring(order)..";")
+
+-- Make sure that pragmas doesn't work anymore
+box.execute("PRAGMA defer_foreign_keys = 'true';")
+box.execute("PRAGMA full_column_names = 1;")
+box.execute("PRAGMA recursive_triggers;")
+box.execute("PRAGMA reverse_unordered_selects = 0;")
+box.execute("PRAGMA sql_compound_select_limit = 15;")
+box.execute("PRAGMA sql_default_engine = 'some_engine';")
+
+box.execute("PRAGMA parser_trace = true;")
+box.execute("PRAGMA select_trace = 0;")
+box.execute("PRAGMA sql_trace = 1;")
+box.execute("PRAGMA vdbe_addoptrace = 'OFF'")
+box.execute("PRAGMA vdbe_debug = 'ON'")
+box.execute("PRAGMA vdbe_eqp;")
+box.execute("PRAGMA vdbe_listing = 'false'")
+box.execute("PRAGMA vdbe_trace;")
+box.execute("PRAGMA where_trace = false")

  parent reply	other threads:[~2019-11-28  8:59 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-07 10:36 [Tarantool-patches] [PATCH v3 0/5] Replace control pragmas by SET 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 [this message]
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
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=20191128085925.GB8088@tarantool.org \
    --to=imeevma@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v3 3/5] sql: introduce SET statement' \
    /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