Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH v2 0/6] sql: set column types for EXPLAIN and PRAGMA
@ 2018-12-15 11:51 imeevma
  2018-12-15 11:54 ` [tarantool-patches] [PATCH v2 1/6] sql: remove unnecessary macros from pragma.* imeevma
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: imeevma @ 2018-12-15 11:51 UTC (permalink / raw)
  To: v.shpilevoy, tarantool-patches

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 second version:
  - Fixes for problems in PRAGMA commands that have something to
    do with their result.
  - Refactoring.

v1: https://www.freelists.org/post/tarantool-patches/PATCH-v1-11-sql-set-column-types-for-EXPLAIN-and-PRAGMA

Mergen Imeev (6):
  sql: remove unnecessary macros from pragma.*
  sql: fix "PRAGMA parser_trace" result
  sql: Show currently set sql_default_engine
  sql: fix "PRAGMA case_sensitive_like" result
  sql: 'PRAGMA' result in Tarantool format
  sql: set column types for EXPLAIN and PRAGMA

 src/box/execute.c                    |   5 +-
 src/box/sql/pragma.c                 | 179 ++++++++++++++-----------
 src/box/sql/pragma.h                 | 252 ++++++++++++++++++++++-------------
 src/box/sql/prepare.c                |  52 ++++++--
 src/box/sql/sqliteInt.h              |   4 +
 test/sql-tap/gh-2367-pragma.test.lua |  35 +++--
 test/sql/errinj.result               |  19 +++
 test/sql/errinj.test.lua             |  16 +++
 test/sql/iproto.result               |  69 ++++++++++
 test/sql/iproto.test.lua             |  18 ++-
 test/sql/misc.result                 |  29 ++++
 test/sql/misc.test.lua               |  18 +++
 12 files changed, 501 insertions(+), 195 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] [PATCH v2 1/6] sql: remove unnecessary macros from pragma.*
  2018-12-15 11:51 [tarantool-patches] [PATCH v2 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
@ 2018-12-15 11:54 ` imeevma
  2018-12-20 20:41   ` [tarantool-patches] " Vladislav Shpilevoy
  2018-12-24 14:01   ` n.pettik
  2018-12-15 11:56 ` [tarantool-patches] [PATCH v2 2/6] sql: fix "PRAGMA parser_trace" result imeevma
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: imeevma @ 2018-12-15 11:54 UTC (permalink / raw)
  To: v.shpilevoy, tarantool-patches

Some of the macros used in pragma.c and pragma.h are obsolete
because the values they checked were deleted.

Part of #3832
---
 src/box/sql/pragma.c |  4 ----
 src/box/sql/pragma.h | 30 +++---------------------------
 2 files changed, 3 insertions(+), 31 deletions(-)

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 5c35017..c052015 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -467,7 +467,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);
@@ -496,9 +495,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;
@@ -535,7 +532,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] 14+ messages in thread

* [tarantool-patches] [PATCH v2 2/6] sql: fix "PRAGMA parser_trace" result
  2018-12-15 11:51 [tarantool-patches] [PATCH v2 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
  2018-12-15 11:54 ` [tarantool-patches] [PATCH v2 1/6] sql: remove unnecessary macros from pragma.* imeevma
