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 D03B526FDF for ; Mon, 25 Feb 2019 15:02:33 -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 c_sm-_mIdm7y for ; Mon, 25 Feb 2019 15:02:33 -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 008C926E54 for ; Mon, 25 Feb 2019 15:02:32 -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 2/5] sql: rework syntax errors From: "n.pettik" In-Reply-To: Date: Mon, 25 Feb 2019 23:02:30 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: 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 > This patch reworks SQL syntax errors. After this patch, these > error will be set as Tarantool errors. >=20 > Part of #3965 > --- > src/box/errcode.h | 7 +++++ > src/box/sql/build.c | 6 ++--- > src/box/sql/parse.y | 41 = ++++++++++++++++++------------ > src/box/sql/select.c | 11 ++++---- > src/box/sql/tokenize.c | 7 ++--- > test/box/misc.result | 6 +++++ > test/sql-tap/alter2.test.lua | 4 +-- > test/sql-tap/blob.test.lua | 20 +++++++-------- > test/sql-tap/check.test.lua | 10 ++++---- > test/sql-tap/colname.test.lua | 2 +- > test/sql-tap/count.test.lua | 2 +- > test/sql-tap/default.test.lua | 4 +-- > test/sql-tap/e_select1.test.lua | 18 ++++++------- > test/sql-tap/gh-2367-pragma.test.lua | 4 +-- > test/sql-tap/gh2168-temp-tables.test.lua | 4 +-- > test/sql-tap/identifier_case.test.lua | 2 +- > test/sql-tap/index-info.test.lua | 2 +- > test/sql-tap/index1.test.lua | 2 +- > test/sql-tap/index7.test.lua | 2 +- > test/sql-tap/join.test.lua | 2 +- > test/sql-tap/keyword1.test.lua | 2 +- > test/sql-tap/misc1.test.lua | 8 +++--- > test/sql-tap/misc5.test.lua | 6 ++--- > test/sql-tap/null.test.lua | 8 +++--- > test/sql-tap/select1.test.lua | 20 +++++++-------- > test/sql-tap/select3.test.lua | 4 +-- > test/sql-tap/start-transaction.test.lua | 14 +++++----- > test/sql-tap/table.test.lua | 10 ++++---- > test/sql-tap/tkt3935.test.lua | 14 +++++----- > test/sql-tap/tokenize.test.lua | 26 +++++++++---------- > test/sql-tap/trigger1.test.lua | 14 +++++----- > test/sql-tap/view.test.lua | 4 +-- > test/sql-tap/with1.test.lua | 6 ++--- > test/sql-tap/with2.test.lua | 18 ++++++------- > test/sql/checks.result | 2 +- > test/sql/collation.result | 12 ++++----- > test/sql/foreign-keys.result | 10 +++++--- > test/sql/gh-3888-values-blob-assert.result | 8 +++--- > test/sql/iproto.result | 10 ++++---- > test/sql/misc.result | 14 +++++----- > test/sql/on-conflict.result | 16 ++++++------ > test/sql/types.result | 13 ++++++---- > 42 files changed, 213 insertions(+), 182 deletions(-) >=20 > diff --git a/src/box/errcode.h b/src/box/errcode.h > index a1fcf01..6546b2f 100644 > --- a/src/box/errcode.h > +++ b/src/box/errcode.h > @@ -231,6 +231,13 @@ struct errcode_record { > /*176 */_(ER_SQL_CANT_RESOLVE_FIELD, "Can=E2=80=99t resolve = field '%s'") \ > /*177 */_(ER_INDEX_EXISTS_IN_SPACE, "Index '%s' already = exists in space '%s'") \ > /*178 */_(ER_INCONSISTENT_TYPES, "Inconsistent types: = expected %s got %s") \ > + /*179 */_(ER_SQL_SYNTAX, "Syntax error in %s: = %s") \ > + /*180 */_(ER_SQL_STACK_OVERFLOW, "Failed to parse SQL = statement: parser stack limit reached") \ > + /*181 */_(ER_SQL_SELECT_WILDCARD, "Failed to expand '*' in = SELECT statement without FROM clause") \ > + /*182 */_(ER_SQL_STATEMENT_EMPTY, "Failed to execute an = empty SQL statement") \ > + /*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'") \ Quite strange name for err code. I=E2=80=99d rather say ER_SQL_UNRECOGNIZED_SYMBOL/SYNTAX. Is this message suggested by Konstantin? To be honest, I would prefer old one. =E2=80=9CUnrecognized syntax=E2=80=9D doesn=E2=80=99t = sound good and clear enough, at least for me. Personally I would say =E2=80=9CSyntax error near = %s=E2=80=9D. The last one is used in several DBs, so I suppose it is common way to = raise errors like that. > diff --git a/test/sql-tap/alter2.test.lua = b/test/sql-tap/alter2.test.lua > index e219a36..ef57955 100755 > --- a/test/sql-tap/alter2.test.lua > +++ b/test/sql-tap/alter2.test.lua > @@ -223,7 +223,7 @@ test:do_catchsql_test( > ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY REFERENCES = child(id); > ]], { > -- > - 1, "near \"REFERENCES\": syntax error" > + 1, "Unrecognized syntax near 'REFERENCES'" > -- > }) The rest is obvious and OK.