* [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 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