@ 2018-12-15 11:56 ` imeevma
  2018-12-24 14:01   ` [tarantool-patches] " n.pettik
  2018-12-15 12:01 ` [tarantool-patches] [PATCH v2 3/6] sql: Show currently set sql_default_engine imeevma
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: imeevma @ 2018-12-15 11:56 UTC (permalink / raw)
  To: v.shpilevoy, tarantool-patches

Currently PRAGMA parser_trace returns an empty table. This seems
wrong, since other similar pragmas return their status. Fixed in
the current patch.

Part of #3832
---
 src/box/sql/pragma.c     | 20 +++++++++++++-------
 src/box/sql/pragma.h     |  4 ++--
 src/box/sql/sqliteInt.h  |  2 ++
 test/sql/errinj.result   | 19 +++++++++++++++++++
 test/sql/errinj.test.lua | 16 ++++++++++++++++
 5 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index c052015..1fe5b3e 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -568,15 +568,21 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 	}
 #ifndef NDEBUG
 	case PragTyp_PARSER_TRACE:{
-			if (zRight) {
-				if (sqlite3GetBoolean(zRight, 0)) {
-					sqlite3ParserTrace(stdout, "parser: ");
-				} else {
-					sqlite3ParserTrace(0, 0);
-				}
+		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;
 		}
+		break;
+	}
 #endif
 
 		/*
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 84ab478..59e0e1e 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -138,9 +138,9 @@ static const PragmaName aPragmaName[] = {
 #if defined(SQLITE_DEBUG)
 	{ /* zName:     */ "parser_trace",
 	 /* ePragTyp:  */ PragTyp_PARSER_TRACE,
-	 /* ePragFlg:  */ 0,
+	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
 	 /* ColNames:  */ 0, 0,
-	 /* iArg:      */ 0},
+	 /* iArg:      */ SQLITE_ParserTrace},
 #endif
 	{ /* zName:     */ "query_only",
 	 /* ePragTyp:  */ PragTyp_FLAG,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 1ec52b8..3d4dead 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 SQLITE_ParserTrace    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 cb993f8..6ba7e72 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -280,3 +280,22 @@ 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 fa7f9f2..a938eda 100644
--- a/test/sql/errinj.test.lua
+++ b/test/sql/errinj.test.lua
@@ -97,3 +97,19 @@ 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])
-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] [PATCH v2 3/6] sql: Show currently set sql_default_engine
  2018-12-15 11:51 [tarantool-patches] [PATCH v2 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
  2018-12-15 11:54 ` [tarantool-patches] [PATCH v2 1/6] sql: remove unnecessary macros from pragma.* imeevma
  2018-12-15 11:56 ` [tarantool-patches] [PATCH v2 2/6] sql: fix "PRAGMA parser_trace" result imeevma
@ 2018-12-15 12:01 ` imeevma
  2018-12-24 14:01   ` [tarantool-patches] " n.pettik
  2018-12-15 12:03 ` [tarantool-patches] [PATCH v2 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: imeevma @ 2018-12-15 12:01 UTC (permalink / raw)
  To: v.shpilevoy, tarantool-patches

After this patch, "PRAGMA sql_default_engine" called without
arguments will return currently set sql_default_engine.

Part of #3832
---
 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 1fe5b3e..f34e7b8 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -600,12 +600,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] 14+ messages in thread

