[tarantool-patches] [PATCH v3 1/3] sql: add OP_SetDiag opcode in VDBE
imeevma at tarantool.org
imeevma at tarantool.org
Thu Jul 4 13:49:41 MSK 2019
Hi! Thank you for review. New patch and diff between versions of
the patch below.
On 7/3/19 2:27 AM, n.pettik wrote:
>
>>>> + */
>>>> +case OP_Error: { /* jump */
>>>> + box_error_set(__FILE__, __LINE__, pOp->p1, pOp->p4.z);
>>>
>>> Why not simple diag_set() ?
>>>
>> I did it like this because using diag_set() would be a bit
>> troublesome if we decided to use an argument of a different type
>> or more than one argument.
>
> Do not understand what you mean. Could you provide
> some examples?
>
Sorry, I can't. But I have beed said so by Vlad:
On 6/2/19 7:35 PM, Vladislav Shpilevoy wrote:
> There was a concrete reason, why it was possible - because different
> error codes have different arguments of various types, and the only way
> to set an error at parsing stage is to allow to set arbitrary error
> message to any error code. Without '...', va_arg etc. Besides, we could
> use it to set correct line number and function name in future. Now you
> use diag_set, which restricts us.
>
> So why do you need that patch? We will need to revert it when an error
> appears requiring more than one argument, or an argument of not
> const char * type. That will definitely happen.
>
It can be ween here:
https://www.freelists.org/post/tarantool-patches/PATCH-v1-49-sql-rework-diag-set-in-OP-Halt,1
>> New patch:
>>
>> From 55b8f0d0ac93ed6df10a538e8485d9309c12f2b7 Mon Sep 17 00:00:00 2001
>> Date: Thu, 27 Jun 2019 16:28:30 +0300
>> Subject: [PATCH] sql: add OP_SetDiag opcode in VDBE
>>
>> This opcode is required to set an error using diag_set() without
>> halting VDBE. This will allow us to run destructors between error
>> setting and VDBE halting.
>
> What is destructor? You didn’t mention that now OP_Halt
> doesn’t set errors at all from now. Please, I am asking you
> not for the first time, provide decent commit messages.
>
Fixed.
>> Needed for #4183
>>
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index 292168f..aab60dd 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -3277,15 +3277,15 @@ 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 jmp = 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, jmp);
>
> Are these manipulations with labels related to patch?
> ‘label’ or ‘addr' are more appropriate names IMHO.
>
I think that these manipulations should be done, since in this
patch we should jump to different lengths depending on the value
of no_error. This will be even more severe after the next patch.
>> sqlVdbeAddOp1(v, OP_Close, cursor);
>> return 0;
>> }
>>
>> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
>> index b353148..b065de3 100644
>> --- a/src/box/sql/insert.c
>> +++ b/src/box/sql/insert.c
>> @@ -809,10 +809,10 @@ vdbe_emit_ck_constraint(struct Parse *parser, struct Expr *expr,
>> int check_is_passed = sqlVdbeMakeLabel(v);
>> 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);
>> + const char *err = tt_sprintf(fmt, ck_constraint_name, expr_str);
>
> Why did you rename error_msg to err? It’s redundant diff.
>
Fixed.
>> + sqlVdbeAddOp4(v, OP_SetDiag, ER_CK_CONSTRAINT_FAILED, 0, 0,
>> + sqlDbStrDup(parser->db, err), 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);
>> }
>>
>> @@ -5405,10 +5408,11 @@ 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 = 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);
>> + const char *err = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE),
>> + "Expression subquery returned more than "
>> + "1 row”);
>
> Why do you enlarge diff providing meaningless renaming - error -> err?
>
Fixed.
>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index c8887f9..ffb0df4 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -885,6 +885,21 @@ case OP_Goto: { /* jump */
>> goto jump_to_p2;
>> }
>>
>> +/* Opcode: SetDiag P1 P2 * P4 *
>> + *
>> + * Set an error. After that jump to address P2 if P2 is not 0,
>> + * else go to the next instruction.
>> + *
>> + * The P1 parameter is an error code to be set. The P4 parameter
>> + * is a text description.
>> + */
>> +case OP_SetDiag: { /* jump */
>> + box_error_set(__FILE__, __LINE__, pOp->p1, pOp->p4.z);
>> + if (pOp->p2 == 0)
>> + break;
>> + goto jump_to_p2;
>> +}
>> +
>
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index ffb0df4b3..f0d21dc88 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -885,19 +885,22 @@ case OP_Goto: { /* jump */
> goto jump_to_p2;
> }
>
> -/* Opcode: SetDiag P1 P2 * P4 *
> +/* Opcode: SetDiag P1 P2 * P4 *
> *
> - * Set an error. After that jump to address P2 if P2 is not 0,
> - * else go to the next instruction.
> + * 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.
> *
> - * The P1 parameter is an error code to be set. The P4 parameter
> - * is a text description.
> + * 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)
> - break;
> - goto jump_to_p2;
> + if (pOp->p2 != 0)
> + goto jump_to_p2;
> + break;
> }
>
> /* Opcode: Gosub P1 P2 * * *
>
Fixed.
Diff:
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index aab60dd..5eb11c2 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3277,7 +3277,8 @@ 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 jmp = sqlVdbeAddOp4Int(v, cond_opcode, cursor, 0, key_reg, key_len);
+ int addr = sqlVdbeAddOp4Int(v, cond_opcode, cursor, 0, key_reg,
+ key_len);
if (no_error) {
sqlVdbeAddOp0(v, OP_Halt);
} else {
@@ -3285,7 +3286,7 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id,
P4_DYNAMIC);
sqlVdbeAddOp1(v, OP_Halt, -1);
}
- sqlVdbeJumpHere(v, jmp);
+ sqlVdbeJumpHere(v, addr);
sqlVdbeAddOp1(v, OP_Close, cursor);
return 0;
}
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index b065de3..4386140 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -809,9 +809,9 @@ vdbe_emit_ck_constraint(struct Parse *parser, struct Expr *expr,
int check_is_passed = sqlVdbeMakeLabel(v);
sqlExprIfTrue(parser, expr, check_is_passed, SQL_JUMPIFNULL);
const char *fmt = tnt_errcode_desc(ER_CK_CONSTRAINT_FAILED);
- const char *err = tt_sprintf(fmt, ck_constraint_name, expr_str);
+ const char *error_msg = tt_sprintf(fmt, ck_constraint_name, expr_str);
sqlVdbeAddOp4(v, OP_SetDiag, ER_CK_CONSTRAINT_FAILED, 0, 0,
- sqlDbStrDup(parser->db, err), P4_DYNAMIC);
+ 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);
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 301a4c4..bd144c1 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -5406,10 +5406,10 @@ 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 *err = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE),
- "Expression subquery returned more than "
- "1 row");
- sqlVdbeAddOp4(v, OP_SetDiag, ER_SQL_EXECUTE, 0, 0, err, P4_STATIC);
+ const char *error = tt_sprintf(tnt_errcode_desc(ER_SQL_EXECUTE),
+ "Expression subquery returned more "\
+ "than 1 row");
+ 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 ffb0df4..be78c9b 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -885,19 +885,22 @@ case OP_Goto: { /* jump */
goto jump_to_p2;
}
-/* Opcode: SetDiag P1 P2 * P4 *
+/* Opcode: SetDiag P1 P2 * P4 *
*
- * Set an error. After that jump to address P2 if P2 is not 0,
- * else go to the next instruction.
+ * 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.
*
- * The P1 parameter is an error code to be set. The P4 parameter
- * is a text description.
+ * 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)
- break;
- goto jump_to_p2;
+ if (pOp->p2 != 0)
+ goto jump_to_p2;
+ break;
}
/* Opcode: Gosub P1 P2 * * *
@@ -1006,20 +1009,23 @@ case OP_Yield: { /* in1, jump */
/* Opcode: Halt P1 P2 * * *
*
- * Exit immediately. All open cursors, etc are closed
+ * Exit immediately. All open cursors, etc are closed
* automatically.
*
- * P1 is the result code. For a normal halt, this should be 0. For
- * errors, it is -1. If P1 != 0 then P2 will determine whether or
- * not to rollback the current transaction. Do not rollback if
- * P2 is ON_CONFLICT_ACTION_FAIL. Do the rollback if P2 is
- * ON_CONFLICT_ACTION_ROLLBACK. If P2 is ON_CONFLICT_ACTION_ABORT,
- * then back out all changes that have occurred during this
- * execution of the VDBE, but do not rollback the transaction.
- *
- * There is an implied "Halt 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.
+ * P1 is the result code returned by sql_exec(),
+ * sql_reset(), or sql_finalize(). For a normal halt,
+ * this should be 0.
+ * For errors, it can be some other value. If P1!=0 then P2 will
+ * determine whether or not to rollback the current transaction.
+ * Do not rollback if P2==ON_CONFLICT_ACTION_FAIL. Do the rollback
+ * if P2==ON_CONFLICT_ACTION_ROLLBACK. If
+ * P2==ON_CONFLICT_ACTION_ABORT, then back out all changes that
+ * have occurred during this execution of the VDBE, but do not
+ * rollback the transaction.
+ *
+ * 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.
*/
case OP_Halt: {
VdbeFrame *pFrame;
New patch:
>From 0a0f232c4c4c7068df8514230c149bc953a080a5 Mon Sep 17 00:00:00 2001
Date: Thu, 27 Jun 2019 16:28:30 +0300
Subject: [PATCH] sql: add OP_SetDiag opcode in VDBE
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
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 292168f..5eb11c2 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3277,15 +3277,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 d7104d8..d7d0375 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 c8887f9..be78c9b 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;
More information about the Tarantool-patches
mailing list