[tarantool-patches] Re: [PATCH v1 5/9] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Tue Jun 4 22:34:52 MSK 2019
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 at 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);
More information about the Tarantool-patches
mailing list