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 9A522299A3 for ; Thu, 14 Mar 2019 15:26:20 -0400 (EDT) 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 zB4c0n684y91 for ; Thu, 14 Mar 2019 15:26:20 -0400 (EDT) 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 C5E7429982 for ; Thu, 14 Mar 2019 15:26:19 -0400 (EDT) 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 v4 2/8] sql: set SQL parser errors via diag_set() From: "n.pettik" In-Reply-To: <3be89ad14633e9b03c01200ee3d1b3c6776fccc5.1552494059.git.imeevma@gmail.com> Date: Thu, 14 Mar 2019 22:26:16 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <21543710-C2CB-4818-AE87-FA1BE3AEB0A4@tarantool.org> References: <3be89ad14633e9b03c01200ee3d1b3c6776fccc5.1552494059.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 > On 13 Mar 2019, at 20:03, imeevma@tarantool.org wrote: >=20 > Hi! Thank you for review! Diff between versions and new version of > patch below. >=20 > Diff between patches: >=20 > commit 61bc67e61298129d66a436d58957bb411b6c9b81 > Author: Mergen Imeev > Date: Wed Mar 6 21:27:51 2019 +0300 >=20 > Temporary: Review fix >=20 > diff --git a/src/box/sql/malloc.c b/src/box/sql/malloc.c > index e0d2ec8..8812298 100644 > --- a/src/box/sql/malloc.c > +++ b/src/box/sql/malloc.c > @@ -55,9 +55,7 @@ sql_sized_malloc(int nByte) > p[0] =3D nByte; > p++; > } else { > - testcase(sqlGlobalConfig.xLog !=3D 0); > - sql_log(SQL_NOMEM, > - "failed to allocate %u bytes of memory", nByte); > + diag_set(OutOfMemory, nByte, "realloc", "p=E2=80=9D); This function doesn=E2=80=99t set mallocFailed flag. A lot of callers of this function don=E2=80=99d check its return value. So I guess this could result in installed diag error, but it would be ignored. Can we set here at least mallocFailed? > } > return (void *)p; > } > @@ -115,10 +113,7 @@ sql_sized_realloc(void *pPrior, int nByte) > p[0] =3D nByte; > p++; > } else { > - testcase(sqlGlobalConfig.xLog !=3D 0); > - sql_log(SQL_NOMEM, > - "failed memory resize %u to %u bytes", > - sql_sized_sizeof(pPrior), nByte); > + diag_set(OutOfMemory, nByte, "malloc", "p=E2=80=9D); The same is here. > } > return (void *)p; > } > diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c > index 0c6296d..828a1ae 100644 > --- a/src/box/sql/prepare.c > +++ b/src/box/sql/prepare.c > @@ -102,9 +102,8 @@ sqlPrepare(sql * db, /* Database handle. */ >=20 > if (sParse.rc =3D=3D SQL_DONE) > sParse.rc =3D SQL_OK; > - if (db->mallocFailed) { > - sParse.rc =3D SQL_NOMEM; > - } > + if (db->mallocFailed) > + sParse.rc =3D SQL_TARANTOOL_ERROR; > if (pzTail) { > *pzTail =3D sParse.zTail; > } > diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c > index 58685c4..834c165 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=E2=80=9D); I would add to error message max possible length. > + pParse->rc =3D SQL_TARANTOOL_ERROR; > break; > } > } else { > @@ -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=E2=80=9D); What does it mean? AFAIR it is dead code (i.e. everything connected with =E2=80=9Cinterrupt=E2=80=9D). > + pParse->rc =3D SQL_TARANTOOL_ERROR; > break; > } > if (tokenType =3D=3D TK_ILLEGAL) {