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 45326246FF for ; Tue, 5 Mar 2019 04:06:29 -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 trOcrtNv2-Yl for ; Tue, 5 Mar 2019 04:06:29 -0500 (EST) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (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 0298C20210 for ; Tue, 5 Mar 2019 04:06:29 -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 v3 3/9] sql: remove field nErr of struct Parse From: "n.pettik" In-Reply-To: <0448cc416f3401eeecbad1883fea7193399a72aa.1551530224.git.imeevma@gmail.com> Date: Tue, 5 Mar 2019 12:06:26 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <5CA490C5-A18E-4A2B-8EB0-55A2EE273963@tarantool.org> References: <0448cc416f3401eeecbad1883fea7193399a72aa.1551530224.git.imeevma@gmail.com> 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: Imeev Mergen > New version: >=20 > commit 0448cc416f3401eeecbad1883fea7193399a72aa > Author: Mergen Imeev > Date: Tue Feb 26 22:22:26 2019 +0300 >=20 > sql: remove field nErr of struct Parse Nit: remove from (not of). >=20 > At the moment, the only purpose of the field nErr of struct Parse > is to show whether the field rc of the same struct is SQL_OK or > SQL_TARANTOOL_ERROR. Let's remove it. >=20 > Part of #3965 The same problem with ordering here: you replaced nErr > 0 check with rc =3D=3D SQL_TARANTOOL_ERROR and then replaced it with is_aborted. If you firstly replaced rc with is_aborted and then nErr > 0 -> is_aborted - diff would be slightly smaller. > @@ -145,9 +144,11 @@ sql_finish_coding(struct Parse *parse_context) > "Exit with an error if CREATE statement = fails")); > } >=20 > - if (db->mallocFailed || parse_context->nErr !=3D 0) { > - if (parse_context->rc =3D=3D SQL_OK) > - parse_context->rc =3D SQL_ERROR; > + if (parse_context->rc =3D=3D SQL_TARANTOOL_ERROR) > + return; > + if (db->mallocFailed) { > + diag_set(OutOfMemory, 0, "SQL", "db=E2=80=9D); It is not a place to set OOM. Firstly, there are a lot of other places where mallocFailed is set, but diag is not. You should set this error in sqlOomFault(). Check if any other function also can set this flag. > + parse_context->rc =3D SQL_TARANTOOL_ERROR; > return; > } > /* > @@ -189,13 +190,12 @@ sql_finish_coding(struct Parse *parse_context) > sqlVdbeGoto(v, 1); > } > /* Get the VDBE program ready for execution. */ > - if (parse_context->nErr =3D=3D 0 && !db->mallocFailed) { > + if (parse_context->rc =3D=3D SQL_OK && !db->mallocFailed) { > assert(parse_context->iCacheLevel =3D=3D 0); > sqlVdbeMakeReady(v, parse_context); > - parse_context->rc =3D SQL_DONE; > - } else { > - if (parse_context->rc !=3D SQL_TARANTOOL_ERROR) > - parse_context->rc =3D SQL_ERROR; > + } else if (parse_context->rc !=3D SQL_TARANTOOL_ERROR){ > + diag_set(OutOfMemory, 0, "SQL", "db"); > + parse_context->rc =3D SQL_TARANTOOL_ERROR; The same here. > @@ -1284,7 +1273,7 @@ sql_create_view(struct Parse *parse_context, = struct Token *begin, > } > sqlStartTable(parse_context, name, if_exists); > struct space *space =3D parse_context->new_space; > - if (space =3D=3D NULL || parse_context->nErr !=3D 0) > + if (space =3D=3D NULL || parse_context->rc =3D=3D = SQL_TARANTOOL_ERROR) Instead of checking that rc =3D=3D SQL_TARANTOOL_ERROR, I=E2=80=99d = rather test that rc !=3D 0. It doesn=E2=80=99t matter now, since rc is anyway = removed. > diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c > index 58685c4..fac2781 100644 > --- a/src/box/sql/tokenize.c > +++ b/src/box/sql/tokenize.c > @@ -483,7 +483,9 @@ sqlRunParser(Parse * pParse, const char *zSql, = char **pzErrMsg) > &pParse->sLastToken.isReserved); > i +=3D pParse->sLastToken.n; > if (i > mxSqlLen) { > - pParse->rc =3D SQL_TOOBIG; > + diag_set(ClientError, = ER_SQL_PARSER_GENERIC, > + "string or blob too big"); > + pParse->rc =3D SQL_TARANTOOL_ERROR; > break; Move this change to previous patch. > @@ -502,7 +504,9 @@ sqlRunParser(Parse * pParse, const char *zSql, = char **pzErrMsg) > assert(tokenType =3D=3D TK_SPACE > || tokenType =3D=3D TK_ILLEGAL); > if (db->u1.isInterrupted) { > - pParse->rc =3D SQL_INTERRUPT; > + diag_set(ClientError, = ER_SQL_PARSER_GENERIC, > + "interrupted"); > + pParse->rc =3D SQL_TARANTOOL_ERROR; The same. > @@ -529,7 +533,8 @@ sqlRunParser(Parse * pParse, const char *zSql, = char **pzErrMsg) > #endif /* YYDEBUG */ > sqlParserFree(pEngine, sql_free); > if (db->mallocFailed) { > - pParse->rc =3D SQL_NOMEM; > + diag_set(OutOfMemory, 0, "SQL", "db"); > + pParse->rc =3D SQL_TARANTOOL_ERROR; > } The same. > if (pParse->rc !=3D SQL_OK && pParse->rc !=3D SQL_DONE > && pParse->zErrMsg =3D=3D 0) { > diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c > index f67c32e..a42e872 100644 > --- a/src/box/sql/vdbemem.c > +++ b/src/box/sql/vdbemem.c > @@ -1227,7 +1227,8 @@ valueFromFunction(sql * db, /* The database = connection */ > sql_value_apply_type(pVal, type); > assert(rc =3D=3D SQL_OK); > } > - pCtx->pParse->rc =3D rc; > + if (rc !=3D SQL_OK) > + pCtx->pParse->rc =3D SQL_TARANTOOL_ERROR; >=20 Why did you change this code? > value_from_function_out: > if (rc !=3D SQL_OK) {