[tarantool-patches] Re: [PATCH v5 2/6] sql: fix "PRAGMA parser_trace" result
Imeev Mergen
imeevma at tarantool.org
Thu Jan 31 17:56:43 MSK 2019
Hi Thank you for review! Answer, fixes and patch below.
On 1/30/19 4:57 PM, Vladislav Shpilevoy wrote:
>
>
> On 29/01/2019 17:29, imeevma at tarantool.org wrote:
>> Currently PRAGMA parser_trace returns an empty table. This seems
>> wrong, since other similar pragmas return their status. Fixed in
>> the current patch.
>> ---
>> src/box/sql/pragma.c | 59
>> +++++++++++++++++----------------------------
>> src/box/sql/pragma.h | 7 +++---
>> src/box/sql/sqliteInt.h | 2 ++
>> test/sql/sql-debug.result | 29 ++++++++++++++++++++++
>> test/sql/sql-debug.test.lua | 14 +++++++++++
>> test/sql/suite.ini | 2 +-
>> 6 files changed, 71 insertions(+), 42 deletions(-)
>> create mode 100644 test/sql/sql-debug.result
>> create mode 100644 test/sql/sql-debug.test.lua
>> > diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
>> new file mode 100644
>> index 0000000..0c9ac97
>> --- /dev/null
>> +++ b/test/sql/sql-debug.result
>> @@ -0,0 +1,29 @@
>> +remote = require('net.box')
>> +---
>> +...
>> +test_run = require('test_run').new()
>> +---
>> +...
>> +engine = test_run:get_cfg('engine')
>
> Why do you need an engine here? This test
> never touches the data dictionary.
Fixed.
Diff:
commit 69976f705d2509c48c6a467f51b1745cda2a50e7
Author: Mergen Imeev <imeevma at gmail.com>
Date: Thu Jan 31 16:30:18 2019 +0300
Temporary: Review fix
diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
index 0c9ac97..9388578 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -4,12 +4,6 @@ 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
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
index f946306..721ef19 100644
--- a/test/sql/sql-debug.test.lua
+++ b/test/sql/sql-debug.test.lua
@@ -1,7 +1,5 @@
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
Patch:
commit dbd8a2f9024a4e6bdc4a6e2573e18251e61a141c
Author: Mergen Imeev <imeevma at 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..476771d 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -466,33 +466,30 @@ 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 == NULL) {
+ setPragmaResultColumnNames(v, pPragma);
+ returnSingleInt(v, (user_session->sql_flags &
+ pPragma->iArg) != 0);
+ } else {
+ /* Mask of bits to set or clear. */
+ int mask = pPragma->iArg;
+ bool is_pragma_set = sqlite3GetBoolean(zRight, 0);
+
+ if (is_pragma_set)
+ user_session->sql_flags |= mask;
+ else
+ user_session->sql_flags &= ~mask;
+#if defined(SQLITE_DEBUG)
+ if (mask == PARSER_TRACE_FLAG) {
+ if (is_pragma_set)
+ sqlite3ParserTrace(stdout, "parser: ");
+ else
+ sqlite3ParserTrace(0, 0);
}
- break;
+#endif
}
+ break;
+ }
case PragTyp_TABLE_INFO:
sql_pragma_table_info(pParse, zRight);
@@ -569,18 +566,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 fd76b49..4837923 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 82738b0..9a3ae8f 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1547,6 +1547,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..9388578
--- /dev/null
+++ b/test/sql/sql-debug.result
@@ -0,0 +1,23 @@
+remote = require('net.box')
+---
+...
+test_run = require('test_run').new()
+---
+...
+--
+-- 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')
+---
+...
+box.sql.execute('PRAGMA parser_trace = 1')
+---
+...
+box.sql.execute('PRAGMA parser_trace')
+---
+- - [1]
+...
+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..721ef19
--- /dev/null
+++ b/test/sql/sql-debug.test.lua
@@ -0,0 +1,12 @@
+remote = require('net.box')
+test_run = require('test_run').new()
+
+--
+-- 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')
+box.sql.execute('PRAGMA parser_trace = 1')
+box.sql.execute('PRAGMA parser_trace')
+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
More information about the Tarantool-patches
mailing list