[tarantool-patches] Re: [PATCH v2 4/5] sql: remove file zErrMsg of struct Parse
n.pettik
korablev at tarantool.org
Tue Feb 26 17:47:46 MSK 2019
> 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 = true;
>
> sql_resolve_self_reference(&parser, def, NC_IsCheck, NULL, expr_list);
> - int rc = 0;
> - if (parser.rc != SQL_OK) {
> - /* Tarantool error may be already set with diag. */
> - if (parser.rc != SQL_TARANTOOL_ERROR)
> - diag_set(ClientError, ER_SQL, parser.zErrMsg);
> - rc = -1;
> - }
> + if (parser.rc != SQL_OK)
> + return -1;
Since now we have only one possible RC, lets remove
its name and simply check (parser.rc != 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 = &def->fields[def->field_count - 1];
> if (field->nullable_action != ON_CONFLICT_ACTION_DEFAULT &&
> nullable_action != field->nullable_action) {
> - /* Prevent defining nullable_action many times. */
> - const char *err_msg =
> - 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 = 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 = 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 = space_by_name(tab_name);
> if (space == 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 =
> - 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 = (sql_stmt *) sParse.pVdbe;
> }
>
> - 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.
More information about the Tarantool-patches
mailing list