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 8F44C22212 for ; Fri, 8 Feb 2019 14:12:23 -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 2g1JqorLYGRU for ; Fri, 8 Feb 2019 14:12:23 -0500 (EST) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 BA4AF20B99 for ; Fri, 8 Feb 2019 14:12:22 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH v2] sql: remove useless pragmas From: "n.pettik" In-Reply-To: <20190208101947.8945-1-szudin@tarantool.org> Date: Fri, 8 Feb 2019 22:12:20 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <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 Cc: szudin@tarantool.org > On 8 Feb 2019, at 13:19, Stanislav Zudin wrote: >=20 > 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=20 > 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. >=20 > 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 =3D=3D NULL || tbl_name =3D=3D NULL) > + if (idx_name =3D=3D NULL) { > + diag_set(ClientError, ER_ILLEGAL_PARAMS, > + "index name was not specified"); > + parse->rc =3D SQL_TARANTOOL_ERROR; > + parse->nErr++; > + return; > + } I=E2=80=99m sorry, but it seems that I interpreted comment from P.Gulutzan in a wrong way. We shouldn=E2=80=99t 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 =3D=3D NULL) { > + diag_set(ClientError, ER_ILLEGAL_PARAMS, > + "table name was not specified"); > + parse->rc =3D SQL_TARANTOOL_ERROR; > + parse->nErr++; > return; > + } > + > struct space *space =3D space_by_name(tbl_name); > - if (space =3D=3D NULL) > + if (space =3D=3D NULL) { > + diag_set(ClientError, ER_NO_SUCH_SPACE, > + tbl_name); > + parse->rc =3D SQL_TARANTOOL_ERROR; > + parse->nErr++; > return; > + } > + > uint32_t iid =3D box_index_id_by_name(space->def->id, idx_name, > strlen(idx_name)); > - if (iid =3D=3D BOX_ID_NIL) > + if (iid =3D=3D BOX_ID_NIL) { > + sqlite3ErrorMsg(parse, "no such index: %s", idx_name); Note that in this case you don=E2=80=99t 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 =3D space_index(space, iid); > assert(idx !=3D NULL); > parse->nMem =3D 6; > @@ -389,18 +411,39 @@ sql_pragma_index_list(struct Parse *parse, const = char *tbl_name) > if (tbl_name =3D=3D NULL) > return; > struct space *space =3D space_by_name(tbl_name); > - if (space =3D=3D NULL) > + if (space =3D=3D NULL) { > + diag_set(ClientError, ER_NO_SUCH_SPACE, > + tbl_name); > + parse->rc =3D 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 =3D=3D 0) > + return true; > + return (pToken->n >=3D 2) && > + (pToken->z[0] =3D=3D '\'') && > + (pToken->z[pToken->n - 1] =3D=3D '\''); > +} 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) } } =20 -/* +/** * @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 =3D=3D 0) + if (token =3D=3D NULL || token->n =3D=3D 0) return true; - return (pToken->n >=3D 2) && - (pToken->z[0] =3D=3D '\'') && - (pToken->z[pToken->n - 1] =3D=3D '\''); + return token->n >=3D 2 && token->z[0] =3D=3D '\'' && + token->z[token->n - 1] =3D=3D '\''; } =20 /* > + > /* > * Process a pragma statement. > * > @@ -601,6 +644,14 @@ sqlite3Pragma(Parse * pParse, Token * pId, = /* First part of [schema.]id field */ > } >=20 > case PragTyp_DEFAULT_ENGINE: { > + if (!token_is_string(pValue)) I=E2=80=99d better use sql_token() function. > + { Nit: put bracer at the previous line: if (=E2=80=A6) { > + diag_set(ClientError, ER_ILLEGAL_PARAMS, > + "string value is expected=E2=80=9D); > + pParse->rc =3D SQL_TARANTOOL_ERROR; > + pParse->nErr++; > + goto pragma_out; > + } > if (sql_default_engine_set(zRight) !=3D 0) { > pParse->rc =3D 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 > +=E2=80=94 In sql-tap/ tests we don=E2=80=99t need cleanup: it is done automatically after execution.=20 > +test:do_execsql_test( > + "pragma-8.1", > + [[ > + DROP TABLE IF EXISTS gh3733; > + ]], { > + > +}) > + > +test:finish_test()