[tarantool-patches] [PATCH v3 1/3] sql: add OP_SetDiag opcode in VDBE
imeevma at tarantool.org
imeevma at tarantool.org
Mon Jul 15 18:16:21 MSK 2019
To separate the error setting and execution halting, a new opcode
OP_SetDiag was created. The only functionality of the opcode is
the execution of diag_set(). It is important to note that
OP_SetDiag does not set is_aborted to true, so we can continue
working with other opcodes, if necessary. This function allows us
to perform cleanup in some special cases, for example, when
creating a constraint failed because of the creation of two or
more constraints with the same name in the same CREATE TABLE
statement.
Since now diag_set() is executed in OP_SetDiag, this functionality
has been removed from OP_Halt.
Needed for #4183
---
src/box/sql/build.c | 9 +++++----
src/box/sql/expr.c | 6 ++++--
src/box/sql/fk_constraint.c | 3 ++-
src/box/sql/insert.c | 11 ++++++-----
src/box/sql/select.c | 14 +++++++++-----
src/box/sql/vdbe.c | 31 ++++++++++++++++++++-----------
6 files changed, 46 insertions(+), 28 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 396de63..e56b9da 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3297,15 +3297,16 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id,
int cursor = parser->nTab++;
vdbe_emit_open_cursor(parser, cursor, index_id, space_by_id(space_id));
sqlVdbeChangeP5(v, OPFLAG_SYSTEMSP);
- int label = sqlVdbeCurrentAddr(v);
- sqlVdbeAddOp4Int(v, cond_opcode, cursor, label + 3, key_reg,
- key_len);
+ int addr = sqlVdbeAddOp4Int(v, cond_opcode, cursor, 0, key_reg,
+ key_len);
if (no_error) {
sqlVdbeAddOp0(v, OP_Halt);
} else {
- sqlVdbeAddOp4(v, OP_Halt, -1, 0, tarantool_error_code, error,
+ sqlVdbeAddOp4(v, OP_SetDiag, tarantool_error_code, 0, 0, error,
P4_DYNAMIC);
+ sqlVdbeAddOp1(v, OP_Halt, -1);
}
+ sqlVdbeJumpHere(v, addr);
sqlVdbeAddOp1(v, OP_Close, cursor);
return 0;
}
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 7673832..916b0a0 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4339,8 +4339,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
const char *err =
tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE),
pExpr->u.zToken);
- sqlVdbeAddOp4(v, OP_Halt, -1, pExpr->on_conflict_action,
- ER_SQL_EXECUTE, err, 0);
+ sqlVdbeAddOp4(v, OP_SetDiag, ER_SQL_EXECUTE, 0, 0, err,
+ P4_STATIC);
+ sqlVdbeAddOp2(v, OP_Halt, -1,
+ pExpr->on_conflict_action);
}
break;
}
diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c
index 06c863a..4717677 100644
--- a/src/box/sql/fk_constraint.c
+++ b/src/box/sql/fk_constraint.c
@@ -287,8 +287,9 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent,
assert(incr_count == 1);
const char *err = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE),
"FOREIGN KEY constraint failed");
- sqlVdbeAddOp4(v, OP_Halt, -1, 0, ER_SQL_EXECUTE, err,
+ sqlVdbeAddOp4(v, OP_SetDiag, ER_SQL_EXECUTE, 0, 0, err,
P4_STATIC);
+ sqlVdbeAddOp1(v, OP_Halt, -1);
} else {
sqlVdbeAddOp2(v, OP_FkCounter, fk_def->is_deferred,
incr_count);
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index b353148..4386140 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -810,9 +810,9 @@ vdbe_emit_ck_constraint(struct Parse *parser, struct Expr *expr,
sqlExprIfTrue(parser, expr, check_is_passed, SQL_JUMPIFNULL);
const char *fmt = tnt_errcode_desc(ER_CK_CONSTRAINT_FAILED);
const char *error_msg = tt_sprintf(fmt, ck_constraint_name, expr_str);
- sqlVdbeAddOp4(v, OP_Halt, -1, ON_CONFLICT_ACTION_ABORT,
- 0, sqlDbStrDup(parser->db, error_msg), P4_DYNAMIC);
- sqlVdbeChangeP5(v, ER_CK_CONSTRAINT_FAILED);
+ sqlVdbeAddOp4(v, OP_SetDiag, ER_CK_CONSTRAINT_FAILED, 0, 0,
+ sqlDbStrDup(parser->db, error_msg), P4_DYNAMIC);
+ sqlVdbeAddOp2(v, OP_Halt, -1, ON_CONFLICT_ACTION_ABORT);
VdbeNoopComment((v, "END: ck constraint %s test", ck_constraint_name));
sqlVdbeResolveLabel(v, check_is_passed);
}
@@ -863,8 +863,9 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space,
"failed: %s.%s", def->name,
def->fields[i].name));
addr = sqlVdbeAddOp1(v, OP_NotNull, new_tuple_reg + i);
- sqlVdbeAddOp4(v, OP_Halt, -1, on_conflict_nullable,
- ER_SQL_EXECUTE, err, P4_STATIC);
+ sqlVdbeAddOp4(v, OP_SetDiag, ER_SQL_EXECUTE, 0, 0, err,
+ P4_STATIC);
+ sqlVdbeAddOp2(v, OP_Halt, -1, on_conflict_nullable);
sqlVdbeJumpHere(v, addr);
break;
case ON_CONFLICT_ACTION_IGNORE:
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 7c8da25..bd144c1 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -2092,8 +2092,9 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
"Only positive integers are "\
"allowed in the LIMIT clause");
sqlVdbeResolveLabel(v, halt_label);
- sqlVdbeAddOp4(v, OP_Halt, -1, 0, ER_SQL_EXECUTE, err,
+ sqlVdbeAddOp4(v, OP_SetDiag, ER_SQL_EXECUTE, 0, 0, err,
P4_STATIC);
+ sqlVdbeAddOp1(v, OP_Halt, -1);
sqlVdbeResolveLabel(v, positive_limit_label);
VdbeCoverage(v);
@@ -2124,8 +2125,9 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
err = tt_sprintf(err, "Expression subquery "\
"could be limited only "\
"with 1");
- sqlVdbeAddOp4(v, OP_Halt, -1, 0, ER_SQL_EXECUTE,
- err, P4_STATIC);
+ sqlVdbeAddOp4(v, OP_SetDiag, ER_SQL_EXECUTE, 0,
+ 0, err, P4_STATIC);
+ sqlVdbeAddOp1(v, OP_Halt, -1);
sqlVdbeResolveLabel(v, no_err);
sqlReleaseTempReg(pParse, r1);
@@ -2150,8 +2152,9 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
"Only positive integers are allowed "\
"in the OFFSET clause");
sqlVdbeResolveLabel(v, offset_error_label);
- sqlVdbeAddOp4(v, OP_Halt, -1, 0, ER_SQL_EXECUTE, err,
+ sqlVdbeAddOp4(v, OP_SetDiag, ER_SQL_EXECUTE, 0, 0, err,
P4_STATIC);
+ sqlVdbeAddOp1(v, OP_Halt, -1);
sqlVdbeResolveLabel(v, positive_offset_label);
sqlReleaseTempReg(pParse, r1);
@@ -5406,7 +5409,8 @@ vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_ma
const char *error = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE),
"Expression subquery returned more "\
"than 1 row");
- sqlVdbeAddOp4(v, OP_Halt, -1, 0, ER_SQL_EXECUTE, error, P4_STATIC);
+ sqlVdbeAddOp4(v, OP_SetDiag, ER_SQL_EXECUTE, 0, 0, error, P4_STATIC);
+ sqlVdbeAddOp1(v, OP_Halt, -1);
sqlReleaseTempReg(parser, r1);
}
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 9f4ee7a..6a4a303 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -885,6 +885,24 @@ case OP_Goto: { /* jump */
goto jump_to_p2;
}
+/* Opcode: SetDiag P1 P2 * P4 *
+ *
+ * Set diag error. After that jump to address P2 if it is not 0.
+ * Otherwise, go to the next instruction. Note that is_aborted
+ * flag is not set in this case, which allows to continue VDBE
+ * execution. For instance, to provide auxiliary query-specific
+ * clean-up.
+ *
+ * P1 parameter is an error code to be set. The P4 parameter is a
+ * text description of the error.
+ */
+case OP_SetDiag: { /* jump */
+ box_error_set(__FILE__, __LINE__, pOp->p1, pOp->p4.z);
+ if (pOp->p2 != 0)
+ goto jump_to_p2;
+ break;
+}
+
/* Opcode: Gosub P1 P2 * * *
*
* Write the current address onto register P1
@@ -989,7 +1007,7 @@ case OP_Yield: { /* in1, jump */
break;
}
-/* Opcode: Halt P1 P2 P3 P4 *
+/* Opcode: Halt P1 P2 * * *
*
* Exit immediately. All open cursors, etc are closed
* automatically.
@@ -1005,11 +1023,6 @@ case OP_Yield: { /* in1, jump */
* have occurred during this execution of the VDBE, but do not
* rollback the transaction.
*
- * If P4 is not null then it is an error message string.
- *
- * If P1 is -1 then P3 is a ClientError code and P4 is error
- * message to set.
- *
* There is an implied "Halt 0 0 0" instruction inserted at the
* very end of every program. So a jump past the last instruction
* of the program is the same as executing Halt.
@@ -1017,6 +1030,7 @@ case OP_Yield: { /* in1, jump */
case OP_Halt: {
VdbeFrame *pFrame;
int pcx;
+ assert(pOp->p1 == 0 || ! diag_is_empty(diag_get()));
pcx = (int)(pOp - aOp);
if (pOp->p1 == 0 && p->pFrame != NULL) {
@@ -1045,11 +1059,6 @@ case OP_Halt: {
p->is_aborted = true;
p->errorAction = (u8)pOp->p2;
p->pc = pcx;
- if (p->is_aborted) {
- if (pOp->p4.z != NULL)
- box_error_set(__FILE__, __LINE__, pOp->p3, pOp->p4.z);
- assert(! diag_is_empty(diag_get()));
- }
sqlVdbeHalt(p);
rc = p->is_aborted ? -1 : SQL_DONE;
goto vdbe_return;
--
2.7.4
More information about the Tarantool-patches
mailing list