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 A5359306EA for ; Tue, 4 Jun 2019 15:34:56 -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 RMrTdjPPhPTN for ; Tue, 4 Jun 2019 15:34:56 -0400 (EDT) Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (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 60E3830639 for ; Tue, 4 Jun 2019 15:34:56 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 5/9] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt References: <20190603115351.GA24317@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Tue, 4 Jun 2019 21:34:52 +0200 MIME-Version: 1.0 In-Reply-To: <20190603115351.GA24317@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Mergen Imeev Cc: tarantool-patches@freelists.org Hi! Thanks for the fixes! I noticed, that in many places you copy-pasted ER_SQL_EXECUTE error message. It should not be so. Consider my fixes below and on the branch: ============================================================================== commit b7b929b7cfd3f45a26be797bfbe1cdc7e7379edc Author: Vladislav Shpilevoy Date: Tue Jun 4 21:20:22 2019 +0200 Review fixes diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 4fbc8f53f..652e5a273 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -4386,9 +4386,9 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) ON_CONFLICT_ACTION_IGNORE, 0, pExpr->u.zToken, 0); } else { - const char *err = tt_sprintf("Failed to execute SQL "\ - "statement: %s", - pExpr->u.zToken); + const char *err = + tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE), + pExpr->u.zToken); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, pExpr->on_conflict_action, ER_SQL_EXECUTE, err, 0); diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c index 5256bec82..660674122 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -287,8 +287,8 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent, * transaction. */ assert(incr_count == 1); - const char *err = "Failed to execute SQL statement: FOREIGN "\ - "KEY constraint failed"; + const char *err = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE), + "FOREIGN KEY constraint failed"); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, ER_SQL_EXECUTE, err, P4_STATIC); } else { diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 286a72134..4e93a83e3 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -856,6 +856,7 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space, bool is_update = upd_cols != NULL; assert(space != NULL); struct space_def *def = space->def; + const char *err; /* Insertion into VIEW is prohibited. */ assert(!def->opts.is_view); uint32_t autoinc_fieldno = sql_space_autoinc_fieldno(space); @@ -880,15 +881,15 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space, if (on_conflict_nullable == ON_CONFLICT_ACTION_REPLACE && dflt == NULL) on_conflict_nullable = ON_CONFLICT_ACTION_ABORT; - const char *err; int addr; switch (on_conflict_nullable) { case ON_CONFLICT_ACTION_ABORT: case ON_CONFLICT_ACTION_ROLLBACK: case ON_CONFLICT_ACTION_FAIL: - err = tt_sprintf("Failed to execute SQL statement: "\ - "NOT NULL constraint failed: %s.%s", - def->name, def->fields[i].name); + err = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE), + tt_sprintf("NOT NULL constraint "\ + "failed: %s.%s", def->name, + def->fields[i].name)); addr = sqlVdbeAddOp1(v, OP_NotNull, new_tuple_reg + i); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, on_conflict_nullable, ER_SQL_EXECUTE, @@ -936,11 +937,10 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space, char *name = checks->a[i].zName; if (name == NULL) name = def->name; - const char *err = - tt_sprintf("Failed to execute SQL "\ - "statement: CHECK "\ - "constraint failed: %s", - name); + err = tnt_errcode_desc(ER_SQL_EXECUTE); + err = tt_sprintf(err, tt_sprintf("CHECK "\ + "constraint failed: %s", + name)); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, on_conflict_check, ER_SQL_EXECUTE, err, P4_STATIC); diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 4fc59d8b9..9a01c438c 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -2107,12 +2107,12 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) sqlVdbeAddOp2(v, OP_Integer, 0, r1); sqlVdbeAddOp3(v, OP_Ge, r1, positive_limit_label, iLimit); /* Otherwise return an error and stop */ - const char *wrong_limit_error = - "Failed to execute SQL statement: Only positive "\ - "integers are allowed in the LIMIT clause"; + const char *err = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE), + "Only positive integers are "\ + "allowed in the LIMIT clause"); sqlVdbeResolveLabel(v, halt_label); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, - ER_SQL_EXECUTE, wrong_limit_error, P4_STATIC); + ER_SQL_EXECUTE, err, P4_STATIC); sqlVdbeResolveLabel(v, positive_limit_label); VdbeCoverage(v); @@ -2139,12 +2139,12 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) sqlVdbeAddOp2(v, OP_Integer, 1, r1); int no_err = sqlVdbeMakeLabel(v); sqlVdbeAddOp3(v, OP_Eq, iLimit, no_err, r1); - const char *error = - "Failed to execute SQL statement: "\ - "Expression subquery could be limited "\ - "only with 1"; + err = tnt_errcode_desc(ER_SQL_EXECUTE); + err = tt_sprintf(err, "Expression subquery "\ + "could be limited only "\ + "with 1"); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, - 0, ER_SQL_EXECUTE, error, + 0, ER_SQL_EXECUTE, err, P4_STATIC); sqlVdbeResolveLabel(v, no_err); sqlReleaseTempReg(pParse, r1); @@ -2166,14 +2166,12 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) sqlVdbeAddOp3(v, OP_Ge, r1, positive_offset_label, iOffset); /* Otherwise return an error and stop */ - const char *wrong_offset_error = - "Failed to execute SQL statement: Only "\ - "positive integers are allowed in the OFFSET "\ - "clause"; + err = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE), + "Only positive integers are allowed "\ + "in the OFFSET clause"); sqlVdbeResolveLabel(v, offset_error_label); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, - ER_SQL_EXECUTE, wrong_offset_error, - P4_STATIC); + ER_SQL_EXECUTE, err, P4_STATIC); sqlVdbeResolveLabel(v, positive_offset_label); sqlReleaseTempReg(pParse, r1); @@ -5442,9 +5440,9 @@ vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_ma int r1 = sqlGetTempReg(parser); sqlVdbeAddOp2(v, OP_Integer, 0, r1); sqlVdbeAddOp3(v, OP_Ne, r1, end_mark, limit_reg); - const char *error = - "Failed to execute SQL statement: Expression subquery "\ - "returned more than 1 row"; + const char *error = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE), + "Expression subquery returned more "\ + "than 1 row"); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, ER_SQL_EXECUTE, error, P4_STATIC); sqlReleaseTempReg(parser, r1);