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 5434B248D7 for ; Thu, 4 Jul 2019 06:49:44 -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 rKMvsqYBShOI for ; Thu, 4 Jul 2019 06:49:44 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 E0AE820D93 for ; Thu, 4 Jul 2019 06:49:43 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v3 1/3] sql: add OP_SetDiag opcode in VDBE Date: Thu, 4 Jul 2019 13:49:41 +0300 Message-Id: <0a0f232c4c4c7068df8514230c149bc953a080a5.1562235700.git.imeevma@gmail.com> MIME-Version: 1.0 In-Reply-To: References: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit 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: korablev@tarantool.org Cc: tarantool-patches@freelists.org 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;