* [tarantool-patches] [PATCH v3 0/6] sql: set column types for EXPLAIN and PRAGMA
@ 2018-12-26 18:17 imeevma
2018-12-26 18:17 ` [tarantool-patches] [PATCH v3 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: imeevma @ 2018-12-26 18:17 UTC (permalink / raw)
To: tarantool-patches, korablev
This patch-set defines the types for the result columns of EXPLAIN
and PRAGMA commands. In addition, it fixes some problems of the
PRAGMA commands that have something to do with their result.
https://github.com/tarantool/tarantool/issues/3832
https://github.com/tarantool/tarantool/tree/imeevma/gh-3832-no-column-types
Changes in third version:
- Fixed function/varible/constant names
- Fixes commit-messages
- Types of two pragmas were changed to FLAG.
- Created new test and tests of two pathes were moved into it.
v1: https://www.freelists.org/post/tarantool-patches/PATCH-v1-11-sql-set-column-types-for-EXPLAIN-and-PRAGMA
v2: https://www.freelists.org/post/tarantool-patches/PATCH-v2-06-sql-set-column-types-for-EXPLAIN-and-PRAGMA
Mergen Imeev (6):
sql: remove unused macros from pragma.c and pragma.h
sql: fix "PRAGMA parser_trace" result
sql: Show currently set sql_default_engine
sql: fix "PRAGMA case_sensitive_like" result
sql: 'PRAGMA' result in the appropriate format
sql: set column types for EXPLAIN and PRAGMA
src/box/execute.c | 5 +-
src/box/sql/pragma.c | 222 ++++++++++++++----------------
src/box/sql/pragma.h | 258 ++++++++++++++++++++++-------------
src/box/sql/prepare.c | 52 +++++--
src/box/sql/sqliteInt.h | 4 +
test/sql-tap/gh-2367-pragma.test.lua | 35 +++--
test/sql/iproto.result | 69 ++++++++++
test/sql/iproto.test.lua | 18 ++-
test/sql/misc.result | 20 +++
test/sql/misc.test.lua | 13 ++
test/sql/sql-debug.result | 72 ++++++++++
test/sql/sql-debug.test.lua | 25 ++++
test/sql/suite.ini | 2 +-
13 files changed, 557 insertions(+), 238 deletions(-)
create mode 100644 test/sql/sql-debug.result
create mode 100644 test/sql/sql-debug.test.lua
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] [PATCH v3 1/6] sql: remove unused macros from pragma.c and pragma.h
2018-12-26 18:17 [tarantool-patches] [PATCH v3 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
@ 2018-12-26 18:17 ` imeevma
2019-01-16 15:34 ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 2/6] sql: fix "PRAGMA parser_trace" result imeevma
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: imeevma @ 2018-12-26 18:17 UTC (permalink / raw)
To: tarantool-patches, korablev
Hi! Thank you for review! My answers and new version below. I
didn't include diff, because all that changed was the commit
message.
On 12/24/18 5:01 PM, n.pettik wrote:
> Nit: at the end of commit subject redundant ‘*’ and dot.
Fixed.
> They weren’t deleted, they are simply not used.
> I guess they still can be enabled, if add appropriate commands
> to cmake lists. On the other hand, I doubt that someday we
> really may need them.
Made changes in in commit-message.
> In fact, this commit has nothing in common with mentioned issue.
Removed from commit-message.
> Why didn’t you remove ENABLE_SELECTTRACE as well?
I didn't remove it because it can be set through cmake.
> The same question.
Can also be set via cmake.
New version:
commit bce2e738fc866b2637bc167aa87bc37bd308474b
Author: Mergen Imeev <imeevma@gmail.com>
Date: Wed Dec 12 21:58:54 2018 +0300
sql: remove unused macros from pragma.c and pragma.h
Some macros in pragma.c and pragma.h are obsolete because the
values they are checking are no longer used. Let's remove them.
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index eef1ed9..5729fe6 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -465,7 +465,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
/* Jump to the appropriate pragma handler */
switch (pPragma->ePragTyp) {
-#ifndef SQLITE_OMIT_FLAG_PRAGMAS
case PragTyp_FLAG:{
if (zRight == 0) {
setPragmaResultColumnNames(v, pPragma);
@@ -494,9 +493,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
}
break;
}
-#endif /* SQLITE_OMIT_FLAG_PRAGMAS */
-#ifndef SQLITE_OMIT_SCHEMA_PRAGMAS
case PragTyp_TABLE_INFO:
sql_pragma_table_info(pParse, zRight);
break;
@@ -538,7 +535,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
box_iterator_free(iter);
break;
}
-#endif /* SQLITE_OMIT_SCHEMA_PRAGMAS */
case PragTyp_FOREIGN_KEY_LIST:{
if (zRight == NULL)
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index e608016..84ab478 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -97,41 +97,32 @@ static const PragmaName aPragmaName[] = {
/* ePragFlg: */ PragFlg_NoColumns,
/* ColNames: */ 0, 0,
/* iArg: */ 0},
-#if !defined(SQLITE_OMIT_SCHEMA_PRAGMAS)
{ /* zName: */ "collation_list",
/* ePragTyp: */ PragTyp_COLLATION_LIST,
/* ePragFlg: */ PragFlg_Result0,
/* ColNames: */ 21, 2,
/* iArg: */ 0},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
{ /* zName: */ "count_changes",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
/* iArg: */ SQLITE_CountRows},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
{ /* zName: */ "defer_foreign_keys",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
/* iArg: */ SQLITE_DeferFKs},
-#endif
{ /* zName: */ "foreign_key_list",
/* ePragTyp: */ PragTyp_FOREIGN_KEY_LIST,
/* ePragFlg: */
PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
/* ColNames: */ 23, 8,
/* iArg: */ 0},
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
{ /* zName: */ "full_column_names",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
/* iArg: */ SQLITE_FullColNames},
-#endif
-#if !defined(SQLITE_OMIT_SCHEMA_PRAGMAS)
{ /* zName: */ "index_info",
/* ePragTyp: */ PragTyp_INDEX_INFO,
/* ePragFlg: */
@@ -144,15 +135,13 @@ static const PragmaName aPragmaName[] = {
PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
/* ColNames: */ 16, 5,
/* iArg: */ 0},
-#endif
-#if defined(SQLITE_DEBUG) && !defined(SQLITE_OMIT_PARSER_TRACE)
+#if defined(SQLITE_DEBUG)
{ /* zName: */ "parser_trace",
/* ePragTyp: */ PragTyp_PARSER_TRACE,
/* ePragFlg: */ 0,
/* ColNames: */ 0, 0,
/* iArg: */ 0},
#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
{ /* zName: */ "query_only",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
@@ -168,28 +157,23 @@ static const PragmaName aPragmaName[] = {
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
/* iArg: */ SQLITE_RecTriggers},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
{ /* zName: */ "reverse_unordered_selects",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
/* iArg: */ SQLITE_ReverseOrder},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS) && defined(SQLITE_ENABLE_SELECTTRACE)
+#if defined(SQLITE_ENABLE_SELECTTRACE)
{ /* zName: */ "select_trace",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
/* iArg: */ SQLITE_SelectTrace},
#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
{ /* zName: */ "short_column_names",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
/* iArg: */ SQLITE_ShortColNames},
-#endif
{ /* zName: */ "sql_compound_select_limit",
/* ePragTyp: */ PragTyp_COMPOUND_SELECT_LIMIT,
/* ePragFlg: */ PragFlg_Result0,
@@ -200,7 +184,6 @@ static const PragmaName aPragmaName[] = {
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
/* iArg: */ 0},
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
#if defined(SQLITE_DEBUG)
{ /* zName: */ "sql_trace",
/* ePragTyp: */ PragTyp_FLAG,
@@ -208,24 +191,18 @@ static const PragmaName aPragmaName[] = {
/* ColNames: */ 0, 0,
/* iArg: */ SQLITE_SqlTrace},
#endif
-#endif
-#if !defined(SQLITE_OMIT_SCHEMA_PRAGMAS)
{ /* zName: */ "stats",
/* ePragTyp: */ PragTyp_STATS,
/* ePragFlg: */
PragFlg_NeedSchema | PragFlg_Result0 | PragFlg_SchemaReq,
/* ColNames: */ 6, 4,
/* iArg: */ 0},
-#endif
-#if !defined(SQLITE_OMIT_SCHEMA_PRAGMAS)
{ /* zName: */ "table_info",
/* ePragTyp: */ PragTyp_TABLE_INFO,
/* ePragFlg: */
PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
/* ColNames: */ 0, 6,
/* iArg: */ 0},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
#if defined(SQLITE_DEBUG)
{ /* zName: */ "vdbe_addoptrace",
/* ePragTyp: */ PragTyp_FLAG,
@@ -254,8 +231,7 @@ static const PragmaName aPragmaName[] = {
/* ColNames: */ 0, 0,
/* iArg: */ SQLITE_VdbeTrace},
#endif
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS) && defined(SQLITE_ENABLE_WHERETRACE)
+#if defined(SQLITE_ENABLE_WHERETRACE)
{ /* zName: */ "where_trace",
/* ePragTyp: */ PragTyp_FLAG,
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] [PATCH v3 2/6] sql: fix "PRAGMA parser_trace" result
2018-12-26 18:17 [tarantool-patches] [PATCH v3 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2018-12-26 18:17 ` [tarantool-patches] [PATCH v3 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
@ 2018-12-26 18:18 ` imeevma
2019-01-16 15:35 ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 3/6] sql: Show currently set sql_default_engine imeevma
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: imeevma @ 2018-12-26 18:18 UTC (permalink / raw)
To: tarantool-patches, korablev
Hi! Thank you for review! My answers, diff between versions and
new version below.
On 12/24/18 5:01 PM, n.pettik wrote:
> Why is this patch part of #3832?
> Couldn’t you implement #3832 without this patch?
Removed this from commit-message.
> Why can’t you simply set to this pragma type ‘flag’?
> It works almost that way.
Done.
> Let's avoid using ’sqlite’ prefix in any added code.
Fixed.
> Personally I'd rather create new test file for this (debug) purpose.
> Also, your test doesn’t check value which has been set (it checks
> only that it is bool):
>
> Pragma parser_trace=1
> pragma parser_trace — this must return 1, not 0.
Done. I created new test file.
Diff between versions:
commit 4b1229191fbf15fd9d0d9bb178e8ab8de2c3280f
Author: Mergen Imeev <imeevma@gmail.com>
Date: Tue Dec 25 18:19:35 2018 +0300
Temporary: review fixes
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index b8edc76..6122986 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -466,33 +466,31 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
switch (pPragma->ePragTyp) {
case PragTyp_FLAG:{
- if (zRight == 0) {
- setPragmaResultColumnNames(v, pPragma);
- returnSingleInt(v,
- (user_session->
- sql_flags & pPragma->iArg) !=
- 0);
- } else {
- int mask = pPragma->iArg; /* Mask of bits to set
- * or clear.
- */
-
- if (sqlite3GetBoolean(zRight, 0)) {
- user_session->sql_flags |= mask;
- } else {
- user_session->sql_flags &= ~mask;
- }
-
- /* Many of the flag-pragmas modify the code
- * generated by the SQL * compiler (eg.
- * count_changes). So add an opcode to expire
- * all * compiled SQL statements after
- * modifying a pragma value.
- */
- sqlite3VdbeAddOp0(v, OP_Expire);
+ if (zRight == 0) {
+ setPragmaResultColumnNames(v, pPragma);
+ returnSingleInt(v, (user_session->sql_flags &
+ pPragma->iArg) != 0);
+ } else {
+ int mask = pPragma->iArg; /* Mask of bits to set
+ * or clear.
+ */
+ bool is_value_true = sqlite3GetBoolean(zRight, 0);
+
+ if (is_value_true)
+ user_session->sql_flags |= mask;
+ else
+ user_session->sql_flags &= ~mask;
+#ifndef NDEBUG
+ if (mask == PARSER_TRACE_FLAG) {
+ if (is_value_true)
+ sqlite3ParserTrace(stdout, "parser: ");
+ else
+ sqlite3ParserTrace(0, 0);
}
- break;
+#endif
}
+ break;
+ }
case PragTyp_TABLE_INFO:
sql_pragma_table_info(pParse, zRight);
@@ -569,24 +567,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
}
break;
}
-#ifndef NDEBUG
- case PragTyp_PARSER_TRACE:{
- int mask = pPragma->iArg;
- if (zRight == NULL) {
- returnSingleInt(v,
- (user_session->sql_flags & mask) != 0);
- } else {
- if (sqlite3GetBoolean(zRight, 0)) {
- sqlite3ParserTrace(stdout, "parser: ");
- user_session->sql_flags |= mask;
- } else {
- sqlite3ParserTrace(0, 0);
- user_session->sql_flags &= ~mask;
- }
- }
- break;
- }
-#endif
/*
* Reinstall the LIKE and functions. The variant
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 59e0e1e..ae8e030 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -14,7 +14,6 @@
#define PragTyp_INDEX_LIST 11
#define PragTyp_STATS 15
#define PragTyp_TABLE_INFO 17
-#define PragTyp_PARSER_TRACE 24
#define PragTyp_DEFAULT_ENGINE 25
#define PragTyp_COMPOUND_SELECT_LIMIT 26
@@ -137,10 +136,10 @@ static const PragmaName aPragmaName[] = {
/* iArg: */ 0},
#if defined(SQLITE_DEBUG)
{ /* zName: */ "parser_trace",
- /* ePragTyp: */ PragTyp_PARSER_TRACE,
+ /* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
- /* iArg: */ SQLITE_ParserTrace},
+ /* iArg: */ PARSER_TRACE_FLAG},
#endif
{ /* zName: */ "query_only",
/* ePragTyp: */ PragTyp_FLAG,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 212f75a..a484486 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1564,7 +1564,7 @@ struct sqlite3 {
*/
#define SQLITE_VdbeTrace 0x00000001 /* True to trace VDBE execution */
/* Debug print info about SQL query as it parsed */
-#define SQLITE_ParserTrace 0x00000002
+#define PARSER_TRACE_FLAG 0x00000002
#define SQLITE_FullColNames 0x00000004 /* Show full column names on SELECT */
#define SQLITE_ShortColNames 0x00000040 /* Show short columns names */
#define SQLITE_CountRows 0x00000080 /* Count rows changed by INSERT, */
diff --git a/test/sql/errinj.result b/test/sql/errinj.result
index 6ba7e72..cb993f8 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -280,22 +280,3 @@ errinj.set("ERRINJ_WAL_IO", false)
box.sql.execute("DROP TABLE t3;")
---
...
---
--- gh-3832: Some statements do not return column type
--- This test placed here because it should be skipped in release
--- build.
--- Check that "PRAGMA parser_trace" returns 0 or 1 if called
--- without parameter.
-result = box.sql.execute('PRAGMA parser_trace')
----
-...
--- Should be TRUE.
-result[1][1] == 0 or result[1][1] == 1
----
-- true
-...
--- Check that "PRAGMA parser_trace" returns nothing if called
--- with parameter.
-box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
----
-...
diff --git a/test/sql/errinj.test.lua b/test/sql/errinj.test.lua
index a938eda..fa7f9f2 100644
--- a/test/sql/errinj.test.lua
+++ b/test/sql/errinj.test.lua
@@ -97,19 +97,3 @@ box.sql.execute("ALTER TABLE t3 DROP CONSTRAINT fk1;")
box.sql.execute("INSERT INTO t3 VALUES(1, 1, 3);")
errinj.set("ERRINJ_WAL_IO", false)
box.sql.execute("DROP TABLE t3;")
-
---
--- gh-3832: Some statements do not return column type
-
--- This test placed here because it should be skipped in release
--- build.
-
--- Check that "PRAGMA parser_trace" returns 0 or 1 if called
--- without parameter.
-result = box.sql.execute('PRAGMA parser_trace')
--- Should be TRUE.
-result[1][1] == 0 or result[1][1] == 1
-
--- Check that "PRAGMA parser_trace" returns nothing if called
--- with parameter.
-box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
new file mode 100644
index 0000000..692fa82
--- /dev/null
+++ b/test/sql/sql-debug.result
@@ -0,0 +1,32 @@
+remote = require('net.box')
+---
+...
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+---
+...
+--
+-- gh-3832: Some statements do not return column type
+-- Check that "PRAGMA parser_trace" returns 0 or 1 if called
+-- without parameter.
+result = box.sql.execute('PRAGMA parser_trace')
+---
+...
+-- Should be nothing.
+box.sql.execute('PRAGMA parser_trace = 1')
+---
+...
+-- Should be 1.
+box.sql.execute('PRAGMA parser_trace')
+---
+- - [1]
+...
+-- Should be nothing.
+box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
+---
+...
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
new file mode 100644
index 0000000..66f47b3
--- /dev/null
+++ b/test/sql/sql-debug.test.lua
@@ -0,0 +1,17 @@
+remote = require('net.box')
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+--
+-- gh-3832: Some statements do not return column type
+
+-- Check that "PRAGMA parser_trace" returns 0 or 1 if called
+-- without parameter.
+result = box.sql.execute('PRAGMA parser_trace')
+-- Should be nothing.
+box.sql.execute('PRAGMA parser_trace = 1')
+-- Should be 1.
+box.sql.execute('PRAGMA parser_trace')
+-- Should be nothing.
+box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
diff --git a/test/sql/suite.ini b/test/sql/suite.ini
index 4504731..ce6ccb7 100644
--- a/test/sql/suite.ini
+++ b/test/sql/suite.ini
@@ -6,4 +6,4 @@ use_unix_sockets = True
config = engine.cfg
is_parallel = True
lua_libs = lua/sql_tokenizer.lua
-release_disabled = errinj.test.lua view_delayed_wal.test.lua
+release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua
New version:
commit efa1434cc4186a026f416f7506319f89009fd029
Author: Mergen Imeev <imeevma@gmail.com>
Date: Wed Dec 12 22:16:33 2018 +0300
sql: fix "PRAGMA parser_trace" result
Currently PRAGMA parser_trace returns an empty table. This seems
wrong, since other similar pragmas return their status. Fixed in
the current patch.
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 5729fe6..6122986 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -466,33 +466,31 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
switch (pPragma->ePragTyp) {
case PragTyp_FLAG:{
- if (zRight == 0) {
- setPragmaResultColumnNames(v, pPragma);
- returnSingleInt(v,
- (user_session->
- sql_flags & pPragma->iArg) !=
- 0);
- } else {
- int mask = pPragma->iArg; /* Mask of bits to set
- * or clear.
- */
-
- if (sqlite3GetBoolean(zRight, 0)) {
- user_session->sql_flags |= mask;
- } else {
- user_session->sql_flags &= ~mask;
- }
-
- /* Many of the flag-pragmas modify the code
- * generated by the SQL * compiler (eg.
- * count_changes). So add an opcode to expire
- * all * compiled SQL statements after
- * modifying a pragma value.
- */
- sqlite3VdbeAddOp0(v, OP_Expire);
+ if (zRight == 0) {
+ setPragmaResultColumnNames(v, pPragma);
+ returnSingleInt(v, (user_session->sql_flags &
+ pPragma->iArg) != 0);
+ } else {
+ int mask = pPragma->iArg; /* Mask of bits to set
+ * or clear.
+ */
+ bool is_value_true = sqlite3GetBoolean(zRight, 0);
+
+ if (is_value_true)
+ user_session->sql_flags |= mask;
+ else
+ user_session->sql_flags &= ~mask;
+#ifndef NDEBUG
+ if (mask == PARSER_TRACE_FLAG) {
+ if (is_value_true)
+ sqlite3ParserTrace(stdout, "parser: ");
+ else
+ sqlite3ParserTrace(0, 0);
}
- break;
+#endif
}
+ break;
+ }
case PragTyp_TABLE_INFO:
sql_pragma_table_info(pParse, zRight);
@@ -569,18 +567,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
}
break;
}
-#ifndef NDEBUG
- case PragTyp_PARSER_TRACE:{
- if (zRight) {
- if (sqlite3GetBoolean(zRight, 0)) {
- sqlite3ParserTrace(stdout, "parser: ");
- } else {
- sqlite3ParserTrace(0, 0);
- }
- }
- break;
- }
-#endif
/*
* Reinstall the LIKE and functions. The variant
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 84ab478..ae8e030 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -14,7 +14,6 @@
#define PragTyp_INDEX_LIST 11
#define PragTyp_STATS 15
#define PragTyp_TABLE_INFO 17
-#define PragTyp_PARSER_TRACE 24
#define PragTyp_DEFAULT_ENGINE 25
#define PragTyp_COMPOUND_SELECT_LIMIT 26
@@ -137,10 +136,10 @@ static const PragmaName aPragmaName[] = {
/* iArg: */ 0},
#if defined(SQLITE_DEBUG)
{ /* zName: */ "parser_trace",
- /* ePragTyp: */ PragTyp_PARSER_TRACE,
- /* ePragFlg: */ 0,
+ /* ePragTyp: */ PragTyp_FLAG,
+ /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
- /* iArg: */ 0},
+ /* iArg: */ PARSER_TRACE_FLAG},
#endif
{ /* zName: */ "query_only",
/* ePragTyp: */ PragTyp_FLAG,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 4110a59..a484486 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1563,6 +1563,8 @@ struct sqlite3 {
* Possible values for the sqlite3.flags.
*/
#define SQLITE_VdbeTrace 0x00000001 /* True to trace VDBE execution */
+/* Debug print info about SQL query as it parsed */
+#define PARSER_TRACE_FLAG 0x00000002
#define SQLITE_FullColNames 0x00000004 /* Show full column names on SELECT */
#define SQLITE_ShortColNames 0x00000040 /* Show short columns names */
#define SQLITE_CountRows 0x00000080 /* Count rows changed by INSERT, */
diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
new file mode 100644
index 0000000..692fa82
--- /dev/null
+++ b/test/sql/sql-debug.result
@@ -0,0 +1,32 @@
+remote = require('net.box')
+---
+...
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+---
+...
+--
+-- gh-3832: Some statements do not return column type
+-- Check that "PRAGMA parser_trace" returns 0 or 1 if called
+-- without parameter.
+result = box.sql.execute('PRAGMA parser_trace')
+---
+...
+-- Should be nothing.
+box.sql.execute('PRAGMA parser_trace = 1')
+---
+...
+-- Should be 1.
+box.sql.execute('PRAGMA parser_trace')
+---
+- - [1]
+...
+-- Should be nothing.
+box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
+---
+...
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
new file mode 100644
index 0000000..66f47b3
--- /dev/null
+++ b/test/sql/sql-debug.test.lua
@@ -0,0 +1,17 @@
+remote = require('net.box')
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+--
+-- gh-3832: Some statements do not return column type
+
+-- Check that "PRAGMA parser_trace" returns 0 or 1 if called
+-- without parameter.
+result = box.sql.execute('PRAGMA parser_trace')
+-- Should be nothing.
+box.sql.execute('PRAGMA parser_trace = 1')
+-- Should be 1.
+box.sql.execute('PRAGMA parser_trace')
+-- Should be nothing.
+box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
diff --git a/test/sql/suite.ini b/test/sql/suite.ini
index 4504731..ce6ccb7 100644
--- a/test/sql/suite.ini
+++ b/test/sql/suite.ini
@@ -6,4 +6,4 @@ use_unix_sockets = True
config = engine.cfg
is_parallel = True
lua_libs = lua/sql_tokenizer.lua
-release_disabled = errinj.test.lua view_delayed_wal.test.lua
+release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] [PATCH v3 3/6] sql: Show currently set sql_default_engine
2018-12-26 18:17 [tarantool-patches] [PATCH v3 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2018-12-26 18:17 ` [tarantool-patches] [PATCH v3 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 2/6] sql: fix "PRAGMA parser_trace" result imeevma
@ 2018-12-26 18:18 ` imeevma
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: imeevma @ 2018-12-26 18:18 UTC (permalink / raw)
To: tarantool-patches, korablev
After this patch, "PRAGMA sql_default_engine" called without
arguments will return currently set sql_default_engine.
---
src/box/sql/pragma.c | 18 +++++++++++++-----
test/sql-tap/gh-2367-pragma.test.lua | 35 +++++++++++++++++++++++++++--------
2 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 6122986..9b68c13 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -583,12 +583,20 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
}
case PragTyp_DEFAULT_ENGINE: {
- if (sql_default_engine_set(zRight) != 0) {
- pParse->rc = SQL_TARANTOOL_ERROR;
- pParse->nErr++;
- goto pragma_out;
+ if (zRight == NULL) {
+ const char *engine_name =
+ sql_storage_engine_strs[current_session()->
+ sql_default_engine];
+ sqlite3VdbeLoadString(v, 1, engine_name);
+ sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 1);
+ } else {
+ if (sql_default_engine_set(zRight) != 0) {
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
+ goto pragma_out;
+ }
+ sqlite3VdbeAddOp0(v, OP_Expire);
}
- sqlite3VdbeAddOp0(v, OP_Expire);
break;
}
diff --git a/test/sql-tap/gh-2367-pragma.test.lua b/test/sql-tap/gh-2367-pragma.test.lua
index c0792c9..90ecd56 100755
--- a/test/sql-tap/gh-2367-pragma.test.lua
+++ b/test/sql-tap/gh-2367-pragma.test.lua
@@ -1,7 +1,7 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(7)
+test:plan(8)
test:do_catchsql_test(
"pragma-1.3",
@@ -41,25 +41,44 @@ test:do_catchsql_test(
test:do_catchsql_test(
"pragma-2.4",
[[
- pragma sql_default_engine;
+ pragma sql_default_engine 'memtx';
]], {
- 1, 'Illegal parameters, \'sql_default_engine\' was not specified'
+ 1, 'near \"\'memtx\'\": syntax error'
})
test:do_catchsql_test(
"pragma-2.5",
[[
- pragma sql_default_engine 'memtx';
+ pragma sql_default_engine 1;
]], {
- 1, 'near \"\'memtx\'\": syntax error'
+ 1, 'near \"1\": syntax error'
})
+--
+-- gh-3832: Some statements do not return column type
+--
+-- Check that "PRAGMA sql_default_engine" called without arguments
+-- returns currently set sql_default_engine.
test:do_catchsql_test(
- "pragma-2.5",
+ "pragma-3.1",
[[
- pragma sql_default_engine 1;
+ pragma sql_default_engine='vinyl';
+ pragma sql_default_engine;
]], {
- 1, 'near \"1\": syntax error'
+ -- <pragma-3.1>
+ 0, {'vinyl'}
+ -- <pragma-3.1>
+})
+
+test:do_catchsql_test(
+ "pragma-3.2",
+ [[
+ pragma sql_default_engine='memtx';
+ pragma sql_default_engine;
+ ]], {
+ -- <pragma-3.2>
+ 0, {'memtx'}
+ -- <pragma-3.2>
})
test:finish_test()
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] [PATCH v3 4/6] sql: fix "PRAGMA case_sensitive_like" result
2018-12-26 18:17 [tarantool-patches] [PATCH v3 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
` (2 preceding siblings ...)
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 3/6] sql: Show currently set sql_default_engine imeevma
@ 2018-12-26 18:18 ` imeevma
2019-01-16 15:35 ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 5/6] sql: 'PRAGMA' result in the appropriate format imeevma
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
5 siblings, 1 reply; 12+ messages in thread
From: imeevma @ 2018-12-26 18:18 UTC (permalink / raw)
To: tarantool-patches, korablev
Hi! Thank you for review! My answers, diff between versions and
new version below.
On 12/24/18 5:01 PM, n.pettik wrote:
> Nit: since pragma is called ‘case_sensitive_like’, I would revert variable
> meaning:
This part was removed in new version.
> Also, the same question here: why you didn’t make it be of
> type ‘flag’? The only difference in additional call of sqlite3RegisterLikeFunctions.
Done. Moved to 'flag'.
> Lets avoid using ’sqlite’ prefixes for added code.
Fixed.
Diff between version:
commit 4662242039ad7c02b5401ecc5e21c3aa4707039f
Author: Mergen Imeev <imeevma@gmail.com>
Date: Tue Dec 25 19:17:11 2018 +0300
Temporary: review fixes 2
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 2651cad..67defed 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -488,6 +488,15 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
sqlite3ParserTrace(0, 0);
}
#endif
+ /*
+ * Reinstall the LIKE and functions. The
+ * variant of LIKE * used will be case
+ * sensitive or not depending on the RHS.
+ */
+ if (mask == LIKE_CASE_SENS_FLAG) {
+ sqlite3RegisterLikeFunctions(db,
+ !is_value_true);
+ }
}
break;
}
@@ -568,27 +577,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
break;
}
- /*
- * Reinstall the LIKE and functions. The variant
- * of LIKE * used will be case sensitive or not
- * depending on the RHS.
- */
- case PragTyp_CASE_SENSITIVE_LIKE:{
- int mask = pPragma->iArg;
- if (zRight == NULL) {
- returnSingleInt(v,
- (user_session->sql_flags & mask) != 0);
- } else {
- int is_like_ci = !(sqlite3GetBoolean(zRight, 0));
- if (!is_like_ci)
- user_session->sql_flags |= mask;
- else
- user_session->sql_flags &= ~mask;
- sqlite3RegisterLikeFunctions(db, is_like_ci);
- }
- break;
- }
-
case PragTyp_DEFAULT_ENGINE: {
if (zRight == NULL) {
const char *engine_name =
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index fed4b33..07700e7 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -6,7 +6,6 @@
/* The various pragma types */
#define PragTyp_BUSY_TIMEOUT 1
-#define PragTyp_CASE_SENSITIVE_LIKE 2
#define PragTyp_COLLATION_LIST 3
#define PragTyp_FLAG 5
#define PragTyp_FOREIGN_KEY_LIST 9
@@ -92,10 +91,10 @@ static const PragmaName aPragmaName[] = {
/* ColNames: */ 31, 1,
/* iArg: */ 0},
{ /* zName: */ "case_sensitive_like",
- /* ePragTyp: */ PragTyp_CASE_SENSITIVE_LIKE,
+ /* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
- /* iArg: */ SQLITE_LikeIsCaseSens},
+ /* iArg: */ LIKE_CASE_SENS_FLAG},
{ /* zName: */ "collation_list",
/* ePragTyp: */ PragTyp_COLLATION_LIST,
/* ePragFlg: */ PragFlg_Result0,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 334b12e..9c53fe9 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1565,9 +1565,9 @@ struct sqlite3 {
#define SQLITE_VdbeTrace 0x00000001 /* True to trace VDBE execution */
/* Debug print info about SQL query as it parsed */
#define PARSER_TRACE_FLAG 0x00000002
-/* True if LIKE is case sensitive. */
-#define SQLITE_LikeIsCaseSens 0x00000008
#define SQLITE_FullColNames 0x00000004 /* Show full column names on SELECT */
+/* True if LIKE is case sensitive. */
+#define LIKE_CASE_SENS_FLAG 0x00000008
#define SQLITE_ShortColNames 0x00000040 /* Show short columns names */
#define SQLITE_CountRows 0x00000080 /* Count rows changed by INSERT, */
/* DELETE, or UPDATE and return */
diff --git a/test/sql/misc.result b/test/sql/misc.result
index f1031f7..66f5a7b 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -47,13 +47,16 @@ box.sql.execute('\n\n\n\t\t\t ')
result = box.sql.execute('PRAGMA case_sensitive_like')
---
...
--- Should be TRUE.
-result[1][1] == 0 or result[1][1] == 1
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = 1')
---
-- true
...
--- Check that "PRAGMA case_sensitive_like" returns nothing if
--- called with parameter.
+-- Should be 1.
+box.sql.execute('PRAGMA case_sensitive_like')
+---
+- - [1]
+...
+-- Should be nothing.
box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
---
...
diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
index 5952035..cc31a5d 100644
--- a/test/sql/misc.test.lua
+++ b/test/sql/misc.test.lua
@@ -18,9 +18,9 @@ box.sql.execute('\n\n\n\t\t\t ')
-- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
-- called without parameter.
result = box.sql.execute('PRAGMA case_sensitive_like')
--- Should be TRUE.
-result[1][1] == 0 or result[1][1] == 1
-
--- Check that "PRAGMA case_sensitive_like" returns nothing if
--- called with parameter.
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = 1')
+-- Should be 1.
+box.sql.execute('PRAGMA case_sensitive_like')
+-- Should be nothing.
box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
New version:
commit 2589721198edcf039cfdc98a18d9e0670e6e67af
Author: Mergen Imeev <imeevma@gmail.com>
Date: Thu Dec 13 15:18:54 2018 +0300
sql: fix "PRAGMA case_sensitive_like" result
Currently PRAGMA case_sensitive_like returns nothing. This seems
wrong, since other similar pragmas return their status. Fixed in
the current patch.
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 9b68c13..67defed 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -488,6 +488,15 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
sqlite3ParserTrace(0, 0);
}
#endif
+ /*
+ * Reinstall the LIKE and functions. The
+ * variant of LIKE * used will be case
+ * sensitive or not depending on the RHS.
+ */
+ if (mask == LIKE_CASE_SENS_FLAG) {
+ sqlite3RegisterLikeFunctions(db,
+ !is_value_true);
+ }
}
break;
}
@@ -568,20 +577,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
break;
}
- /*
- * Reinstall the LIKE and functions. The variant
- * of LIKE * used will be case sensitive or not
- * depending on the RHS.
- */
- case PragTyp_CASE_SENSITIVE_LIKE:{
- if (zRight) {
- int is_like_ci =
- !(sqlite3GetBoolean(zRight, 0));
- sqlite3RegisterLikeFunctions(db, is_like_ci);
- }
- break;
- }
-
case PragTyp_DEFAULT_ENGINE: {
if (zRight == NULL) {
const char *engine_name =
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index ae8e030..07700e7 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -6,7 +6,6 @@
/* The various pragma types */
#define PragTyp_BUSY_TIMEOUT 1
-#define PragTyp_CASE_SENSITIVE_LIKE 2
#define PragTyp_COLLATION_LIST 3
#define PragTyp_FLAG 5
#define PragTyp_FOREIGN_KEY_LIST 9
@@ -92,10 +91,10 @@ static const PragmaName aPragmaName[] = {
/* ColNames: */ 31, 1,
/* iArg: */ 0},
{ /* zName: */ "case_sensitive_like",
- /* ePragTyp: */ PragTyp_CASE_SENSITIVE_LIKE,
- /* ePragFlg: */ PragFlg_NoColumns,
+ /* ePragTyp: */ PragTyp_FLAG,
+ /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
/* ColNames: */ 0, 0,
- /* iArg: */ 0},
+ /* iArg: */ LIKE_CASE_SENS_FLAG},
{ /* zName: */ "collation_list",
/* ePragTyp: */ PragTyp_COLLATION_LIST,
/* ePragFlg: */ PragFlg_Result0,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index a484486..9c53fe9 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1566,6 +1566,8 @@ struct sqlite3 {
/* Debug print info about SQL query as it parsed */
#define PARSER_TRACE_FLAG 0x00000002
#define SQLITE_FullColNames 0x00000004 /* Show full column names on SELECT */
+/* True if LIKE is case sensitive. */
+#define LIKE_CASE_SENS_FLAG 0x00000008
#define SQLITE_ShortColNames 0x00000040 /* Show short columns names */
#define SQLITE_CountRows 0x00000080 /* Count rows changed by INSERT, */
/* DELETE, or UPDATE and return */
diff --git a/test/sql/misc.result b/test/sql/misc.result
index ef104c1..66f5a7b 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -40,3 +40,23 @@ box.sql.execute('\n\n\n\t\t\t ')
---
- error: 'syntax error: empty request'
...
+--
+-- gh-3832: Some statements do not return column type
+-- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
+-- called without parameter.
+result = box.sql.execute('PRAGMA case_sensitive_like')
+---
+...
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = 1')
+---
+...
+-- Should be 1.
+box.sql.execute('PRAGMA case_sensitive_like')
+---
+- - [1]
+...
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
+---
+...
diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
index 994e64f..cc31a5d 100644
--- a/test/sql/misc.test.lua
+++ b/test/sql/misc.test.lua
@@ -11,3 +11,16 @@ box.sql.execute(';')
box.sql.execute('')
box.sql.execute(' ;')
box.sql.execute('\n\n\n\t\t\t ')
+
+--
+-- gh-3832: Some statements do not return column type
+
+-- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
+-- called without parameter.
+result = box.sql.execute('PRAGMA case_sensitive_like')
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = 1')
+-- Should be 1.
+box.sql.execute('PRAGMA case_sensitive_like')
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] [PATCH v3 5/6] sql: 'PRAGMA' result in the appropriate format
2018-12-26 18:17 [tarantool-patches] [PATCH v3 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
` (3 preceding siblings ...)
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
@ 2018-12-26 18:18 ` imeevma
2019-01-16 15:35 ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
5 siblings, 1 reply; 12+ messages in thread
From: imeevma @ 2018-12-26 18:18 UTC (permalink / raw)
To: tarantool-patches, korablev
Hi! Thank you for review! My answers, diff between versions and
new version below.
On 12/24/18 5:02 PM, n.pettik wrote:
> There is no ’Tarantool format’...
Changed to "more appropriate".
> Why do you consider it as wrong? AFAIR I made it exclusively to
> extend debug facilities.
>
> Things like
> - ['index_info', null]
> - ['index_list', null]
>
> look quite strange IMO. What is more, it may be misleading:
> index_list with null arg looks like there is no indexes at all.
Changed rows with values == NULL. Now instean of NULL they shows
"No value".
> Btw I don’t understand all these efforts to make pragma
> better. As I already said we are going to remove it anyway.
I thought it would be correct, since it can be used by the user.
> Name seems to be a mess, lets rename it to ’sql_pragma_status’ or
> ‘vdbe_emit_pragma_status’.
Changed name to ‘vdbe_emit_pragma_status’.
> This test look ridiculous: number of elements in res
> doesn’t mean anything; first element with ‘busy_timeout’
> value as well. I guess you are willing to avoid fixing
> this test each time number/content of pragmes are changed.
> And it this acceptable solution. On the other hand,
> it barely tests anything. So I propose to display full
> content of pragma.
I changed the test and moved it to the previously created file
sql-debug.test.lua. The new test shows a table with the first
column of the command result. I have not included the values here,
because they may differ from run to run.
Diff between version:
commit 8ec19ea1a9fe10a5329ae047369ec8ebd3980258
Author: Mergen Imeev <imeevma@gmail.com>
Date: Tue Dec 25 19:28:20 2018 +0300
Temporary: review fixes 3
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 62ba43b..2ad5806 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -169,7 +169,7 @@ pragmaLocate(const char *zName)
}
static void
-sql_pragma_active_pragmas(struct Parse *parse)
+vdbe_emit_pragma_status(struct Parse *parse)
{
struct Vdbe *v = sqlite3GetVdbe(parse);
struct session *user_session = current_session();
@@ -186,8 +186,6 @@ sql_pragma_active_pragmas(struct Parse *parse)
sqlite3VdbeAddOp4(v, OP_String8, 0, 1, 0, aPragmaName[i].zName,
0);
switch (aPragmaName[i].ePragTyp) {
- case PragTyp_PARSER_TRACE:
- case PragTyp_CASE_SENSITIVE_LIKE:
case PragTyp_FLAG: {
const char *value;
if ((user_session->sql_flags &
@@ -219,8 +217,11 @@ sql_pragma_active_pragmas(struct Parse *parse)
sqlite3VdbeAddOp2(v, OP_Cast, 2, AFFINITY_TEXT);
break;
}
- default:
- sqlite3VdbeAddOp2(v, OP_Null, 0, 2);
+ default: {
+ const char *value = "No value";
+ sqlite3VdbeAddOp4(v, OP_String8, 0, 2, 0, value,
+ 0);
+ }
}
sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 2);
}
@@ -451,7 +452,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
zLeft = sqlite3NameFromToken(db, pId);
if (!zLeft) {
- sql_pragma_active_pragmas(pParse);
+ vdbe_emit_pragma_status(pParse);
return;
}
diff --git a/test/sql/misc.result b/test/sql/misc.result
index bcbdec1..66f5a7b 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -60,15 +60,3 @@ box.sql.execute('PRAGMA case_sensitive_like')
box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
---
...
--- Make command "PRAGMA" return result in Tarantool format.
-res = box.sql.execute('PRAGMA')
----
-...
-#res > 0
----
-- true
-...
-res[1][1]
----
-- busy_timeout
-...
diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
index 87224c4..cc31a5d 100644
--- a/test/sql/misc.test.lua
+++ b/test/sql/misc.test.lua
@@ -24,8 +24,3 @@ box.sql.execute('PRAGMA case_sensitive_like = 1')
box.sql.execute('PRAGMA case_sensitive_like')
-- Should be nothing.
box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
-
--- Make command "PRAGMA" return result in Tarantool format.
-res = box.sql.execute('PRAGMA')
-#res > 0
-res[1][1]
diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
index 692fa82..9c5c9b3 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -30,3 +30,43 @@ box.sql.execute('PRAGMA parser_trace')
box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
---
...
+--
+-- Make PRAGMA command return the result in a more appropriate
+-- format.
+--
+result = box.sql.execute('PRAGMA')
+---
+...
+for _,v in pairs(result) do v[2] = nil end
+---
+...
+result
+---
+- - ['busy_timeout']
+ - ['case_sensitive_like']
+ - ['collation_list']
+ - ['count_changes']
+ - ['defer_foreign_keys']
+ - ['foreign_key_list']
+ - ['full_column_names']
+ - ['index_info']
+ - ['index_list']
+ - ['parser_trace']
+ - ['query_only']
+ - ['read_uncommitted']
+ - ['recursive_triggers']
+ - ['reverse_unordered_selects']
+ - ['select_trace']
+ - ['short_column_names']
+ - ['sql_compound_select_limit']
+ - ['sql_default_engine']
+ - ['sql_trace']
+ - ['stats']
+ - ['table_info']
+ - ['vdbe_addoptrace']
+ - ['vdbe_debug']
+ - ['vdbe_eqp']
+ - ['vdbe_listing']
+ - ['vdbe_trace']
+ - ['where_trace']
+...
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
index 66f47b3..e145d0c 100644
--- a/test/sql/sql-debug.test.lua
+++ b/test/sql/sql-debug.test.lua
@@ -15,3 +15,11 @@ box.sql.execute('PRAGMA parser_trace = 1')
box.sql.execute('PRAGMA parser_trace')
-- Should be nothing.
box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
+
+--
+-- Make PRAGMA command return the result in a more appropriate
+-- format.
+--
+result = box.sql.execute('PRAGMA')
+for _,v in pairs(result) do v[2] = nil end
+result
New version:
commit 1b0d86990ecf8c9b310cdfdc58ec9006c77c038a
Author: Mergen Imeev <imeevma@gmail.com>
Date: Thu Dec 13 21:07:31 2018 +0300
sql: 'PRAGMA' result in the appropriate format
Currently, box.sql.execute('PRAGMA') returns nothing but it prints
some information on stdout. This looks wrong. This patch makes the
command to return result in more appropriate format.
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 67defed..2ad5806 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -168,48 +168,62 @@ pragmaLocate(const char *zName)
return lwr > upr ? 0 : &aPragmaName[mid];
}
-#ifdef PRINT_PRAGMA
-#undef PRINT_PRAGMA
-#endif
-#define PRINT_PRAGMA(pragma_name, pragma_flag) do { \
- int nCoolSpaces = 30 - strlen(pragma_name); \
- if (user_session->sql_flags & (pragma_flag)) { \
- printf("%s %*c -- [true] \n", pragma_name, nCoolSpaces, ' '); \
- } else { \
- printf("%s %*c -- [false] \n", pragma_name, nCoolSpaces, ' ');\
- } \
-} while (0)
-
-#define PRINT_STR_PRAGMA(pragma_name, str_value) do { \
- int nCoolSpaces = 30 - strlen(pragma_name); \
- printf("%s %*c -- '%s' \n", pragma_name, nCoolSpaces, ' ', str_value);\
-} while (0)
-
static void
-printActivePragmas(struct session *user_session)
+vdbe_emit_pragma_status(struct Parse *parse)
{
- int i;
- for (i = 0; i < ArraySize(aPragmaName); ++i) {
+ struct Vdbe *v = sqlite3GetVdbe(parse);
+ struct session *user_session = current_session();
+
+ sqlite3VdbeSetNumCols(v, 2);
+ sqlite3VdbeSetColName(v, 0, COLNAME_NAME, "pragma_name", SQLITE_STATIC);
+ sqlite3VdbeSetColName(v, 0, COLNAME_DECLTYPE, "TEXT", SQLITE_STATIC);
+ sqlite3VdbeSetColName(v, 1, COLNAME_NAME, "pragma_value",
+ SQLITE_STATIC);
+ sqlite3VdbeSetColName(v, 1, COLNAME_DECLTYPE, "TEXT", SQLITE_STATIC);
+
+ parse->nMem = 2;
+ for (int i = 0; i < ArraySize(aPragmaName); ++i) {
+ sqlite3VdbeAddOp4(v, OP_String8, 0, 1, 0, aPragmaName[i].zName,
+ 0);
switch (aPragmaName[i].ePragTyp) {
- case PragTyp_FLAG:
- PRINT_PRAGMA(aPragmaName[i].zName, aPragmaName[i].iArg);
+ case PragTyp_FLAG: {
+ const char *value;
+ if ((user_session->sql_flags &
+ aPragmaName[i].iArg) != 0)
+ value = "true";
+ else
+ value = "false";
+ sqlite3VdbeAddOp4(v, OP_String8, 0, 2, 0, value,
+ 0);
break;
+ }
case PragTyp_DEFAULT_ENGINE: {
- const char *engine_name =
- sql_storage_engine_strs[
- current_session()->
- sql_default_engine];
- PRINT_STR_PRAGMA(aPragmaName[i].zName,
- engine_name);
+ const char *value = sql_storage_engine_strs[
+ user_session->sql_default_engine];
+ sqlite3VdbeAddOp4(v, OP_String8, 0, 2, 0, value,
+ 0);
+ break;
+ }
+ case PragTyp_BUSY_TIMEOUT: {
+ int value = parse->db->busyTimeout;
+ sqlite3VdbeAddOp2(v, OP_Integer, value, 2);
+ sqlite3VdbeAddOp2(v, OP_Cast, 2, AFFINITY_TEXT);
+ break;
+ }
+ case PragTyp_COMPOUND_SELECT_LIMIT: {
+ int value = sqlite3_limit(parse->db,
+ SQL_LIMIT_COMPOUND_SELECT, -1);
+ sqlite3VdbeAddOp2(v, OP_Integer, value, 2);
+ sqlite3VdbeAddOp2(v, OP_Cast, 2, AFFINITY_TEXT);
break;
}
+ default: {
+ const char *value = "No value";
+ sqlite3VdbeAddOp4(v, OP_String8, 0, 2, 0, value,
+ 0);
+ }
}
- }
-
- printf("Other available pragmas: \n");
- for (i = 0; i < ArraySize(aPragmaName); ++i) {
- if (aPragmaName[i].ePragTyp != PragTyp_FLAG)
- printf("-- %s \n", aPragmaName[i].zName);
+ sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 2);
}
}
@@ -438,7 +452,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
zLeft = sqlite3NameFromToken(db, pId);
if (!zLeft) {
- printActivePragmas(user_session);
+ vdbe_emit_pragma_status(pParse);
return;
}
diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
index 692fa82..9c5c9b3 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -30,3 +30,43 @@ box.sql.execute('PRAGMA parser_trace')
box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
---
...
+--
+-- Make PRAGMA command return the result in a more appropriate
+-- format.
+--
+result = box.sql.execute('PRAGMA')
+---
+...
+for _,v in pairs(result) do v[2] = nil end
+---
+...
+result
+---
+- - ['busy_timeout']
+ - ['case_sensitive_like']
+ - ['collation_list']
+ - ['count_changes']
+ - ['defer_foreign_keys']
+ - ['foreign_key_list']
+ - ['full_column_names']
+ - ['index_info']
+ - ['index_list']
+ - ['parser_trace']
+ - ['query_only']
+ - ['read_uncommitted']
+ - ['recursive_triggers']
+ - ['reverse_unordered_selects']
+ - ['select_trace']
+ - ['short_column_names']
+ - ['sql_compound_select_limit']
+ - ['sql_default_engine']
+ - ['sql_trace']
+ - ['stats']
+ - ['table_info']
+ - ['vdbe_addoptrace']
+ - ['vdbe_debug']
+ - ['vdbe_eqp']
+ - ['vdbe_listing']
+ - ['vdbe_trace']
+ - ['where_trace']
+...
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
index 66f47b3..e145d0c 100644
--- a/test/sql/sql-debug.test.lua
+++ b/test/sql/sql-debug.test.lua
@@ -15,3 +15,11 @@ box.sql.execute('PRAGMA parser_trace = 1')
box.sql.execute('PRAGMA parser_trace')
-- Should be nothing.
box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
+
+--
+-- Make PRAGMA command return the result in a more appropriate
+-- format.
+--
+result = box.sql.execute('PRAGMA')
+for _,v in pairs(result) do v[2] = nil end
+result
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] [PATCH v3 6/6] sql: set column types for EXPLAIN and PRAGMA
2018-12-26 18:17 [tarantool-patches] [PATCH v3 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
` (4 preceding siblings ...)
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 5/6] sql: 'PRAGMA' result in the appropriate format imeevma
@ 2018-12-26 18:18 ` imeevma
2019-01-16 15:35 ` [tarantool-patches] " n.pettik
5 siblings, 1 reply; 12+ messages in thread
From: imeevma @ 2018-12-26 18:18 UTC (permalink / raw)
To: tarantool-patches, korablev
Hi! Thank you for review! My answers, diff between versions and
new version below.
On 12/24/18 5:02 PM, n.pettik wrote:
> These two comments are too obvious, lets remove them.
Done.
>
>
>>
>> @@ -473,16 +468,14 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
>> }
>> /* Register the result column names for pragmas that return results */
>> if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0
>> - && ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0)
>> - ) {
>> - setPragmaResultColumnNames(v, pPragma);
>> - }
>> + && ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0))
>
> Nit: put operator on previous line:
Done.
> Nit: uppercase all SQL keywords pls. Otherwise it looks pretty ugly.
Done.
Diff between version:
commit 0f2d20f14096ba55460b866acf3cca72b069ffca
Author: Mergen Imeev <imeevma@gmail.com>
Date: Wed Dec 26 18:52:55 2018 +0300
Temporary: review fixes 4
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index d132d46..351bd06 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -120,10 +120,8 @@ vdbe_set_pragma_result_columns(struct Vdbe *v, const struct PragmaName *pragma)
assert(n > 0);
sqlite3VdbeSetNumCols(v, n);
for (int i = 0, j = pragma->iPragCName; i < n; ++i) {
- /* Value of j is index of column name in array */
sqlite3VdbeSetColName(v, i, COLNAME_NAME, pragCName[j++],
SQLITE_STATIC);
- /* Value of j is index of column type in array */
sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, pragCName[j++],
SQLITE_STATIC);
}
@@ -466,8 +464,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
goto pragma_out;
}
/* Register the result column names for pragmas that return results */
- if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0
- && ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0))
+ if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0 &&
+ ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0))
vdbe_set_pragma_result_columns(v, pPragma);
/* Jump to the appropriate pragma handler */
switch (pPragma->ePragTyp) {
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index e128001..0a17e4a 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -848,7 +848,7 @@ res.metadata
type: INTEGER
...
-- EXPLAIN
-res = cn:execute("EXPLAIN select 1")
+res = cn:execute("EXPLAIN SELECT 1")
---
...
res.metadata
@@ -870,7 +870,7 @@ res.metadata
- name: comment
type: TEXT
...
-res = cn:execute("EXPLAIN QUERY PLAN select count(*) from t1")
+res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1")
---
...
res.metadata
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 9f9a382..6fa5d95 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -272,9 +272,9 @@ res = cn:execute("PRAGMA table_info(t1)")
res.metadata
-- EXPLAIN
-res = cn:execute("EXPLAIN select 1")
+res = cn:execute("EXPLAIN SELECT 1")
res.metadata
-res = cn:execute("EXPLAIN QUERY PLAN select count(*) from t1")
+res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1")
res.metadata
cn:close()
New version:
commit c087a86c4da62b9b73361b8e483606facd2f92b2
Author: Mergen Imeev <imeevma@gmail.com>
Date: Wed Dec 5 14:30:49 2018 +0300
sql: set column types for EXPLAIN and PRAGMA
Currently, EXPLAIN and PRAGMA do not set the column types for the
result. This is incorrect, since any returned row must have a
column type. This patch defines the types for these columns.
Closes #3832
diff --git a/src/box/execute.c b/src/box/execute.c
index 7fff5fd..201d9c1 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -482,11 +482,12 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
const char *name = sqlite3_column_name(stmt, i);
const char *type = sqlite3_column_datatype(stmt, i);
/*
- * Can not fail, since all column names are
- * preallocated during prepare phase and the
+ * Can not fail, since all column names and types
+ * are preallocated during prepare phase and the
* column_name simply returns them.
*/
assert(name != NULL);
+ assert(type != NULL);
size += mp_sizeof_str(strlen(name));
size += mp_sizeof_str(strlen(type));
char *pos = (char *) obuf_alloc(out, size);
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 2ad5806..351bd06 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -112,25 +112,18 @@ sqlite3GetBoolean(const char *z, u8 dflt)
* the rest of the file if PRAGMAs are omitted from the build.
*/
-/*
- * Set result column names for a pragma.
- */
+/** Set result column names and types for a pragma. */
static void
-setPragmaResultColumnNames(Vdbe * v, /* The query under construction */
- const PragmaName * pPragma /* The pragma */
- )
+vdbe_set_pragma_result_columns(struct Vdbe *v, const struct PragmaName *pragma)
{
- u8 n = pPragma->nPragCName;
- sqlite3VdbeSetNumCols(v, n == 0 ? 1 : n);
- if (n == 0) {
- sqlite3VdbeSetColName(v, 0, COLNAME_NAME, pPragma->zName,
+ int n = pragma->nPragCName;
+ assert(n > 0);
+ sqlite3VdbeSetNumCols(v, n);
+ for (int i = 0, j = pragma->iPragCName; i < n; ++i) {
+ sqlite3VdbeSetColName(v, i, COLNAME_NAME, pragCName[j++],
+ SQLITE_STATIC);
+ sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, pragCName[j++],
SQLITE_STATIC);
- } else {
- int i, j;
- for (i = 0, j = pPragma->iPragCName; i < n; i++, j++) {
- sqlite3VdbeSetColName(v, i, COLNAME_NAME, pragCName[j],
- SQLITE_STATIC);
- }
}
}
@@ -471,17 +464,15 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
goto pragma_out;
}
/* Register the result column names for pragmas that return results */
- if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0
- && ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0)
- ) {
- setPragmaResultColumnNames(v, pPragma);
- }
+ if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0 &&
+ ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0))
+ vdbe_set_pragma_result_columns(v, pPragma);
/* Jump to the appropriate pragma handler */
switch (pPragma->ePragTyp) {
case PragTyp_FLAG:{
if (zRight == 0) {
- setPragmaResultColumnNames(v, pPragma);
+ vdbe_set_pragma_result_columns(v, pPragma);
returnSingleInt(v, (user_session->sql_flags &
pPragma->iArg) != 0);
} else {
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 07700e7..18dfceb 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -25,50 +25,142 @@
#define PragFlg_SchemaOpt 0x40 /* Schema restricts name search if present */
#define PragFlg_SchemaReq 0x80 /* Schema required - "main" is default */
-/* Names of columns for pragmas that return multi-column result
- * or that return single-column results where the name of the
- * result column is different from the name of the pragma
+/**
+ * Column names and types for pragmas. The type of the column is
+ * the following value after its name.
*/
static const char *const pragCName[] = {
/* Used by: table_info */
/* 0 */ "cid",
- /* 1 */ "name",
- /* 2 */ "type",
- /* 3 */ "notnull",
- /* 4 */ "dflt_value",
- /* 5 */ "pk",
+ /* 1 */ "INTEGER",
+ /* 2 */ "name",
+ /* 3 */ "TEXT",
+ /* 4 */ "type",
+ /* 3 */ "TEXT",
+ /* 6 */ "notnull",
+ /* 1 */ "INTEGER",
+ /* 8 */ "dflt_value",
+ /* 9 */ "TEXT",
+ /* 10 */ "pk",
+ /* 11 */ "INTEGER",
/* Used by: stats */
- /* 6 */ "table",
- /* 7 */ "index",
- /* 8 */ "width",
- /* 9 */ "height",
+ /* 12 */ "table",
+ /* 13 */ "TEXT",
+ /* 14 */ "index",
+ /* 15 */ "TEXT",
+ /* 16 */ "width",
+ /* 17 */ "INTEGER",
+ /* 18 */ "height",
+ /* 19 */ "INTEGER",
/* Used by: index_info */
- /* 10 */ "seqno",
- /* 11 */ "cid",
- /* 12 */ "name",
- /* 13 */ "desc",
- /* 14 */ "coll",
- /* 15 */ "type",
+ /* 20 */ "seqno",
+ /* 21 */ "INTEGER",
+ /* 22 */ "cid",
+ /* 23 */ "INTEGER",
+ /* 24 */ "name",
+ /* 25 */ "TEXT",
+ /* 26 */ "desc",
+ /* 27 */ "INTEGER",
+ /* 28 */ "coll",
+ /* 29 */ "TEXT",
+ /* 30 */ "type",
+ /* 31 */ "TEXT",
/* Used by: index_list */
- /* 16 */ "seq",
- /* 17 */ "name",
- /* 18 */ "unique",
- /* 19 */ "origin",
- /* 20 */ "partial",
+ /* 32 */ "seq",
+ /* 33 */ "INTEGER",
+ /* 34 */ "name",
+ /* 35 */ "TEXT",
+ /* 36 */ "unique",
+ /* 37 */ "INTEGER",
+ /* 38 */ "origin",
+ /* 39 */ "TEXT",
+ /* 40 */ "partial",
+ /* 41 */ "INTEGER",
/* Used by: collation_list */
- /* 21 */ "seq",
- /* 22 */ "name",
+ /* 42 */ "seq",
+ /* 43 */ "INTEGER",
+ /* 44 */ "name",
+ /* 45 */ "TEXT",
/* Used by: foreign_key_list */
- /* 23 */ "id",
- /* 24 */ "seq",
- /* 25 */ "table",
- /* 26 */ "from",
- /* 27 */ "to",
- /* 28 */ "on_update",
- /* 29 */ "on_delete",
- /* 30 */ "match",
+ /* 46 */ "id",
+ /* 47 */ "INTEGER",
+ /* 48 */ "seq",
+ /* 49 */ "INTEGER",
+ /* 50 */ "table",
+ /* 51 */ "TEXT",
+ /* 52 */ "from",
+ /* 53 */ "TEXT",
+ /* 54 */ "to",
+ /* 55 */ "TEXT",
+ /* 56 */ "on_update",
+ /* 57 */ "TEXT",
+ /* 58 */ "on_delete",
+ /* 59 */ "TEXT",
+ /* 60 */ "match",
+ /* 61 */ "TEXT",
/* Used by: busy_timeout */
- /* 31 */ "timeout",
+ /* 62 */ "timeout",
+ /* 63 */ "INTEGER",
+ /* Used by: case_sensitive_like */
+ /* 64 */ "case_sensitive_like",
+ /* 65 */ "INTEGER",
+ /* Used by: count_changes */
+ /* 66 */ "count_changes",
+ /* 67 */ "INTEGER",
+ /* Used by: defer_foreign_keys */
+ /* 68 */ "defer_foreign_keys",
+ /* 69 */ "INTEGER",
+ /* Used by: full_column_names */
+ /* 70 */ "full_column_names",
+ /* 71 */ "INTEGER",
+ /* Used by: parser_trace */
+ /* 72 */ "parser_trace",
+ /* 73 */ "INTEGER",
+ /* Used by: query_only */
+ /* 74 */ "query_only",
+ /* 75 */ "INTEGER",
+ /* Used by: read_uncommitted */
+ /* 76 */ "read_uncommitted",
+ /* 77 */ "INTEGER",
+ /* Used by: recursive_triggers */
+ /* 78 */ "recursive_triggers",
+ /* 79 */ "INTEGER",
+ /* Used by: reverse_unordered_selects */
+ /* 80 */ "reverse_unordered_selects",
+ /* 81 */ "INTEGER",
+ /* Used by: select_trace */
+ /* 82 */ "select_trace",
+ /* 83 */ "INTEGER",
+ /* Used by: short_column_names */
+ /* 84 */ "short_column_names",
+ /* 85 */ "INTEGER",
+ /* Used by: sql_compound_select_limit */
+ /* 86 */ "sql_compound_select_limit",
+ /* 87 */ "INTEGER",
+ /* Used by: sql_default_engine */
+ /* 88 */ "sql_default_engine",
+ /* 89 */ "TEXT",
+ /* Used by: sql_trace */
+ /* 90 */ "sql_trace",
+ /* 91 */ "INTEGER",
+ /* Used by: vdbe_addoptrace */
+ /* 92 */ "vdbe_addoptrace",
+ /* 93 */ "INTEGER",
+ /* Used by: vdbe_debug */
+ /* 94 */ "vdbe_debug",
+ /* 95 */ "INTEGER",
+ /* Used by: vdbe_eqp */
+ /* 96 */ "vdbe_eqp",
+ /* 97 */ "INTEGER",
+ /* Used by: vdbe_listing */
+ /* 98 */ "vdbe_listing",
+ /* 99 */ "INTEGER",
+ /* Used by: vdbe_trace */
+ /* 100 */ "vdbe_trace",
+ /* 101 */ "INTEGER",
+ /* Used by: where_trace */
+ /* 102 */ "where_trace",
+ /* 103 */ "INTEGER",
};
/* Definitions of all built-in pragmas */
@@ -77,7 +169,7 @@ typedef struct PragmaName {
u8 ePragTyp; /* PragTyp_XXX value */
u8 mPragFlg; /* Zero or more PragFlg_XXX values */
u8 iPragCName; /* Start of column names in pragCName[] */
- u8 nPragCName; /* Num of col names. 0 means use pragma name */
+ u8 nPragCName; /* Num of col names. */
u32 iArg; /* Extra argument */
} PragmaName;
/**
@@ -88,112 +180,112 @@ static const PragmaName aPragmaName[] = {
{ /* zName: */ "busy_timeout",
/* ePragTyp: */ PragTyp_BUSY_TIMEOUT,
/* ePragFlg: */ PragFlg_Result0,
- /* ColNames: */ 31, 1,
+ /* ColNames: */ 62, 1,
/* iArg: */ 0},
{ /* zName: */ "case_sensitive_like",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 64, 1,
/* iArg: */ LIKE_CASE_SENS_FLAG},
{ /* zName: */ "collation_list",
/* ePragTyp: */ PragTyp_COLLATION_LIST,
/* ePragFlg: */ PragFlg_Result0,
- /* ColNames: */ 21, 2,
+ /* ColNames: */ 42, 2,
/* iArg: */ 0},
{ /* zName: */ "count_changes",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 66, 1,
/* iArg: */ SQLITE_CountRows},
{ /* zName: */ "defer_foreign_keys",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 68, 1,
/* iArg: */ SQLITE_DeferFKs},
{ /* zName: */ "foreign_key_list",
/* ePragTyp: */ PragTyp_FOREIGN_KEY_LIST,
/* ePragFlg: */
PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
- /* ColNames: */ 23, 8,
+ /* ColNames: */ 46, 8,
/* iArg: */ 0},
{ /* zName: */ "full_column_names",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 70, 1,
/* iArg: */ SQLITE_FullColNames},
{ /* zName: */ "index_info",
/* ePragTyp: */ PragTyp_INDEX_INFO,
/* ePragFlg: */
PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
- /* ColNames: */ 10, 6,
+ /* ColNames: */ 20, 6,
/* iArg: */ 1},
{ /* zName: */ "index_list",
/* ePragTyp: */ PragTyp_INDEX_LIST,
/* ePragFlg: */
PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
- /* ColNames: */ 16, 5,
+ /* ColNames: */ 32, 5,
/* iArg: */ 0},
#if defined(SQLITE_DEBUG)
{ /* zName: */ "parser_trace",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 72, 1,
/* iArg: */ PARSER_TRACE_FLAG},
#endif
{ /* zName: */ "query_only",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 74, 1,
/* iArg: */ SQLITE_QueryOnly},
{ /* zName: */ "read_uncommitted",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 76, 1,
/* iArg: */ SQLITE_ReadUncommitted},
{ /* zName: */ "recursive_triggers",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 78, 1,
/* iArg: */ SQLITE_RecTriggers},
{ /* zName: */ "reverse_unordered_selects",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 80, 1,
/* iArg: */ SQLITE_ReverseOrder},
#if defined(SQLITE_ENABLE_SELECTTRACE)
{ /* zName: */ "select_trace",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 82, 1,
/* iArg: */ SQLITE_SelectTrace},
#endif
{ /* zName: */ "short_column_names",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 84, 1,
/* iArg: */ SQLITE_ShortColNames},
{ /* zName: */ "sql_compound_select_limit",
/* ePragTyp: */ PragTyp_COMPOUND_SELECT_LIMIT,
/* ePragFlg: */ PragFlg_Result0,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 86, 1,
/* iArg: */ 0},
{ /* zName: */ "sql_default_engine",
/* ePragTyp: */ PragTyp_DEFAULT_ENGINE,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 88, 1,
/* iArg: */ 0},
#if defined(SQLITE_DEBUG)
{ /* zName: */ "sql_trace",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 90, 1,
/* iArg: */ SQLITE_SqlTrace},
#endif
{ /* zName: */ "stats",
/* ePragTyp: */ PragTyp_STATS,
/* ePragFlg: */
PragFlg_NeedSchema | PragFlg_Result0 | PragFlg_SchemaReq,
- /* ColNames: */ 6, 4,
+ /* ColNames: */ 12, 4,
/* iArg: */ 0},
{ /* zName: */ "table_info",
/* ePragTyp: */ PragTyp_TABLE_INFO,
@@ -205,28 +297,28 @@ static const PragmaName aPragmaName[] = {
{ /* zName: */ "vdbe_addoptrace",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 92, 1,
/* iArg: */ SQLITE_VdbeAddopTrace},
{ /* zName: */ "vdbe_debug",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 94, 1,
/* iArg: */
SQLITE_SqlTrace | SQLITE_VdbeListing | SQLITE_VdbeTrace},
{ /* zName: */ "vdbe_eqp",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 96, 1,
/* iArg: */ SQLITE_VdbeEQP},
{ /* zName: */ "vdbe_listing",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 98, 1,
/* iArg: */ SQLITE_VdbeListing},
{ /* zName: */ "vdbe_trace",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 100, 1,
/* iArg: */ SQLITE_VdbeTrace},
#endif
#if defined(SQLITE_ENABLE_WHERETRACE)
@@ -234,7 +326,7 @@ static const PragmaName aPragmaName[] = {
{ /* zName: */ "where_trace",
/* ePragTyp: */ PragTyp_FLAG,
/* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1,
- /* ColNames: */ 0, 0,
+ /* ColNames: */ 102, 1,
/* iArg: */ SQLITE_WhereTrace},
#endif
};
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 824578e..87326da 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -54,7 +54,6 @@ sqlite3Prepare(sqlite3 * db, /* Database handle. */
{
char *zErrMsg = 0; /* Error message */
int rc = SQLITE_OK; /* Result code */
- int i; /* Loop counter */
Parse sParse; /* Parsing context */
sql_parser_create(&sParse, db);
sParse.pReprepare = pReprepare;
@@ -113,23 +112,48 @@ sqlite3Prepare(sqlite3 * db, /* Database handle. */
if (rc == SQLITE_OK && sParse.pVdbe && sParse.explain) {
static const char *const azColName[] = {
- "addr", "opcode", "p1", "p2", "p3", "p4", "p5",
- "comment",
- "selectid", "order", "from", "detail"
+ /* 0 */ "addr",
+ /* 1 */ "INTEGER",
+ /* 2 */ "opcode",
+ /* 3 */ "TEXT",
+ /* 4 */ "p1",
+ /* 5 */ "INTEGER",
+ /* 6 */ "p2",
+ /* 7 */ "INTEGER",
+ /* 8 */ "p3",
+ /* 9 */ "INTEGER",
+ /* 10 */ "p4",
+ /* 11 */ "TEXT",
+ /* 12 */ "p5",
+ /* 13 */ "TEXT",
+ /* 14 */ "comment",
+ /* 15 */ "TEXT",
+ /* 16 */ "selectid",
+ /* 17 */ "INTEGER",
+ /* 18 */ "order",
+ /* 19 */ "INTEGER",
+ /* 20 */ "from",
+ /* 21 */ "INTEGER",
+ /* 22 */ "detail",
+ /* 23 */ "TEXT",
};
- int iFirst, mx;
+
+ int name_first, name_count;
if (sParse.explain == 2) {
- sqlite3VdbeSetNumCols(sParse.pVdbe, 4);
- iFirst = 8;
- mx = 12;
+ name_first = 16;
+ name_count = 4;
} else {
- sqlite3VdbeSetNumCols(sParse.pVdbe, 8);
- iFirst = 0;
- mx = 8;
+ name_first = 0;
+ name_count = 8;
}
- for (i = iFirst; i < mx; i++) {
- sqlite3VdbeSetColName(sParse.pVdbe, i - iFirst,
- COLNAME_NAME, azColName[i],
+ sqlite3VdbeSetNumCols(sParse.pVdbe, name_count);
+ for (int i = 0; i < name_count; i++) {
+ int name_index = 2 * i + name_first;
+ sqlite3VdbeSetColName(sParse.pVdbe, i, COLNAME_NAME,
+ azColName[name_index],
+ SQLITE_STATIC);
+ sqlite3VdbeSetColName(sParse.pVdbe, i, COLNAME_DECLTYPE,
+ azColName[name_index + 1],
SQLITE_STATIC);
}
}
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index 9ace282..0a17e4a 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -821,6 +821,75 @@ box.sql.execute('DROP TABLE t1')
cn:close()
---
...
+-- gh-3832: Some statements do not return column type
+box.sql.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY)')
+---
+...
+cn = remote.connect(box.cfg.listen)
+---
+...
+-- PRAGMA:
+res = cn:execute("PRAGMA table_info(t1)")
+---
+...
+res.metadata
+---
+- - name: cid
+ type: INTEGER
+ - name: name
+ type: TEXT
+ - name: type
+ type: TEXT
+ - name: notnull
+ type: INTEGER
+ - name: dflt_value
+ type: TEXT
+ - name: pk
+ type: INTEGER
+...
+-- EXPLAIN
+res = cn:execute("EXPLAIN SELECT 1")
+---
+...
+res.metadata
+---
+- - name: addr
+ type: INTEGER
+ - name: opcode
+ type: TEXT
+ - name: p1
+ type: INTEGER
+ - name: p2
+ type: INTEGER
+ - name: p3
+ type: INTEGER
+ - name: p4
+ type: TEXT
+ - name: p5
+ type: TEXT
+ - name: comment
+ type: TEXT
+...
+res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1")
+---
+...
+res.metadata
+---
+- - name: selectid
+ type: INTEGER
+ - name: order
+ type: INTEGER
+ - name: from
+ type: INTEGER
+ - name: detail
+ type: TEXT
+...
+cn:close()
+---
+...
+box.sql.execute('DROP TABLE t1')
+---
+...
box.schema.user.revoke('guest', 'read,write,execute', 'universe')
---
...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 6640903..6fa5d95 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -263,10 +263,26 @@ box.sql.execute('DROP TABLE t1')
cn:close()
+-- gh-3832: Some statements do not return column type
+box.sql.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY)')
+cn = remote.connect(box.cfg.listen)
+
+-- PRAGMA:
+res = cn:execute("PRAGMA table_info(t1)")
+res.metadata
+
+-- EXPLAIN
+res = cn:execute("EXPLAIN SELECT 1")
+res.metadata
+res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1")
+res.metadata
+
+cn:close()
+box.sql.execute('DROP TABLE t1')
+
box.schema.user.revoke('guest', 'read,write,execute', 'universe')
box.schema.user.revoke('guest', 'create', 'space')
space = nil
-- Cleanup xlog
box.snapshot()
-
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH v3 1/6] sql: remove unused macros from pragma.c and pragma.h
2018-12-26 18:17 ` [tarantool-patches] [PATCH v3 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
@ 2019-01-16 15:34 ` n.pettik
0 siblings, 0 replies; 12+ messages in thread
From: n.pettik @ 2019-01-16 15:34 UTC (permalink / raw)
To: tarantool-patches; +Cc: Imeev Mergen
> On 26 Dec 2018, at 20:17, imeevma@tarantool.org wrote:
>
> Hi! Thank you for review! My answers and new version below. I
> didn't include diff, because all that changed was the commit
> message.
>
>
> On 12/24/18 5:01 PM, n.pettik wrote:
>> Nit: at the end of commit subject redundant ‘*’ and dot.
> Fixed.
>
>> They weren’t deleted, they are simply not used.
>> I guess they still can be enabled, if add appropriate commands
>> to cmake lists. On the other hand, I doubt that someday we
>> really may need them.
> Made changes in in commit-message.
>
>> In fact, this commit has nothing in common with mentioned issue.
> Removed from commit-message.
>
>> Why didn’t you remove ENABLE_SELECTTRACE as well?
> I didn't remove it because it can be set through cmake.
So? As I already said, I also was able to set OMIT_FLAG_PRAGMAS
Look:
diff --git a/src/box/sql/CMakeLists.txt b/src/box/sql/CMakeLists.txt
index 7f7b60e22..4d4cbb21f 100644
--- a/src/box/sql/CMakeLists.txt
+++ b/src/box/sql/CMakeLists.txt
@@ -1,6 +1,7 @@
if(CMAKE_BUILD_TYPE STREQUAL "Debug")
add_definitions(-DSQLITE_DEBUG=1)
add_definitions(-DSQLITE_ENABLE_SELECTTRACE)
+ add_definitions(-DSQLITE_OMIT_FLAG_PRAGMAS=1)
add_definitions(-DSQLITE_ENABLE_WHERETRACE)
endif()
Make sure that build is debug but flag-pragmas are disabled:
tarantool> \set language sql
---
- true
...
tarantool> pragma vdbe_debug=1
---
- error: 'no such pragma: VDBE_DEBUG'
...
tarantool> pragma parser_trace=1
parser: Shift 'cmd', go to state 419
parser: Shift 'SEMI'
parser: Return. Stack=[explain cmd SEMI]
parser: Input '$'
parser: Reduce [ecmd ::= explain cmdx SEMI], go to state 0.
parser: Shift 'ecmd', go to state 420
parser: Reduce [input ::= ecmd], go to state 0.
parser: Accept!
parser: Return. Stack=]
---
- []
...
Lets remove it and replace with simple ifdef debug.
The same if for ENABLE_WHERETRACE
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH v3 2/6] sql: fix "PRAGMA parser_trace" result
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 2/6] sql: fix "PRAGMA parser_trace" result imeevma
@ 2019-01-16 15:35 ` n.pettik
0 siblings, 0 replies; 12+ messages in thread
From: n.pettik @ 2019-01-16 15:35 UTC (permalink / raw)
To: tarantool-patches; +Cc: Imeev Mergen
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index b8edc76..6122986 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -466,33 +466,31 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
> switch (pPragma->ePragTyp) {
>
> case PragTyp_FLAG:{
> - if (zRight == 0) {
> - setPragmaResultColumnNames(v, pPragma);
> - returnSingleInt(v,
> - (user_session->
> - sql_flags & pPragma->iArg) !=
> - 0);
> - } else {
> - int mask = pPragma->iArg; /* Mask of bits to set
> - * or clear.
> - */
> -
> - if (sqlite3GetBoolean(zRight, 0)) {
> - user_session->sql_flags |= mask;
> - } else {
> - user_session->sql_flags &= ~mask;
> - }
> -
> - /* Many of the flag-pragmas modify the code
> - * generated by the SQL * compiler (eg.
> - * count_changes). So add an opcode to expire
> - * all * compiled SQL statements after
> - * modifying a pragma value.
> - */
> - sqlite3VdbeAddOp0(v, OP_Expire);
> + if (zRight == 0) {
Since you have already aligned code, I propose to fix it
according to our codestyle:
zRight == NULL
> + setPragmaResultColumnNames(v, pPragma);
> + returnSingleInt(v, (user_session->sql_flags &
> + pPragma->iArg) != 0);
> + } else {
> + int mask = pPragma->iArg; /* Mask of bits to set
> + * or clear.
> + */
Smth wrong with comment.
> + bool is_value_true = sqlite3GetBoolean(zRight, 0);
Bad name imho. I would call it “is_set” or “is_pragma_set” or “pragma_status”.
Explanation: variable itself is of type bool and “value” says nothing.
> diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
> new file mode 100644
> index 0000000..66f47b3
> --- /dev/null
> +++ b/test/sql/sql-debug.test.lua
> @@ -0,0 +1,17 @@
> +remote = require('net.box')
> +test_run = require('test_run').new()
> +engine = test_run:get_cfg('engine')
> +box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
> +
> +--
> +-- gh-3832: Some statements do not return column type
> +
> +-- Check that "PRAGMA parser_trace" returns 0 or 1 if called
> +-- without parameter.
> +result = box.sql.execute('PRAGMA parser_trace')
> +-- Should be nothing.
> +box.sql.execute('PRAGMA parser_trace = 1')
> +-- Should be 1.
> +box.sql.execute('PRAGMA parser_trace')
> +-- Should be nothing.
These three comments say nothing.
Results and mismatches can be seen from .result and
.reject files.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH v3 4/6] sql: fix "PRAGMA case_sensitive_like" result
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
@ 2019-01-16 15:35 ` n.pettik
0 siblings, 0 replies; 12+ messages in thread
From: n.pettik @ 2019-01-16 15:35 UTC (permalink / raw)
To: tarantool-patches; +Cc: Imeev Mergen
> diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
> index 994e64f..cc31a5d 100644
> --- a/test/sql/misc.test.lua
> +++ b/test/sql/misc.test.lua
> @@ -11,3 +11,16 @@ box.sql.execute(';')
> box.sql.execute('')
> box.sql.execute(' ;')
> box.sql.execute('\n\n\n\t\t\t ')
> +
> +--
> +-- gh-3832: Some statements do not return column type
> +
> +-- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
> +-- called without parameter.
> +result = box.sql.execute('PRAGMA case_sensitive_like')
> +-- Should be nothing.
> +box.sql.execute('PRAGMA case_sensitive_like = 1')
> +-- Should be 1.
> +box.sql.execute('PRAGMA case_sensitive_like')
> +-- Should be nothing.
Again: useless comments.
The rest is OK.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH v3 5/6] sql: 'PRAGMA' result in the appropriate format
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 5/6] sql: 'PRAGMA' result in the appropriate format imeevma
@ 2019-01-16 15:35 ` n.pettik
0 siblings, 0 replies; 12+ messages in thread
From: n.pettik @ 2019-01-16 15:35 UTC (permalink / raw)
To: tarantool-patches; +Cc: Imeev Mergen
>> There is no ’Tarantool format’...
> Changed to "more appropriate”.
Cmon, it is called YAML...
>
>> Why do you consider it as wrong? AFAIR I made it exclusively to
>> extend debug facilities.
>>
>> Things like
>> - ['index_info', null]
>> - ['index_list', null]
>>
>> look quite strange IMO. What is more, it may be misleading:
>> index_list with null arg looks like there is no indexes at all.
> Changed rows with values == NULL. Now instean of NULL they shows
> "No value”.
Now it is even worse.
I vote for displaying only flag pragmas and make
second field of resulting filed to be of type bool.
You can ask other members for their opinion.
>> Btw I don’t understand all these efforts to make pragma
>> better. As I already said we are going to remove it anyway.
> I thought it would be correct, since it can be used by the user.
It can be used by users until we remove it. That’s it.
> commit 1b0d86990ecf8c9b310cdfdc58ec9006c77c038a
> Author: Mergen Imeev <imeevma@gmail.com>
> Date: Thu Dec 13 21:07:31 2018 +0300
>
> sql: 'PRAGMA' result in the appropriate format
>
> Currently, box.sql.execute('PRAGMA') returns nothing but it prints
> some information on stdout.
Nit: be specific: not ’some information’ but list of pragmas
and their statuses.
Nit: print to stdout.
> This looks wrong.
"Such strategy is considered to be wrong since output
of this command would be unavailable for users who
redirect stdout, use net box connection etc."
> This patch makes the
> command to return result in more appropriate format.
"In yaml format as the rest of SQL commands.”
Sorry for being too meticulous.
>
> diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
> index 692fa82..9c5c9b3 100644
> --- a/test/sql/sql-debug.result
> +++ b/test/sql/sql-debug.result
Why did you move test to this file?
PRAGMA itself is not debug-only command.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH v3 6/6] sql: set column types for EXPLAIN and PRAGMA
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
@ 2019-01-16 15:35 ` n.pettik
0 siblings, 0 replies; 12+ messages in thread
From: n.pettik @ 2019-01-16 15:35 UTC (permalink / raw)
To: tarantool-patches; +Cc: Imeev Mergen
> @@ -471,17 +464,15 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
> goto pragma_out;
> }
> /* Register the result column names for pragmas that return results */
> - if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0
> - && ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0)
> - ) {
> - setPragmaResultColumnNames(v, pPragma);
> - }
> + if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0 &&
> + ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0))
> + vdbe_set_pragma_result_columns(v, pPragma);
zRight != NULL
The rest seems to be OK.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-01-16 15:35 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-26 18:17 [tarantool-patches] [PATCH v3 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2018-12-26 18:17 ` [tarantool-patches] [PATCH v3 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
2019-01-16 15:34 ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 2/6] sql: fix "PRAGMA parser_trace" result imeevma
2019-01-16 15:35 ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 3/6] sql: Show currently set sql_default_engine imeevma
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
2019-01-16 15:35 ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 5/6] sql: 'PRAGMA' result in the appropriate format imeevma
2019-01-16 15:35 ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2019-01-16 15:35 ` [tarantool-patches] " n.pettik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox