From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 7D72525C51 for ; Thu, 31 Jan 2019 09:56:45 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id u-fDkzI4ve2q for ; Thu, 31 Jan 2019 09:56:45 -0500 (EST) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 16BC31FE6D for ; Thu, 31 Jan 2019 09:56:45 -0500 (EST) From: Imeev Mergen Subject: [tarantool-patches] Re: [PATCH v5 2/6] sql: fix "PRAGMA parser_trace" result References: <3e0c44e3-21fc-8288-c877-da8fe5996758@tarantool.org> Message-ID: Date: Thu, 31 Jan 2019 17:56:43 +0300 MIME-Version: 1.0 In-Reply-To: <3e0c44e3-21fc-8288-c877-da8fe5996758@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Vladislav Shpilevoy , tarantool-patches@freelists.org Cc: korablev@tarantool.org Hi Thank you for review! Answer, fixes and patch below. On 1/30/19 4:57 PM, Vladislav Shpilevoy wrote: > > > On 29/01/2019 17:29, imeevma@tarantool.org wrote: >> Currently PRAGMA parser_trace returns an empty table. This seems >> wrong, since other similar pragmas return their status. Fixed in >> the current patch. >> --- >>   src/box/sql/pragma.c        | 59 >> +++++++++++++++++---------------------------- >>   src/box/sql/pragma.h        |  7 +++--- >>   src/box/sql/sqliteInt.h     |  2 ++ >>   test/sql/sql-debug.result   | 29 ++++++++++++++++++++++ >>   test/sql/sql-debug.test.lua | 14 +++++++++++ >>   test/sql/suite.ini          |  2 +- >>   6 files changed, 71 insertions(+), 42 deletions(-) >>   create mode 100644 test/sql/sql-debug.result >>   create mode 100644 test/sql/sql-debug.test.lua >> > diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result >> new file mode 100644 >> index 0000000..0c9ac97 >> --- /dev/null >> +++ b/test/sql/sql-debug.result >> @@ -0,0 +1,29 @@ >> +remote = require('net.box') >> +--- >> +... >> +test_run = require('test_run').new() >> +--- >> +... >> +engine = test_run:get_cfg('engine') > > Why do you need an engine here? This test > never touches the data dictionary. Fixed. Diff: commit 69976f705d2509c48c6a467f51b1745cda2a50e7 Author: Mergen Imeev Date:   Thu Jan 31 16:30:18 2019 +0300     Temporary: Review fix diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result index 0c9ac97..9388578 100644 --- a/test/sql/sql-debug.result +++ b/test/sql/sql-debug.result @@ -4,12 +4,6 @@ remote = require('net.box')  test_run = require('test_run').new()  ---  ... -engine = test_run:get_cfg('engine') ---- -... -box.sql.execute('pragma sql_default_engine=\''..engine..'\'') ---- -...  --  -- gh-3832: Some statements do not return column type  -- Check that "PRAGMA parser_trace" returns 0 or 1 if called diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua index f946306..721ef19 100644 --- a/test/sql/sql-debug.test.lua +++ b/test/sql/sql-debug.test.lua @@ -1,7 +1,5 @@  remote = require('net.box')  test_run = require('test_run').new() -engine = test_run:get_cfg('engine') -box.sql.execute('pragma sql_default_engine=\''..engine..'\'')  --  -- gh-3832: Some statements do not return column type Patch: commit dbd8a2f9024a4e6bdc4a6e2573e18251e61a141c Author: Mergen Imeev Date:   Wed Dec 12 22:16:33 2018 +0300     sql: fix "PRAGMA parser_trace" result     Currently PRAGMA parser_trace returns an empty table. This seems     wrong, since other similar pragmas return their status. Fixed in     the current patch. diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 5729fe6..476771d 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -466,33 +466,30 @@ sqlite3Pragma(Parse * pParse, Token * pId,    /* First part of [schema.]id field */      switch (pPragma->ePragTyp) {      case PragTyp_FLAG:{ -            if (zRight == 0) { -                setPragmaResultColumnNames(v, pPragma); -                returnSingleInt(v, -                        (user_session-> -                         sql_flags & pPragma->iArg) != -                        0); -            } else { -                int mask = pPragma->iArg;    /* Mask of bits to set -                                 * or clear. -                                 */ - -                if (sqlite3GetBoolean(zRight, 0)) { -                    user_session->sql_flags |= mask; -                } else { -                    user_session->sql_flags &= ~mask; -                } - -                /* Many of the flag-pragmas modify the code -                 * generated by the SQL * compiler (eg. -                 * count_changes). So add an opcode to expire -                 * all * compiled SQL statements after -                 * modifying a pragma value. -                 */ -                sqlite3VdbeAddOp0(v, OP_Expire); +        if (zRight == NULL) { +            setPragmaResultColumnNames(v, pPragma); +            returnSingleInt(v, (user_session->sql_flags & +                        pPragma->iArg) != 0); +        } else { +            /* Mask of bits to set or clear. */ +            int mask = pPragma->iArg; +            bool is_pragma_set = sqlite3GetBoolean(zRight, 0); + +            if (is_pragma_set) +                user_session->sql_flags |= mask; +            else +                user_session->sql_flags &= ~mask; +#if defined(SQLITE_DEBUG) +            if (mask == PARSER_TRACE_FLAG) { +                if (is_pragma_set) +                    sqlite3ParserTrace(stdout, "parser: "); +                else +                    sqlite3ParserTrace(0, 0);              } -            break; +#endif          } +        break; +    }      case PragTyp_TABLE_INFO:          sql_pragma_table_info(pParse, zRight); @@ -569,18 +566,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */          }          break;      } -#ifndef NDEBUG -    case PragTyp_PARSER_TRACE:{ -            if (zRight) { -                if (sqlite3GetBoolean(zRight, 0)) { -                    sqlite3ParserTrace(stdout, "parser: "); -                } else { -                    sqlite3ParserTrace(0, 0); -                } -            } -            break; -        } -#endif          /*           * Reinstall the LIKE and functions. The variant diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h index fd76b49..4837923 100644 --- a/src/box/sql/pragma.h +++ b/src/box/sql/pragma.h @@ -14,7 +14,6 @@  #define PragTyp_INDEX_LIST                    11  #define PragTyp_STATS                         15  #define PragTyp_TABLE_INFO                    17 -#define PragTyp_PARSER_TRACE                  24  #define PragTyp_DEFAULT_ENGINE                25  #define PragTyp_COMPOUND_SELECT_LIMIT         26 @@ -137,10 +136,10 @@ static const PragmaName aPragmaName[] = {       /* iArg:      */ 0},  #if defined(SQLITE_DEBUG)      { /* zName:     */ "parser_trace", -     /* ePragTyp:  */ PragTyp_PARSER_TRACE, -     /* ePragFlg:  */ 0, +     /* ePragTyp:  */ PragTyp_FLAG, +     /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,       /* ColNames:  */ 0, 0, -     /* iArg:      */ 0}, +     /* iArg:      */ PARSER_TRACE_FLAG},  #endif      { /* zName:     */ "query_only",       /* ePragTyp:  */ PragTyp_FLAG, diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 82738b0..9a3ae8f 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1547,6 +1547,8 @@ struct sqlite3 {   * Possible values for the sqlite3.flags.   */  #define SQLITE_VdbeTrace      0x00000001    /* True to trace VDBE execution */ +/* Debug print info about SQL query as it parsed */ +#define PARSER_TRACE_FLAG     0x00000002  #define SQLITE_FullColNames   0x00000004    /* Show full column names on SELECT */  #define SQLITE_ShortColNames  0x00000040    /* Show short columns names */  #define SQLITE_CountRows      0x00000080    /* Count rows changed by INSERT, */ diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result new file mode 100644 index 0000000..9388578 --- /dev/null +++ b/test/sql/sql-debug.result @@ -0,0 +1,23 @@ +remote = require('net.box') +--- +... +test_run = require('test_run').new() +--- +... +-- +-- gh-3832: Some statements do not return column type +-- Check that "PRAGMA parser_trace" returns 0 or 1 if called +-- without parameter. +result = box.sql.execute('PRAGMA parser_trace') +--- +... +box.sql.execute('PRAGMA parser_trace = 1') +--- +... +box.sql.execute('PRAGMA parser_trace') +--- +- - [1] +... +box.sql.execute('PRAGMA parser_trace = '.. result[1][1]) +--- +... diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua new file mode 100644 index 0000000..721ef19 --- /dev/null +++ b/test/sql/sql-debug.test.lua @@ -0,0 +1,12 @@ +remote = require('net.box') +test_run = require('test_run').new() + +-- +-- gh-3832: Some statements do not return column type + +-- Check that "PRAGMA parser_trace" returns 0 or 1 if called +-- without parameter. +result = box.sql.execute('PRAGMA parser_trace') +box.sql.execute('PRAGMA parser_trace = 1') +box.sql.execute('PRAGMA parser_trace') +box.sql.execute('PRAGMA parser_trace = '.. result[1][1]) diff --git a/test/sql/suite.ini b/test/sql/suite.ini index 4504731..ce6ccb7 100644 --- a/test/sql/suite.ini +++ b/test/sql/suite.ini @@ -6,4 +6,4 @@ use_unix_sockets = True  config = engine.cfg  is_parallel = True  lua_libs = lua/sql_tokenizer.lua -release_disabled = errinj.test.lua view_delayed_wal.test.lua +release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua