From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: szudin@tarantool.org Subject: [tarantool-patches] Re: [PATCH v2] sql: remove useless pragmas Date: Fri, 8 Feb 2019 22:12:20 +0300 [thread overview] Message-ID: <A2AFEEC4-AA8A-49CA-86AE-59E0020F8BE3@tarantool.org> (raw) In-Reply-To: <20190208101947.8945-1-szudin@tarantool.org> > On 8 Feb 2019, at 13:19, Stanislav Zudin <szudin@tarantool.org> wrote: > > 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 Please, fix commit subject: now it is not only about removal of useless pragmas. > --- > 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; > + } I’m sorry, but it seems that I interpreted comment from P.Gulutzan in a wrong way. We shouldn’t throw any errors, according to his clarification. Please, check out his comment and make sure this time I am right. If really so, please discard changes to sql_pragma_index_info(). > + 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); Note that in this case you don’t need set sqlite error: box_index_id_by_name will set diag message. So, you should only set appropriate error code in parse struct. > 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++; Seems that this change should be discarded as well. > return; > +/* > + * @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] == '\''); > +} I fixed codestyle violations: diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 158e6269b..3938ae429 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -428,20 +428,20 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name) } } -/* +/** * @brief Check whether the specified token is a string or ID. - * @param pToken - token to be examined + * @param token - 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) +static bool +token_is_string(const struct Token *token) { - if (!pToken || pToken->n == 0) + if (token == NULL || token->n == 0) return true; - return (pToken->n >= 2) && - (pToken->z[0] == '\'') && - (pToken->z[pToken->n - 1] == '\''); + return token->n >= 2 && token->z[0] == '\'' && + token->z[token->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)) I’d better use sql_token() function. > + { Nit: put bracer at the previous line: if (…) { > + 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/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 > +--- > +--- cleanup > +— In sql-tap/ tests we don’t need cleanup: it is done automatically after execution. > +test:do_execsql_test( > + "pragma-8.1", > + [[ > + DROP TABLE IF EXISTS gh3733; > + ]], { > + > +}) > + > +test:finish_test()
next prev parent reply other threads:[~2019-02-08 19:12 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-08 10:19 [tarantool-patches] " Stanislav Zudin 2019-02-08 19:12 ` n.pettik [this message] 2019-02-11 13:17 ` [tarantool-patches] [PATCH v3] sql: remove useless pragmas and unused code Stanislav Zudin 2019-02-11 18:24 ` [tarantool-patches] " n.pettik
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=A2AFEEC4-AA8A-49CA-86AE-59E0020F8BE3@tarantool.org \ --to=korablev@tarantool.org \ --cc=szudin@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v2] sql: remove useless pragmas' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox