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 39A6727405 for ; Tue, 26 Feb 2019 09:47:50 -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 AYpeHyud5SDn for ; Tue, 26 Feb 2019 09:47:50 -0500 (EST) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 7087A273F8 for ; Tue, 26 Feb 2019 09:47:49 -0500 (EST) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH v2 4/5] sql: remove file zErrMsg of struct Parse From: "n.pettik" In-Reply-To: <33421784356c1d83b8f38d1ff4c90982f3ad884b.1551114402.git.imeevma@gmail.com> Date: Tue, 26 Feb 2019 17:47:46 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <33421784356c1d83b8f38d1ff4c90982f3ad884b.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 > diff --git a/src/box/sql.c b/src/box/sql.c > index 580f3fa..116e3e8 100644 > --- a/src/box/sql.c > +++ b/src/box/sql.c > @@ -1362,13 +1362,8 @@ sql_checks_resolve_space_def_reference(ExprList = *expr_list, > parser.parse_only =3D true; >=20 > sql_resolve_self_reference(&parser, def, NC_IsCheck, NULL, = expr_list); > - int rc =3D 0; > - if (parser.rc !=3D SQL_OK) { > - /* Tarantool error may be already set with diag. */ > - if (parser.rc !=3D SQL_TARANTOOL_ERROR) > - diag_set(ClientError, ER_SQL, parser.zErrMsg); > - rc =3D -1; > - } > + if (parser.rc !=3D SQL_OK) > + return -1; Since now we have only one possible RC, lets remove its name and simply check (parser.rc !=3D 0). Or, as suggested Konstantin, better replace rc with bool is_aborted = flag. > sql_parser_destroy(&parser); > - return rc; > + return 0; > } > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > index deb5b89..6afca4a 100644 > --- a/src/box/sql/build.c > +++ b/src/box/sql/build.c > @@ -493,16 +493,10 @@ sql_column_add_nullable_action(struct Parse = *parser, > struct field_def *field =3D &def->fields[def->field_count - 1]; > if (field->nullable_action !=3D ON_CONFLICT_ACTION_DEFAULT && > nullable_action !=3D field->nullable_action) { > - /* Prevent defining nullable_action many times. */ > - const char *err_msg =3D > - tt_sprintf("NULL declaration for column '%s' of = table " > - "'%s' has been already set to '%s'", > - field->name, def->name, > - on_conflict_action_strs[field-> > - = nullable_action]); > - diag_set(ClientError, ER_SQL, err_msg); > - parser->rc =3D SQL_TARANTOOL_ERROR; > - parser->nErr++; > + sqlErrorMsg(parser, "NULL declaration for column '%s' of = "\ > + "table '%s' has been already set to '%s'", > + field->name, def->name, > + on_conflict_action_strs[field-> = nullable_action]); This looks like step back in our attempt at using diag_set. We do you need to incapsulate diag into sqlErrorMsg? > return; > } > field->nullable_action =3D nullable_action; > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c > index 5170c7f..a7bf3b3 100644 > --- a/src/box/sql/delete.c > +++ b/src/box/sql/delete.c > @@ -94,31 +94,22 @@ sql_table_truncate(struct Parse *parse, struct = SrcList *tab_list) > struct space *space =3D space_by_name(tab_name); > if (space =3D=3D NULL) { > diag_set(ClientError, ER_NO_SUCH_SPACE, tab_name); > - goto tarantool_error; > + sql_parser_error(parse); Look, anyway you remove this function in next commit. Next time please consider order of refactoring. > } > if (! rlist_empty(&space->parent_fk_constraint)) { > - const char *err_msg =3D > - tt_sprintf("can not truncate space '%s' because = other " > - "objects depend on it", = space->def->name); > - diag_set(ClientError, ER_SQL, err_msg); > - goto tarantool_error; > + sqlErrorMsg(parse, "can not truncate space '%s' because = other " > + "objects depend on it", space->def->name); Replace invocation of sqlErrorMsg with diag_set + parser->rc. The same in other places. > @@ -146,11 +145,7 @@ sqlPrepare(sql * db, /* Database handle. */ > *ppStmt =3D (sql_stmt *) sParse.pVdbe; > } >=20 > - if (zErrMsg) { > - sqlErrorWithMsg(db, rc, "%s", zErrMsg); > - } else { > - sqlError(db, rc); > - } > + sqlError(db, rc); sqlError seems to be useless/dead. Please, make a note somewhere to remove it as follow-up to error-refactoring patch-set.