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