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 4315826250 for ; Fri, 8 Feb 2019 05:19:52 -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 bV1irJ0mqEn4 for ; Fri, 8 Feb 2019 05:19:52 -0500 (EST) Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (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 BC08426205 for ; Fri, 8 Feb 2019 05:19:51 -0500 (EST) From: Stanislav Zudin Subject: [tarantool-patches] [PATCH v2] sql: remove useless pragmas Date: Fri, 8 Feb 2019 13:19:47 +0300 Message-Id: <20190208101947.8945-1-szudin@tarantool.org> 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: tarantool-patches@freelists.org, korablev@tarantool.org Cc: Stanislav Zudin The pragmas "query_only" and "read_uncommitted" didn't affect anything and were removed. Fixed an error in pragma index_list which caused a segmantation fault. pragmas index_list and index_info return an error when receive an invalid table or index names. pragma sql_default_engine accepts only strings. Thus pragma sql_default_engine('memtx') is a well-formed command, while pragma sql_default_engine(memtx) or pragma sql_default_engine("memtx") are considered as an ill-formed and raise an error. Closes #3733 --- Branch: https://github.com/tarantool/tarantool/tree/stanztt/gh-3733-obsolete-pragmas Issue: https://github.com/tarantool/tarantool/issues/3733 src/box/sql/pragma.c | 65 +++++++++- src/box/sql/pragma.h | 42 +++--- src/box/sql/sqliteInt.h | 3 - test/sql-tap/gh-3733-pragma.test.lua | 184 +++++++++++++++++++++++++++ 4 files changed, 257 insertions(+), 37 deletions(-) create mode 100755 test/sql-tap/gh-3733-pragma.test.lua diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index eef1ed931..158e6269b 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -342,15 +342,37 @@ sql_pragma_index_info(struct Parse *parse, MAYBE_UNUSED const PragmaName *pragma, const char *tbl_name, const char *idx_name) { - if (idx_name == NULL || tbl_name == NULL) + if (idx_name == NULL) { + diag_set(ClientError, ER_ILLEGAL_PARAMS, + "index name was not specified"); + parse->rc = SQL_TARANTOOL_ERROR; + parse->nErr++; + return; + } + if(tbl_name == NULL) { + diag_set(ClientError, ER_ILLEGAL_PARAMS, + "table name was not specified"); + parse->rc = SQL_TARANTOOL_ERROR; + parse->nErr++; return; + } + struct space *space = space_by_name(tbl_name); - if (space == NULL) + if (space == NULL) { + diag_set(ClientError, ER_NO_SUCH_SPACE, + tbl_name); + parse->rc = SQL_TARANTOOL_ERROR; + parse->nErr++; return; + } + uint32_t iid = box_index_id_by_name(space->def->id, idx_name, strlen(idx_name)); - if (iid == BOX_ID_NIL) + if (iid == BOX_ID_NIL) { + sqlite3ErrorMsg(parse, "no such index: %s", idx_name); return; + } + struct index *idx = space_index(space, iid); assert(idx != NULL); parse->nMem = 6; @@ -389,18 +411,39 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name) if (tbl_name == NULL) return; struct space *space = space_by_name(tbl_name); - if (space == NULL) + if (space == NULL) { + diag_set(ClientError, ER_NO_SUCH_SPACE, + tbl_name); + parse->rc = SQL_TARANTOOL_ERROR; + parse->nErr++; return; - parse->nMem = 5; + } + parse->nMem = 3; struct Vdbe *v = sqlite3GetVdbe(parse); for (uint32_t i = 0; i < space->index_count; ++i) { struct index *idx = space->index[i]; - sqlite3VdbeMultiLoad(v, 1, "isisi", i, idx->def->name, + sqlite3VdbeMultiLoad(v, 1, "isi", i, idx->def->name, idx->def->opts.is_unique); - sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 5); + sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 3); } } +/* + * @brief Check whether the specified token is a string or ID. + * @param pToken - token to be examined + * @return true - if the token value is enclosed into quotes (') + * @return false in other cases + * The empty value is considered to be a string. + */ +static bool token_is_string(const Token* pToken) +{ + if (!pToken || pToken->n == 0) + return true; + return (pToken->n >= 2) && + (pToken->z[0] == '\'') && + (pToken->z[pToken->n - 1] == '\''); +} + /* * Process a pragma statement. * @@ -601,6 +644,14 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ } case PragTyp_DEFAULT_ENGINE: { + if (!token_is_string(pValue)) + { + diag_set(ClientError, ER_ILLEGAL_PARAMS, + "string value is expected"); + pParse->rc = SQL_TARANTOOL_ERROR; + pParse->nErr++; + goto pragma_out; + } if (sql_default_engine_set(zRight) != 0) { pParse->rc = SQL_TARANTOOL_ERROR; pParse->nErr++; diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h index e60801608..168c70561 100644 --- a/src/box/sql/pragma.h +++ b/src/box/sql/pragma.h @@ -55,22 +55,20 @@ static const char *const pragCName[] = { /* 16 */ "seq", /* 17 */ "name", /* 18 */ "unique", - /* 19 */ "origin", - /* 20 */ "partial", /* Used by: collation_list */ - /* 21 */ "seq", - /* 22 */ "name", + /* 19 */ "seq", + /* 20 */ "name", /* Used by: foreign_key_list */ - /* 23 */ "id", - /* 24 */ "seq", - /* 25 */ "table", - /* 26 */ "from", - /* 27 */ "to", - /* 28 */ "on_update", - /* 29 */ "on_delete", - /* 30 */ "match", + /* 21 */ "id", + /* 22 */ "seq", + /* 23 */ "table", + /* 24 */ "from", + /* 25 */ "to", + /* 26 */ "on_update", + /* 27 */ "on_delete", + /* 28 */ "match", /* Used by: busy_timeout */ - /* 31 */ "timeout", + /* 29 */ "timeout", }; /* Definitions of all built-in pragmas */ @@ -90,7 +88,7 @@ static const PragmaName aPragmaName[] = { { /* zName: */ "busy_timeout", /* ePragTyp: */ PragTyp_BUSY_TIMEOUT, /* ePragFlg: */ PragFlg_Result0, - /* ColNames: */ 31, 1, + /* ColNames: */ 29, 1, /* iArg: */ 0}, { /* zName: */ "case_sensitive_like", /* ePragTyp: */ PragTyp_CASE_SENSITIVE_LIKE, @@ -101,7 +99,7 @@ static const PragmaName aPragmaName[] = { { /* zName: */ "collation_list", /* ePragTyp: */ PragTyp_COLLATION_LIST, /* ePragFlg: */ PragFlg_Result0, - /* ColNames: */ 21, 2, + /* ColNames: */ 19, 2, /* iArg: */ 0}, #endif #if !defined(SQLITE_OMIT_FLAG_PRAGMAS) @@ -122,7 +120,7 @@ static const PragmaName aPragmaName[] = { /* ePragTyp: */ PragTyp_FOREIGN_KEY_LIST, /* ePragFlg: */ PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt, - /* ColNames: */ 23, 8, + /* ColNames: */ 21, 8, /* iArg: */ 0}, #if !defined(SQLITE_OMIT_FLAG_PRAGMAS) { /* zName: */ "full_column_names", @@ -142,7 +140,7 @@ static const PragmaName aPragmaName[] = { /* ePragTyp: */ PragTyp_INDEX_LIST, /* ePragFlg: */ PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt, - /* ColNames: */ 16, 5, + /* ColNames: */ 16, 3, /* iArg: */ 0}, #endif #if defined(SQLITE_DEBUG) && !defined(SQLITE_OMIT_PARSER_TRACE) @@ -153,16 +151,6 @@ static const PragmaName aPragmaName[] = { /* iArg: */ 0}, #endif #if !defined(SQLITE_OMIT_FLAG_PRAGMAS) - { /* zName: */ "query_only", - /* ePragTyp: */ PragTyp_FLAG, - /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, - /* iArg: */ SQLITE_QueryOnly}, - { /* zName: */ "read_uncommitted", - /* ePragTyp: */ PragTyp_FLAG, - /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, - /* iArg: */ SQLITE_ReadUncommitted}, { /* zName: */ "recursive_triggers", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index ee24e0337..db7a20aee 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1575,16 +1575,13 @@ struct sqlite3 { #define SQLITE_WhereTrace 0x00008000 /* Debug info about optimizer's work */ #define SQLITE_VdbeListing 0x00000400 /* Debug listings of VDBE programs */ #define SQLITE_VdbeAddopTrace 0x00001000 /* Trace sqlite3VdbeAddOp() calls */ -#define SQLITE_ReadUncommitted 0x0004000 /* For shared-cache mode */ #define SQLITE_ReverseOrder 0x00020000 /* Reverse unordered SELECTs */ #define SQLITE_RecTriggers 0x00040000 /* Enable recursive triggers */ #define SQLITE_AutoIndex 0x00100000 /* Enable automatic indexes */ #define SQLITE_PreferBuiltin 0x00200000 /* Preference to built-in funcs */ #define SQLITE_EnableTrigger 0x01000000 /* True to enable triggers */ #define SQLITE_DeferFKs 0x02000000 /* Defer all FK constraints */ -#define SQLITE_QueryOnly 0x04000000 /* Disable database changes */ #define SQLITE_VdbeEQP 0x08000000 /* Debug EXPLAIN QUERY PLAN */ -#define SQLITE_NoCkptOnClose 0x80000000 /* No checkpoint on close()/DETACH */ /* * Bits of the sqlite3.dbOptFlags field that are used by the diff --git a/test/sql-tap/gh-3733-pragma.test.lua b/test/sql-tap/gh-3733-pragma.test.lua new file mode 100755 index 000000000..5b0dcd26a --- /dev/null +++ b/test/sql-tap/gh-3733-pragma.test.lua @@ -0,0 +1,184 @@ +#!/usr/bin/env tarantool +test = require("sqltester") + +test:plan(18) + +--- +--- Prerequisites +--- +test:do_execsql_test( + "pragma-0.1", + [[ + DROP TABLE IF EXISTS gh3733; + CREATE TABLE gh3733(id INT primary key, f float); + INSERT INTO gh3733 VALUES(1, 0.1), (2, 0.2), (3, 0.3); + CREATE INDEX IDX ON GH3733 (id); + ]], { + +}) + +--- +--- pragma query_only is not supported +--- +test:do_catchsql_test( + "pragma-1.1", + [[ + pragma query_only; + ]], { + 1, "no such pragma: QUERY_ONLY" +}) + +--- +--- pragma read_uncommitted is not supported +--- +test:do_catchsql_test( + "pragma-2.1", + [[ + pragma read_uncommitted; + ]], { + 1, "no such pragma: READ_UNCOMMITTED" +}) + +--- +--- pragma index_list returns three columns in a row +--- +test:do_execsql_test( + "pragma-3.1", + [[ + pragma index_list(gh3733) + ]], { + -- + 0, 'pk_unnamed_GH3733_1', 1, 1, 'IDX', 0 + -- +}) + +--- +--- pragma index_list returns an error for unknown table +--- +test:do_catchsql_test( + "pragma-4.1", + [[ + pragma index_list(fufel); + ]], { + 1, "Space 'FUFEL' does not exist" +}) + +--- +--- pragma index_info returns an error for unknown index +--- +test:do_execsql_test( + "pragma-5.1", + [[ + pragma index_info(gh3733.IDX) + ]], { + -- + 0, 0, 'ID', 0, 'BINARY', 'integer' + -- +}) + +test:do_catchsql_test( + "pragma-5.2", + [[ + pragma index_info(no_table); + ]], { + 1, "Illegal parameters, table name was not specified" +}) + +test:do_catchsql_test( + "pragma-5.3", + [[ + pragma index_info(wrong_table.IDX); + ]], { + 1, "Space 'WRONG_TABLE' does not exist" +}) + +test:do_catchsql_test( + "pragma-5.4", + [[ + pragma index_info(gh3733.wrong_index); + ]], { + 1, "no such index: WRONG_INDEX" +}) + +--- +--- pragma sql_default_engine requires value +--- +test:do_catchsql_test( + "pragma-6.1", + [[ + pragma sql_default_engine; + ]], { + 1, "Illegal parameters, 'sql_default_engine' was not specified" +}) + +--- +--- pragma sql_default_engine accepts string values and rejects IDs +--- +test:do_catchsql_test( + "pragma-7.1", + [[ + pragma sql_default_engine(the_engine); + ]], { + 1, "Illegal parameters, string value is expected" +}) +test:do_catchsql_test( + "pragma-7.2", + [[ + pragma sql_default_engine(THE_ENGINE); + ]], { + 1, "Illegal parameters, string value is expected" +}) +test:do_catchsql_test( + "pragma-7.3", + [[ + pragma sql_default_engine("THE_ENGINE"); + ]], { + 1, "Illegal parameters, string value is expected" +}) + +test:do_catchsql_test( + "pragma-7.4", + [[ + pragma sql_default_engine('THE_ENGINE'); + ]], { + 1, "Space engine 'THE_ENGINE' does not exist" +}) + +test:do_catchsql_test( + "pragma-7.5", + [[ + pragma sql_default_engine(memtx); + ]], { + 1, "Illegal parameters, string value is expected" +}) + +test:do_catchsql_test( + "pragma-7.6", + [[ + pragma sql_default_engine("memtx"); + ]], { + 1, "Illegal parameters, string value is expected" +}) + +test:do_execsql_test( + "pragma-7.7", + [[ + pragma sql_default_engine('memtx'); + ]], { + -- + + -- +}) + +--- +--- cleanup +--- +test:do_execsql_test( + "pragma-8.1", + [[ + DROP TABLE IF EXISTS gh3733; + ]], { + +}) + +test:finish_test() -- 2.17.1