[tarantool-patches] Re: [PATCH v1 5/9] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt

[Vladislav Shpilevoy <v.shpilevoy@tarantool.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 <v.shpilevoy@tarantool.org>
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);