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 CE9DA25DA0 for ; Thu, 31 Jan 2019 09:56:49 -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 P-B-T0lxdqs4 for ; Thu, 31 Jan 2019 09:56:49 -0500 (EST) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 5F86F1FE6D for ; Thu, 31 Jan 2019 09:56:49 -0500 (EST) From: Imeev Mergen Subject: [tarantool-patches] Re: [PATCH v5 3/6] sql: Show currently set sql_default_engine References: <3ec5bc03-c10c-243f-fa51-ec471676a347@tarantool.org> Message-ID: <0d1106e6-3f71-2650-2d06-2b8e5b881b5b@tarantool.org> Date: Thu, 31 Jan 2019 17:56:47 +0300 MIME-Version: 1.0 In-Reply-To: <3ec5bc03-c10c-243f-fa51-ec471676a347@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: >> After this patch, "PRAGMA sql_default_engine" called without >> arguments will return currently set sql_default_engine. >> --- >>   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/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 >> +-- >> +-- 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' >> +    -- >> +    0, {'vinyl'} >> +    -- > > If a test does not fail, you should not use > test:do_catchsql_test. We have do_test for this. Fixed. > > Also, this file is named gh-2367-***, but you added > here a test for gh-3832. This is why it made no > sense to create a new file on each issue. Please, > rename it to pragma.test.lua. Fixed. Diff: commit bfae473ec8f64d21006525ffd02eeb05d7756448 Author: Mergen Imeev Date:   Thu Jan 31 16:34:25 2019 +0300     Temporary: Review fix diff --git a/test/sql-tap/gh-2367-pragma.test.lua b/test/sql-tap/gh-2367-pragma.test.lua deleted file mode 100755 index 90ecd56..0000000 --- a/test/sql-tap/gh-2367-pragma.test.lua +++ /dev/null @@ -1,84 +0,0 @@ -#!/usr/bin/env tarantool -test = require("sqltester") - -test:plan(8) - -test:do_catchsql_test( -    "pragma-1.3", -    [[ -        PRAGMA kek = 'ON'; -    ]], { -        1, "no such pragma: KEK" -    }) - ---- ---- gh-2199: SQL default engine pragma ---- -test:do_catchsql_test( -    "pragma-2.1", -    [[ -        pragma sql_default_engine='creepy'; -    ]], { -    1, "Space engine 'creepy' does not exist" -}) - -test:do_catchsql_test( -    "pragma-2.2", -    [[ -        pragma sql_default_engine='vinyl'; -    ]], { -    0 -}) - -test:do_catchsql_test( -    "pragma-2.3", -    [[ -        pragma sql_default_engine='memtx'; -    ]], { -    0 -}) - -test:do_catchsql_test( -    "pragma-2.4", -    [[ -        pragma sql_default_engine 'memtx'; -    ]], { -    1, 'near \"\'memtx\'\": syntax error' -}) - -test:do_catchsql_test( -    "pragma-2.5", -    [[ -        pragma sql_default_engine 1; -    ]], { -    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-3.1", -    [[ -        pragma sql_default_engine='vinyl'; -        pragma sql_default_engine; -    ]], { -    -- -    0, {'vinyl'} -    -- -}) - -test:do_catchsql_test( -    "pragma-3.2", -    [[ -        pragma sql_default_engine='memtx'; -        pragma sql_default_engine; -    ]], { -    -- -    0, {'memtx'} -    -- -}) - -test:finish_test() diff --git a/test/sql-tap/pragma.test.lua b/test/sql-tap/pragma.test.lua new file mode 100755 index 0000000..a847547 --- /dev/null +++ b/test/sql-tap/pragma.test.lua @@ -0,0 +1,84 @@ +#!/usr/bin/env tarantool +test = require("sqltester") + +test:plan(8) + +test:do_catchsql_test( +    "pragma-1.3", +    [[ +        PRAGMA kek = 'ON'; +    ]], { +        1, "no such pragma: KEK" +    }) + +--- +--- gh-2199: SQL default engine pragma +--- +test:do_catchsql_test( +    "pragma-2.1", +    [[ +        pragma sql_default_engine='creepy'; +    ]], { +    1, "Space engine 'creepy' does not exist" +}) + +test:do_catchsql_test( +    "pragma-2.2", +    [[ +        pragma sql_default_engine='vinyl'; +    ]], { +    0 +}) + +test:do_catchsql_test( +    "pragma-2.3", +    [[ +        pragma sql_default_engine='memtx'; +    ]], { +    0 +}) + +test:do_catchsql_test( +    "pragma-2.4", +    [[ +        pragma sql_default_engine 'memtx'; +    ]], { +    1, 'near \"\'memtx\'\": syntax error' +}) + +test:do_catchsql_test( +    "pragma-2.5", +    [[ +        pragma sql_default_engine 1; +    ]], { +    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_execsql_test( +    "pragma-3.1", +    [[ +        pragma sql_default_engine='vinyl'; +        pragma sql_default_engine; +    ]], { +    -- +    'vinyl' +    -- +}) + +test:do_execsql_test( +    "pragma-3.2", +    [[ +        pragma sql_default_engine='memtx'; +        pragma sql_default_engine; +    ]], { +    -- +    'memtx' +    -- +}) + +test:finish_test() Patch: commit 11e1d179228684a7ea776a2626b44ca3936d3d44 Author: Mergen Imeev Date:   Wed Dec 12 22:47:48 2018 +0300     sql: Show currently set sql_default_engine     After this patch, "PRAGMA sql_default_engine" called without     arguments will return currently set sql_default_engine. diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 476771d..5e276b4 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -582,12 +582,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 deleted file mode 100755 index c0792c9..0000000 --- a/test/sql-tap/gh-2367-pragma.test.lua +++ /dev/null @@ -1,65 +0,0 @@ -#!/usr/bin/env tarantool -test = require("sqltester") - -test:plan(7) - -test:do_catchsql_test( -    "pragma-1.3", -    [[ -        PRAGMA kek = 'ON'; -    ]], { -        1, "no such pragma: KEK" -    }) - ---- ---- gh-2199: SQL default engine pragma ---- -test:do_catchsql_test( -    "pragma-2.1", -    [[ -        pragma sql_default_engine='creepy'; -    ]], { -    1, "Space engine 'creepy' does not exist" -}) - -test:do_catchsql_test( -    "pragma-2.2", -    [[ -        pragma sql_default_engine='vinyl'; -    ]], { -    0 -}) - -test:do_catchsql_test( -    "pragma-2.3", -    [[ -        pragma sql_default_engine='memtx'; -    ]], { -    0 -}) - -test:do_catchsql_test( -    "pragma-2.4", -    [[ -        pragma sql_default_engine; -    ]], { -    1, 'Illegal parameters, \'sql_default_engine\' was not specified' -}) - -test:do_catchsql_test( -    "pragma-2.5", -    [[ -        pragma sql_default_engine 'memtx'; -    ]], { -    1, 'near \"\'memtx\'\": syntax error' -}) - -test:do_catchsql_test( -    "pragma-2.5", -    [[ -        pragma sql_default_engine 1; -    ]], { -    1, 'near \"1\": syntax error' -}) - -test:finish_test() diff --git a/test/sql-tap/pragma.test.lua b/test/sql-tap/pragma.test.lua new file mode 100755 index 0000000..a847547 --- /dev/null +++ b/test/sql-tap/pragma.test.lua @@ -0,0 +1,84 @@ +#!/usr/bin/env tarantool +test = require("sqltester") + +test:plan(8) + +test:do_catchsql_test( +    "pragma-1.3", +    [[ +        PRAGMA kek = 'ON'; +    ]], { +        1, "no such pragma: KEK" +    }) + +--- +--- gh-2199: SQL default engine pragma +--- +test:do_catchsql_test( +    "pragma-2.1", +    [[ +        pragma sql_default_engine='creepy'; +    ]], { +    1, "Space engine 'creepy' does not exist" +}) + +test:do_catchsql_test( +    "pragma-2.2", +    [[ +        pragma sql_default_engine='vinyl'; +    ]], { +    0 +}) + +test:do_catchsql_test( +    "pragma-2.3", +    [[ +        pragma sql_default_engine='memtx'; +    ]], { +    0 +}) + +test:do_catchsql_test( +    "pragma-2.4", +    [[ +        pragma sql_default_engine 'memtx'; +    ]], { +    1, 'near \"\'memtx\'\": syntax error' +}) + +test:do_catchsql_test( +    "pragma-2.5", +    [[ +        pragma sql_default_engine 1; +    ]], { +    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_execsql_test( +    "pragma-3.1", +    [[ +        pragma sql_default_engine='vinyl'; +        pragma sql_default_engine; +    ]], { +    -- +    'vinyl' +    -- +}) + +test:do_execsql_test( +    "pragma-3.2", +    [[ +        pragma sql_default_engine='memtx'; +        pragma sql_default_engine; +    ]], { +    -- +    'memtx' +    -- +}) + +test:finish_test()