* [tarantool-patches] [PATCH v2 4/6] sql: fix "PRAGMA case_sensitive_like" result
  2018-12-15 11:51 [tarantool-patches] [PATCH v2 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
                   ` (2 preceding siblings ...)
  2018-12-15 12:01 ` [tarantool-patches] [PATCH v2 3/6] sql: Show currently set sql_default_engine imeevma
@ 2018-12-15 12:03 ` imeevma
  2018-12-24 14:01   ` [tarantool-patches] " n.pettik
  2018-12-15 12:05 ` [tarantool-patches] [PATCH v2 5/6] sql: 'PRAGMA' result in Tarantool format imeevma
  2018-12-15 12:08 ` [tarantool-patches] [PATCH v2 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
  5 siblings, 1 reply; 14+ messages in thread
From: imeevma @ 2018-12-15 12:03 UTC (permalink / raw)
  To: v.shpilevoy, tarantool-patches

Currently PRAGMA case_sensitive_like returns nothing. This seems
wrong, since other similar pragmas return their status. Fixed in
the current patch.

Part of #3832
---
 src/box/sql/pragma.c    | 19 +++++++++++++------
 src/box/sql/pragma.h    |  4 ++--
 src/box/sql/sqliteInt.h |  2 ++
 test/sql/misc.result    | 17 +++++++++++++++++
 test/sql/misc.test.lua  | 13 +++++++++++++
 5 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index f34e7b8..9390f6f 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -591,13 +591,20 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		 * depending on the RHS.
 		 */
 	case PragTyp_CASE_SENSITIVE_LIKE:{
-			if (zRight) {
-				int is_like_ci =
-					!(sqlite3GetBoolean(zRight, 0));
-				sqlite3RegisterLikeFunctions(db, is_like_ci);
-			}
-			break;
+		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) {
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 59e0e1e..11a2e05 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -94,9 +94,9 @@ static const PragmaName aPragmaName[] = {
 	 /* iArg:      */ 0},
 	{ /* zName:     */ "case_sensitive_like",
 	 /* ePragTyp:  */ PragTyp_CASE_SENSITIVE_LIKE,
-	 /* ePragFlg:  */ PragFlg_NoColumns,
+	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
 	 /* ColNames:  */ 0, 0,
-	 /* iArg:      */ 0},
+	 /* iArg:      */ SQLITE_LikeIsCaseSens},
 	{ /* 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 3d4dead..17fb281 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1565,6 +1565,8 @@ struct sqlite3 {
 #define SQLITE_VdbeTrace      0x00000001	/* True to trace VDBE execution */
 /* Debug print info about SQL query as it parsed */
 #define SQLITE_ParserTrace    0x00000002
+/* True if LIKE is case sensitive. */
+#define SQLITE_LikeIsCaseSens 0x00000008
 #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/misc.result b/test/sql/misc.result
index ef104c1..f1031f7 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -40,3 +40,20 @@ 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 TRUE.
+result[1][1] == 0 or result[1][1] == 1
+---
+- true
+...
+-- Check that "PRAGMA case_sensitive_like" returns nothing if
+-- called with parameter.
+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..5952035 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 TRUE.
+result[1][1] == 0 or result[1][1] == 1
+
+-- Check that "PRAGMA case_sensitive_like" returns nothing if
+-- called with parameter.
+box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] [PATCH v2 5/6] sql: 'PRAGMA' result in Tarantool format
  2018-12-15 11:51 [tarantool-patches] [PATCH v2 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
                   ` (3 preceding siblings ...)
  2018-12-15 12:03 ` [tarantool-patches] [PATCH v2 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
@ 2018-12-15 12:05 ` imeevma
  2018-12-24 14:02   ` [tarantool-patches] " n.pettik
  2018-12-15 12:08 ` [tarantool-patches] [PATCH v2 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
  5 siblings, 1 reply; 14+ messages in thread
From: imeevma @ 2018-12-15 12:05 UTC (permalink / raw)
  To: v.shpilevoy, tarantool-patches

Currently, box.sql.execute('PRAGMA') returns nothing but it prints
some information on stdout. This isn't right. This patch makes
the command to return result in Tarantool format.

Part of #3832
---
 src/box/sql/pragma.c   | 83 +++++++++++++++++++++++++++++---------------------
 test/sql/misc.result   | 12 ++++++++
 test/sql/misc.test.lua |  5 +++
 3 files changed, 65 insertions(+), 35 deletions(-)

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 9390f6f..fc87976 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -168,48 +168,61 @@ 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)
+sql_pragma_active_pragmas(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_PARSER_TRACE:
+			case PragTyp_CASE_SENSITIVE_LIKE:
+			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:
+				sqlite3VdbeAddOp2(v, OP_Null, 0, 2);
 		}
-	}
-
-	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);
 	}
 }
 
@@ -440,7 +453,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 
 	zLeft = sqlite3NameFromToken(db, pId);
 	if (!zLeft) {
-		printActivePragmas(user_session);
+		sql_pragma_active_pragmas(pParse);
 		return;
 	}
 
diff --git a/test/sql/misc.result b/test/sql/misc.result
index f1031f7..758734c 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -57,3 +57,15 @@ result[1][1] == 0 or result[1][1] == 1
 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 5952035..1c0dd07 100644
--- a/test/sql/misc.test.lua
+++ b/test/sql/misc.test.lua
@@ -24,3 +24,8 @@ result[1][1] == 0 or result[1][1] == 1
 -- Check that "PRAGMA case_sensitive_like" returns nothing if
 -- called with parameter.
 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]
-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] [PATCH v2 6/6] sql: set column types for EXPLAIN and PRAGMA
  2018-12-15 11:51 [tarantool-patches] [PATCH v2 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
                   ` (4 preceding siblings ...)
  2018-12-15 12:05 ` [tarantool-patches] [PATCH v2 5/6] sql: 'PRAGMA' result in Tarantool format imeevma
@ 2018-12-15 12:08 ` imeevma
  2018-12-24 14:02   ` [tarantool-patches] " n.pettik
  5 siblings, 1 reply; 14+ messages in thread
From: imeevma @ 2018-12-15 12:08 UTC (permalink / raw)
  To: v.shpilevoy, tarantool-patches

Hi! Thank you for review! My answers and new patch below.

> 1. You rewrote this function almost completely. Usually in
> such a case we reformat the function into Tarantool code style.
>
Squashed review fixes.

> 2. I see that sql_pragma_table_stats returns rows of different types
> of the same columns. First row has 4 columns with types "ssii", other
> rows have 3 columns "sii". I think, all rows should have the same
> number of columns of the same types. Also, the first row actually
> duplicates the second - they both describe the primary index.
>
> Please, consult server team chat what to do with it. I think, that we
> should always return 3 columns "sii": index name, avg tuple size, index size.
> So we just should delete the current first row.
>
After some discussion in server chat it was decided to left as it
is, at least for now.

> 3. I do not see where case_sensitive_like returns anything.
> Looks like it is has 'void' return type. So it should not
> have any out names/types. The same about all other pragmas
> below. Or am I wrong?
>
Fixed in one of the patches in the patch-set.

commit f7265e12b4ad98ea4d92487bc258f6c62db2d9b3
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 3a6cadf..d3adacb 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -550,11 +550,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 fc87976..eb8a69e 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -112,25 +112,20 @@ 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) {
+   /* 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);
- } else {
-   int i, j;
-   for (i = 0, j = pPragma->iPragCName; i < n; i++, j++) {
-     sqlite3VdbeSetColName(v, i, COLNAME_NAME, pragCName[j],
-               SQLITE_STATIC);
-   }
  }
 }
 
@@ -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))
+   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) !=
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 11a2e05..28e983e 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -27,50 +27,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 */
@@ -79,7 +171,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;
 /**
@@ -90,112 +182,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_CASE_SENSITIVE_LIKE,
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-  /* ColNames:  */ 0, 0,
+  /* ColNames:  */ 64, 1,
   /* iArg:      */ SQLITE_LikeIsCaseSens},
  { /* 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_PARSER_TRACE,
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-  /* ColNames:  */ 0, 0,
+  /* ColNames:  */ 72, 1,
   /* iArg:      */ SQLITE_ParserTrace},
 #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,
@@ -207,28 +299,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)
@@ -236,7 +328,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..e128001 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..9f9a382 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] 14+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/6] sql: remove unnecessary macros from pragma.*
  2018-12-15 11:54 ` [tarantool-patches] [PATCH v2 1/6] sql: remove unnecessary macros from pragma.* imeevma
@ 2018-12-20 20:41   ` Vladislav Shpilevoy
  2018-12-24 14:01   ` n.pettik
  1 sibling, 0 replies; 14+ messages in thread
From: Vladislav Shpilevoy @ 2018-12-20 20:41 UTC (permalink / raw)
  To: imeevma, tarantool-patches, Nikita Pettik

Nikita, please, do first review.

On 15/12/2018 14:54, imeevma@tarantool.org wrote:
> Some of the macros used in pragma.c and pragma.h are obsolete
> because the values they checked were deleted.
> 
> Part of #3832
> ---
>   src/box/sql/pragma.c |  4 ----
>   src/box/sql/pragma.h | 30 +++---------------------------
>   2 files changed, 3 insertions(+), 31 deletions(-)
> 
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index 5c35017..c052015 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -467,7 +467,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);
> @@ -496,9 +495,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;
> @@ -535,7 +532,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,
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/6] sql: remove unnecessary macros from pragma.*
  2018-12-15 11:54 ` [tarantool-patches] [PATCH v2 1/6] sql: remove unnecessary macros from pragma.* imeevma
  2018-12-20 20:41   ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-12-24 14:01   ` n.pettik
  1 sibling, 0 replies; 14+ messages in thread
From: n.pettik @ 2018-12-24 14:01 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Imeev Mergen

Nit: at the end of commit subject redundant ‘*’ and dot.

> On 15 Dec 2018, at 13:54, imeevma@tarantool.org wrote:
> 
> Some of the macros used in pragma.c and pragma.h are obsolete
> because the values they checked were deleted.

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.

> Part of #3832

In fact, this commit has nothing in common with mentioned issue.

> -#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)

Why didn’t you remove ENABLE_SELECTTRACE as well?

> @@ -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)

The same question.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [PATCH v2 2/6] sql: fix "PRAGMA parser_trace" result
  2018-12-15 11:56 ` [tarantool-patches] [PATCH v2 2/6] sql: fix "PRAGMA parser_trace" result imeevma
@ 2018-12-24 14:01   ` n.pettik
  0 siblings, 0 replies; 14+ messages in thread
From: n.pettik @ 2018-12-24 14:01 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Imeev Mergen


> Part of #3832

Why is this patch part of #3832?
Couldn’t you implement #3832 without this patch?

> ---
> src/box/sql/pragma.c     | 20 +++++++++++++-------
> src/box/sql/pragma.h     |  4 ++--
> src/box/sql/sqliteInt.h  |  2 ++
> test/sql/errinj.result   | 19 +++++++++++++++++++
> test/sql/errinj.test.lua | 16 ++++++++++++++++
> 5 files changed, 52 insertions(+), 9 deletions(-)
> 
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index c052015..1fe5b3e 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -568,15 +568,21 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
> 	}
> #ifndef NDEBUG
> 	case PragTyp_PARSER_TRACE:{
> -			if (zRight) {
> -				if (sqlite3GetBoolean(zRight, 0)) {
> -					sqlite3ParserTrace(stdout, "parser: ");
> -				} else {
> -					sqlite3ParserTrace(0, 0);
> -				}
> +		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;
> 		}

Why can’t you simply set to this pragma type ‘flag’?
It works almost that way.

> +		break;
> +	}
> #endif
> 
> 		/*
> diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
> index 84ab478..59e0e1e 100644
> --- a/src/box/sql/pragma.h
> +++ b/src/box/sql/pragma.h
> @@ -138,9 +138,9 @@ static const PragmaName aPragmaName[] = {
> #if defined(SQLITE_DEBUG)
> 	{ /* zName:     */ "parser_trace",
> 	 /* ePragTyp:  */ PragTyp_PARSER_TRACE,
> -	 /* ePragFlg:  */ 0,
> +	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
> 	 /* ColNames:  */ 0, 0,
> -	 /* iArg:      */ 0},
> +	 /* iArg:      */ SQLITE_ParserTrace},
> #endif
> 	{ /* zName:     */ "query_only",
> 	 /* ePragTyp:  */ PragTyp_FLAG,
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index 1ec52b8..3d4dead 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 SQLITE_ParserTrace    0x00000002

Let's avoid using ’sqlite’ prefix in any added code.

> #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 cb993f8..6ba7e72 100644
> --- a/test/sql/errinj.result
> +++ b/test/sql/errinj.result
> @@ -280,3 +280,22 @@ 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 fa7f9f2..a938eda 100644
> --- a/test/sql/errinj.test.lua
> +++ b/test/sql/errinj.test.lua
> @@ -97,3 +97,19 @@ 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.

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [PATCH v2 3/6] sql: Show currently set sql_default_engine
  2018-12-15 12:01 ` [tarantool-patches] [PATCH v2 3/6] sql: Show currently set sql_default_engine imeevma
@ 2018-12-24 14:01   ` n.pettik
  0 siblings, 0 replies; 14+ messages in thread
From: n.pettik @ 2018-12-24 14:01 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Imeev Mergen

This is OK.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [PATCH v2 4/6] sql: fix "PRAGMA case_sensitive_like" result
  2018-12-15 12:03 ` [tarantool-patches] [PATCH v2 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
@ 2018-12-24 14:01   ` n.pettik
  0 siblings, 0 replies; 14+ messages in thread
From: n.pettik @ 2018-12-24 14:01 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Imeev Mergen


> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index f34e7b8..9390f6f 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -591,13 +591,20 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
> 		 * depending on the RHS.
> 		 */
> 	case PragTyp_CASE_SENSITIVE_LIKE:{
> -			if (zRight) {
> -				int is_like_ci =
> -					!(sqlite3GetBoolean(zRight, 0));
> -				sqlite3RegisterLikeFunctions(db, is_like_ci);
> -			}
> -			break;
> +		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);
> 		}

