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 9F3E220B22 for ; Mon, 25 Feb 2019 18:01:10 -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 xCcIWHjLwNfM for ; Mon, 25 Feb 2019 18:01:10 -0500 (EST) Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (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 573E020B1B for ; Mon, 25 Feb 2019 18:01:10 -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 3/5] sql: save SQL parser errors in diag_set() From: "n.pettik" In-Reply-To: <85c9010267eba42be66e8c945d2b69098238a979.1551114402.git.imeevma@gmail.com> Date: Tue, 26 Feb 2019 02:01:07 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <85c9010267eba42be66e8c945d2b69098238a979.1551114402.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 25 Feb 2019, at 20:14, imeevma@tarantool.org wrote: >=20 > After this patch all SQL parser errors will be saved in diag_set() > instead of field zErrMsg of struct Parse. >=20 > Part of #3965 > --- > src/box/errcode.h | 1 + > src/box/sql/build.c | 3 ++- > src/box/sql/util.c | 6 +++--- > test/box/misc.result | 1 + > test/sql-tap/check.test.lua | 2 +- > test/sql/triggers.result | 4 ++-- > 6 files changed, 10 insertions(+), 7 deletions(-) >=20 > diff --git a/src/box/errcode.h b/src/box/errcode.h > index 6546b2f..779d927 100644 > --- a/src/box/errcode.h > +++ b/src/box/errcode.h > @@ -238,6 +238,7 @@ struct errcode_record { > /*183 */_(ER_SQL_KEYWORD_IS_RESERVED, "Keyword '%.*s' is = reserved. Please use double quotes if '%.*s' is an identifier.") \ > /*184 */_(ER_SQL_SYNTAX_NEAR, "Unrecognized syntax = near '%.*s'") \ > /*185 */_(ER_SQL_UNKNOWN_TOKEN, "Syntax error: = unrecognized token: '%.*s'") \ > + /*186 */_(ER_SQL_PARSER_GENERIC, "%s") \ Hmm, what is this? > /* > * !IMPORTANT! Please follow instructions at start of the file > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > index f112c9f..deb5b89 100644 > --- a/src/box/sql/build.c > +++ b/src/box/sql/build.c > @@ -194,7 +194,8 @@ sql_finish_coding(struct Parse *parse_context) > sqlVdbeMakeReady(v, parse_context); > parse_context->rc =3D SQL_DONE; > } else { > - parse_context->rc =3D SQL_ERROR; > + if (parse_context->rc !=3D SQL_TARANTOOL_ERROR) > + parse_context->rc =3D SQL_ERROR; What a mess, explain this please. > } > } > /** > diff --git a/src/box/sql/util.c b/src/box/sql/util.c > index c89e2e8..e4c93cb 100644 > --- a/src/box/sql/util.c > +++ b/src/box/sql/util.c > @@ -236,10 +236,10 @@ sqlErrorMsg(Parse * pParse, const char *zFormat, = ...) > va_start(ap, zFormat); > zMsg =3D sqlVMPrintf(db, zFormat, ap); > va_end(ap); > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, zMsg); > + sqlDbFree(db, zMsg); > pParse->nErr++; > - sqlDbFree(db, pParse->zErrMsg); > - pParse->zErrMsg =3D zMsg; > - pParse->rc =3D SQL_ERROR; > + pParse->rc =3D SQL_TARANTOOL_ERROR; > } Ok, so you decided to push all errors in one heap. That=E2=80=99s definitely not what we discussed earlier. Konstantin asked you to move each independent error in a separate error code (and I agree with him to a lesser or greater extent). Well, at least group them to semantic/ syntax groups. And instead you simply do this. Then, I don=E2=80=99t understand purpose of all previous patches. Am I missing something?