Nit: since pragma is called ‘case_sensitive_like’, I would revert variable
meaning:

+++ b/src/box/sql/pragma.c
@@ -602,12 +602,12 @@ sqlite3Pragma(Parse * pParse, Token * pId,        /* First part of [schema.]id field */
                        returnSingleInt(v,
                                        (user_session->sql_flags & mask) != 0);
                } else {
-                       int is_like_ci = !(sqlite3GetBoolean(zRight, 0));
-                       if (!is_like_ci)
+                       bool is_like_cs = sqlite3GetBoolean(zRight, 0);
+                       if (is_like_cs)
                                user_session->sql_flags |= mask;
                        else
                                user_session->sql_flags &= ~mask;
-                       sqlite3RegisterLikeFunctions(db, is_like_ci);
+                       sqlite3RegisterLikeFunctions(db, ! is_like_cs);

Also, the same question here: why you didn’t make it be of
type ‘flag’? The only difference in additional call of sqlite3RegisterLikeFunctions.


> +		break;
> +	}
> 
> 	case PragTyp_DEFAULT_ENGINE: {
> 		if (zRight == NULL) {
> diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
> index 59e0e1e..11a2e05 100644
> --- a/src/box/sql/pragma.h
> +++ b/src/box/sql/pragma.h
> @@ -94,9 +94,9 @@ static const PragmaName aPragmaName[] = {
> 	 /* iArg:      */ 0},
> 	{ /* zName:     */ "case_sensitive_like",
> 	 /* ePragTyp:  */ PragTyp_CASE_SENSITIVE_LIKE,
> -	 /* ePragFlg:  */ PragFlg_NoColumns,
> +	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
> 	 /* ColNames:  */ 0, 0,
> -	 /* iArg:      */ 0},
> +	 /* iArg:      */ SQLITE_LikeIsCaseSens},

Lets avoid using ’sqlite’ prefixes for added code.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [PATCH v2 5/6] sql: 'PRAGMA' result in Tarantool format
  2018-12-15 12:05 ` [tarantool-patches] [PATCH v2 5/6] sql: 'PRAGMA' result in Tarantool format imeevma
@ 2018-12-24 14:02   ` n.pettik
  0 siblings, 0 replies; 14+ messages in thread
From: n.pettik @ 2018-12-24 14:02 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Imeev Mergen

There is no ’Tarantool format’...

> On 15 Dec 2018, at 14:05, imeevma@tarantool.org wrote:
> 
> Currently, box.sql.execute('PRAGMA') returns nothing but it prints
> some information on stdout. This isn't right.

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.

Btw I don’t understand all these efforts to make pragma
better. As I already said we are going to remove it anyway.


> This patch makes
> the command to return result in Tarantool format.
> 
> Part of #3832
> ---
> src/box/sql/pragma.c   | 83 +++++++++++++++++++++++++++++---------------------
> test/sql/misc.result   | 12 ++++++++
> test/sql/misc.test.lua |  5 +++
> 3 files changed, 65 insertions(+), 35 deletions(-)
> 
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index 9390f6f..fc87976 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -168,48 +168,61 @@ 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)
> +sql_pragma_active_pragmas(struct Parse *parse)

Name seems to be a mess, lets rename it to ’sql_pragma_status’ or
‘vdbe_emit_pragma_status’.

> {
> -	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_PARSER_TRACE:
> +			case PragTyp_CASE_SENSITIVE_LIKE:
> +			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:
> +				sqlite3VdbeAddOp2(v, OP_Null, 0, 2);
> 		}
> -	}
> -
> -	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);
> 	}
> }
> 
> @@ -440,7 +453,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
> 
> 	zLeft = sqlite3NameFromToken(db, pId);
> 	if (!zLeft) {
> -		printActivePragmas(user_session);
> +		sql_pragma_active_pragmas(pParse);
> 		return;
> 	}
> 
> diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
> index 5952035..1c0dd07 100644
> --- a/test/sql/misc.test.lua
> +++ b/test/sql/misc.test.lua
> @@ -24,3 +24,8 @@ result[1][1] == 0 or result[1][1] == 1
> -- Check that "PRAGMA case_sensitive_like" returns nothing if
> -- called with parameter.
> 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]

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [PATCH v2 6/6] sql: set column types for EXPLAIN and PRAGMA
  2018-12-15 12:08 ` [tarantool-patches] [PATCH v2 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
@ 2018-12-24 14:02   ` n.pettik
  0 siblings, 0 replies; 14+ messages in thread
From: n.pettik @ 2018-12-24 14:02 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Imeev Mergen


> -/*
> - * 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) {
> +   /* 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 */

These two comments are too obvious, lets remove them.

> 
> @@ -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:

-       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))

> ---
> ...
> diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
> index 6640903..9f9a382 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”)

Nit: uppercase all SQL keywords pls. Otherwise it looks pretty ugly.

Patch itself is OK.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-12-24 14:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-15 11:51 [tarantool-patches] [PATCH v2 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2018-12-15 11:54 ` [tarantool-patches] [PATCH v2 1/6] sql: remove unnecessary macros from pragma.* imeevma
2018-12-20 20:41   ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-24 14:01   ` n.pettik
2018-12-15 11:56 ` [tarantool-patches] [PATCH v2 2/6] sql: fix "PRAGMA parser_trace" result imeevma
2018-12-24 14:01   ` [tarantool-patches] " n.pettik
2018-12-15 12:01 ` [tarantool-patches] [PATCH v2 3/6] sql: Show currently set sql_default_engine imeevma
2018-12-24 14:01   ` [tarantool-patches] " n.pettik
2018-12-15 12:03 ` [tarantool-patches] [PATCH v2 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
2018-12-24 14:01   ` [tarantool-patches] " n.pettik
2018-12-15 12:05 ` [tarantool-patches] [PATCH v2 5/6] sql: 'PRAGMA' result in Tarantool format imeevma
2018-12-24 14:02   ` [tarantool-patches] " n.pettik
2018-12-15 12:08 ` [tarantool-patches] [PATCH v2 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2018-12-24 14:02   ` [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