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 48ECA2E756 for ; Mon, 3 Jun 2019 07:53:56 -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 LnZRvkn0Ochr for ; Mon, 3 Jun 2019 07:53:55 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 6DC522E6BD for ; Mon, 3 Jun 2019 07:53:55 -0400 (EDT) Date: Mon, 3 Jun 2019 14:53:51 +0300 From: Mergen Imeev Subject: [tarantool-patches] Re: [PATCH v1 5/9] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt Message-ID: <20190603115351.GA24317@tarantool.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: 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: Vladislav Shpilevoy Cc: tarantool-patches@freelists.org On Sun, Jun 02, 2019 at 06:35:11PM +0200, Vladislav Shpilevoy wrote: > > * This routine is invoked once per CTE by the parser while parsing a > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > > index 6ac42d7..a4a2d71 100644 > > --- a/src/box/sql/expr.c > > +++ b/src/box/sql/expr.c > > @@ -4386,9 +4386,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > > ON_CONFLICT_ACTION_IGNORE, 0, > > pExpr->u.zToken, 0); > > } else { > > - sqlHaltConstraint(pParse, SQL_CONSTRAINT_TRIGGER, > > - pExpr->on_conflict_action, > > - pExpr->u.zToken, 0, 0); > > + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, > > + pExpr->on_conflict_action, 0, > > + pExpr->u.zToken, 0); > > + sqlVdbeChangeP5(v, ER_SQL_EXECUTE); > > Please, remove p5 arg and store error code in p2. Now it can be done > with ease. > I moved the error code from P5 to P3, since P2 is already in use. I also replaced OP_HaltIfNull with OP_NotNull and deleted the opcode OP_HaltIfNull. Diff: diff --git a/src/box/sql/build.c b/src/box/sql/build.c index cc02e70..4c101ca 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -3106,9 +3106,8 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id, if (no_error) { sqlVdbeAddOp0(v, OP_Halt); } else { - sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,0, 0, error, - P4_DYNAMIC); - sqlVdbeChangeP5(v, tarantool_error_code); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, + tarantool_error_code, error, P4_DYNAMIC); } sqlVdbeAddOp1(v, OP_Close, cursor); return 0; diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 5350ed1..4fbc8f5 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -4390,8 +4390,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) "statement: %s", pExpr->u.zToken); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, - pExpr->on_conflict_action, 0, err, 0); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + pExpr->on_conflict_action, ER_SQL_EXECUTE, + err, 0); } break; } diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c index 3468965..5256bec 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -289,9 +289,8 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent, assert(incr_count == 1); const char *err = "Failed to execute SQL statement: FOREIGN "\ "KEY constraint failed"; - sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, 0, err, - P4_STATIC); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, + ER_SQL_EXECUTE, err, P4_STATIC); } 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 eb08aae..286a721 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -889,10 +889,11 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space, err = tt_sprintf("Failed to execute SQL statement: "\ "NOT NULL constraint failed: %s.%s", def->name, def->fields[i].name); - sqlVdbeAddOp4(v, OP_HaltIfNull, SQL_TARANTOOL_ERROR, - on_conflict_nullable, new_tuple_reg + i, + addr = sqlVdbeAddOp1(v, OP_NotNull, new_tuple_reg + i); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, + on_conflict_nullable, ER_SQL_EXECUTE, err, P4_STATIC); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + sqlVdbeJumpHere(v, addr); break; case ON_CONFLICT_ACTION_IGNORE: sqlVdbeAddOp2(v, OP_IsNull, new_tuple_reg + i, @@ -941,9 +942,8 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space, "constraint failed: %s", name); sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, - on_conflict_check, 0, err, - P4_STATIC); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + on_conflict_check, ER_SQL_EXECUTE, + err, P4_STATIC); } sqlVdbeResolveLabel(v, all_ok); } diff --git a/src/box/sql/select.c b/src/box/sql/select.c index b5dc581..4fc59d8 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -2111,12 +2111,8 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) "Failed to execute SQL statement: Only positive "\ "integers are allowed in the LIMIT clause"; sqlVdbeResolveLabel(v, halt_label); - sqlVdbeAddOp4(v, OP_Halt, - SQL_TARANTOOL_ERROR, - 0, 0, - wrong_limit_error, - P4_STATIC); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, + ER_SQL_EXECUTE, wrong_limit_error, P4_STATIC); sqlVdbeResolveLabel(v, positive_limit_label); VdbeCoverage(v); @@ -2147,10 +2143,9 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) "Failed to execute SQL statement: "\ "Expression subquery could be limited "\ "only with 1"; - sqlVdbeAddOp4(v, OP_Halt, - SQL_TARANTOOL_ERROR, - 0, 0, error, P4_STATIC); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, + 0, ER_SQL_EXECUTE, error, + P4_STATIC); sqlVdbeResolveLabel(v, no_err); sqlReleaseTempReg(pParse, r1); @@ -2176,12 +2171,9 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) "positive integers are allowed in the OFFSET "\ "clause"; sqlVdbeResolveLabel(v, offset_error_label); - sqlVdbeAddOp4(v, OP_Halt, - SQL_TARANTOOL_ERROR, - 0, 0, - wrong_offset_error, - P4_STATIC); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, + ER_SQL_EXECUTE, wrong_offset_error, + P4_STATIC); sqlVdbeResolveLabel(v, positive_offset_label); sqlReleaseTempReg(pParse, r1); @@ -5453,9 +5445,8 @@ vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_ma const char *error = "Failed to execute SQL statement: Expression subquery "\ "returned more than 1 row"; - sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, 0, error, - P4_STATIC); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, ER_SQL_EXECUTE, error, + P4_STATIC); sqlReleaseTempReg(parser, r1); } diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index d07c671..33484c5 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -950,29 +950,14 @@ case OP_Yield: { /* in1, jump */ break; } -/* Opcode: HaltIfNull P1 P2 P3 P4 P5 - * Synopsis: if r[P3]=null halt - * - * Check the value in register P3. If it is NULL then Halt using - * parameter P1, P2, and P4 as if this were a Halt instruction. If the - * value in register P3 is not NULL, then this routine is a no-op. - * The P5 parameter should be 1. - */ -case OP_HaltIfNull: { /* in3 */ - pIn3 = &aMem[pOp->p3]; - if ((pIn3->flags & MEM_Null)==0) break; - /* Fall through into OP_Halt */ - FALLTHROUGH; -} - -/* Opcode: Halt P1 P2 * P4 P5 +/* Opcode: Halt P1 P2 P3 P4 * * * Exit immediately. All open cursors, etc are closed * automatically. * * P1 is the result code returned by sql_exec(), * sql_reset(), or sql_finalize(). For a normal halt, - * this should be SQL_OK (0). + * 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 @@ -983,18 +968,8 @@ case OP_HaltIfNull: { /* in3 */ * * If P4 is not null then it is an error message string. * - * If P1 is SQL_TARANTOOL_ERROR then P5 is a ClientError code and - * P4 is error message to set. Else P5 is a value between 0 and 4, - * inclusive, that modifies the P4 string. - * - * 0: (no change) - * 1: NOT NULL contraint failed: P4 - * 2: UNIQUE constraint failed: P4 - * 3: CHECK constraint failed: P4 - * 4: FOREIGN KEY constraint failed: P4 - * - * If P5 is not zero and P4 is NULL, then everything after the - * ":" is omitted. + * If P1 is SQL_TARANTOOL_ERROR 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 @@ -1033,7 +1008,7 @@ case OP_Halt: { if (p->rc) { assert(p->rc == SQL_TARANTOOL_ERROR); if (pOp->p4.z != NULL) - box_error_set(__FILE__, __LINE__, pOp->p5, pOp->p4.z); + box_error_set(__FILE__, __LINE__, pOp->p3, pOp->p4.z); assert(! diag_is_empty(diag_get())); } rc = sqlVdbeHalt(p); New patch: >From 58bb9420e5532868759056503ebe5a2af361aa60 Mon Sep 17 00:00:00 2001 Date: Fri, 12 Apr 2019 14:16:18 +0300 Subject: [PATCH] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt Currently, in OP_Halt, you can get a SQL error other than SQL_TARANTOOL_ERROR, for example, the SQL_CONSTRAINT error. After this patch, all errors going through OP_Halt will have SQL error code SQL_TARANTOOL_ERROR and have diag set. Part of #4074 diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 73748f5..4c101ca 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2998,26 +2998,6 @@ sql_set_multi_write(struct Parse *parse_context, bool is_set) pToplevel->isMultiWrite |= is_set; } -/* - * Code an OP_Halt that causes the vdbe to return an SQL_CONSTRAINT - * error. The onError parameter determines which (if any) of the statement - * and/or current transaction is rolled back. - */ -void -sqlHaltConstraint(Parse * pParse, /* Parsing context */ - int errCode, /* extended error code */ - int onError, /* Constraint type */ - char *p4, /* Error message */ - i8 p4type, /* P4_STATIC or P4_TRANSIENT */ - u8 p5Errmsg /* P5_ErrMsg type */ - ) -{ - Vdbe *v = sqlGetVdbe(pParse); - assert((errCode & 0xff) == SQL_CONSTRAINT); - sqlVdbeAddOp4(v, OP_Halt, errCode, onError, 0, p4, p4type); - sqlVdbeChangeP5(v, p5Errmsg); -} - #ifndef SQL_OMIT_CTE /* * This routine is invoked once per CTE by the parser while parsing a @@ -3126,9 +3106,8 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id, if (no_error) { sqlVdbeAddOp0(v, OP_Halt); } else { - sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,0, 0, error, - P4_DYNAMIC); - sqlVdbeChangeP5(v, tarantool_error_code); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, + tarantool_error_code, error, P4_DYNAMIC); } sqlVdbeAddOp1(v, OP_Close, cursor); return 0; diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 6ac42d7..4fbc8f5 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -4386,9 +4386,12 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) ON_CONFLICT_ACTION_IGNORE, 0, pExpr->u.zToken, 0); } else { - sqlHaltConstraint(pParse, SQL_CONSTRAINT_TRIGGER, - pExpr->on_conflict_action, - pExpr->u.zToken, 0, 0); + const char *err = tt_sprintf("Failed to execute SQL "\ + "statement: %s", + pExpr->u.zToken); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, + pExpr->on_conflict_action, ER_SQL_EXECUTE, + err, 0); } break; } diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c index 7d36edc..5256bec 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -287,10 +287,10 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent, * transaction. */ assert(incr_count == 1); - sqlHaltConstraint(parse_context, - SQL_CONSTRAINT_FOREIGNKEY, - ON_CONFLICT_ACTION_ABORT, 0, P4_STATIC, - P5_ConstraintFK); + const char *err = "Failed to execute SQL statement: FOREIGN "\ + "KEY constraint failed"; + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, + ER_SQL_EXECUTE, err, P4_STATIC); } 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 2950240..286a721 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -851,7 +851,6 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space, enum on_conflict_action on_conflict, int ignore_label, int *upd_cols) { - struct sql *db = parse_context->db; struct Vdbe *v = sqlGetVdbe(parse_context); assert(v != NULL); bool is_update = upd_cols != NULL; @@ -881,20 +880,20 @@ 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; - char *err_msg; + 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_msg = sqlMPrintf(db, "%s.%s", def->name, - def->fields[i].name); - sqlVdbeAddOp3(v, OP_HaltIfNull, - SQL_CONSTRAINT_NOTNULL, - on_conflict_nullable, - new_tuple_reg + i); - sqlVdbeAppendP4(v, err_msg, P4_DYNAMIC); - sqlVdbeChangeP5(v, P5_ConstraintNotNull); + err = tt_sprintf("Failed to execute SQL statement: "\ + "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, + err, P4_STATIC); + sqlVdbeJumpHere(v, addr); break; case ON_CONFLICT_ACTION_IGNORE: sqlVdbeAddOp2(v, OP_IsNull, new_tuple_reg + i, @@ -937,11 +936,14 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space, char *name = checks->a[i].zName; if (name == NULL) name = def->name; - sqlHaltConstraint(parse_context, - SQL_CONSTRAINT_CHECK, - on_conflict_check, name, - P4_TRANSIENT, - P5_ConstraintCheck); + const char *err = + tt_sprintf("Failed to execute SQL "\ + "statement: CHECK "\ + "constraint failed: %s", + name); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, + on_conflict_check, ER_SQL_EXECUTE, + err, P4_STATIC); } sqlVdbeResolveLabel(v, all_ok); } diff --git a/src/box/sql/select.c b/src/box/sql/select.c index d3472a9..4fc59d8 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -2108,14 +2108,11 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) sqlVdbeAddOp3(v, OP_Ge, r1, positive_limit_label, iLimit); /* Otherwise return an error and stop */ const char *wrong_limit_error = - "Only positive integers are allowed " - "in the LIMIT clause"; + "Failed to execute SQL statement: Only positive "\ + "integers are allowed in the LIMIT clause"; sqlVdbeResolveLabel(v, halt_label); - sqlVdbeAddOp4(v, OP_Halt, - SQL_TARANTOOL_ERROR, - 0, 0, - wrong_limit_error, - P4_STATIC); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, + ER_SQL_EXECUTE, wrong_limit_error, P4_STATIC); sqlVdbeResolveLabel(v, positive_limit_label); VdbeCoverage(v); @@ -2143,12 +2140,12 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) int no_err = sqlVdbeMakeLabel(v); sqlVdbeAddOp3(v, OP_Eq, iLimit, no_err, r1); const char *error = - "SQL error: Expression subquery could " - "be limited only with 1"; - sqlVdbeAddOp4(v, OP_Halt, - SQL_TARANTOOL_ERROR, - 0, 0, error, P4_STATIC); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + "Failed to execute SQL statement: "\ + "Expression subquery could be limited "\ + "only with 1"; + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, + 0, ER_SQL_EXECUTE, error, + P4_STATIC); sqlVdbeResolveLabel(v, no_err); sqlReleaseTempReg(pParse, r1); @@ -2170,14 +2167,13 @@ 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 = - "Only positive integers are allowed " - "in the OFFSET clause"; + "Failed to execute SQL statement: Only "\ + "positive integers are allowed in the OFFSET "\ + "clause"; sqlVdbeResolveLabel(v, offset_error_label); - sqlVdbeAddOp4(v, OP_Halt, - SQL_TARANTOOL_ERROR, - 0, 0, - wrong_offset_error, - P4_STATIC); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, + ER_SQL_EXECUTE, wrong_offset_error, + P4_STATIC); sqlVdbeResolveLabel(v, positive_offset_label); sqlReleaseTempReg(pParse, r1); @@ -5447,10 +5443,10 @@ vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_ma sqlVdbeAddOp2(v, OP_Integer, 0, r1); sqlVdbeAddOp3(v, OP_Ne, r1, end_mark, limit_reg); const char *error = - "SQL error: Expression subquery returned more than 1 row"; - sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, 0, error, - P4_STATIC); - sqlVdbeChangeP5(v, ER_SQL_EXECUTE); + "Failed to execute SQL statement: Expression subquery "\ + "returned more than 1 row"; + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, ER_SQL_EXECUTE, error, + P4_STATIC); sqlReleaseTempReg(parser, r1); } diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 05a4042..5c71c51 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3912,7 +3912,6 @@ vdbe_emit_insertion_completion(struct Vdbe *v, struct space *space, void sql_set_multi_write(Parse *, bool); -void sqlHaltConstraint(Parse *, int, int, char *, i8, u8); Expr *sqlExprDup(sql *, Expr *, int); SrcList *sqlSrcListDup(sql *, SrcList *, int); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index a64c846..33484c5 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -950,29 +950,14 @@ case OP_Yield: { /* in1, jump */ break; } -/* Opcode: HaltIfNull P1 P2 P3 P4 P5 - * Synopsis: if r[P3]=null halt - * - * Check the value in register P3. If it is NULL then Halt using - * parameter P1, P2, and P4 as if this were a Halt instruction. If the - * value in register P3 is not NULL, then this routine is a no-op. - * The P5 parameter should be 1. - */ -case OP_HaltIfNull: { /* in3 */ - pIn3 = &aMem[pOp->p3]; - if ((pIn3->flags & MEM_Null)==0) break; - /* Fall through into OP_Halt */ - FALLTHROUGH; -} - -/* Opcode: Halt P1 P2 * P4 P5 +/* Opcode: Halt P1 P2 P3 P4 * * * Exit immediately. All open cursors, etc are closed * automatically. * * P1 is the result code returned by sql_exec(), * sql_reset(), or sql_finalize(). For a normal halt, - * this should be SQL_OK (0). + * 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 @@ -983,18 +968,8 @@ case OP_HaltIfNull: { /* in3 */ * * If P4 is not null then it is an error message string. * - * If P1 is SQL_TARANTOOL_ERROR then P5 is a ClientError code and - * P4 is error message to set. Else P5 is a value between 0 and 4, - * inclusive, that modifies the P4 string. - * - * 0: (no change) - * 1: NOT NULL contraint failed: P4 - * 2: UNIQUE constraint failed: P4 - * 3: CHECK constraint failed: P4 - * 4: FOREIGN KEY constraint failed: P4 - * - * If P5 is not zero and P4 is NULL, then everything after the - * ":" is omitted. + * If P1 is SQL_TARANTOOL_ERROR 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 @@ -1031,28 +1006,10 @@ case OP_Halt: { p->errorAction = (u8)pOp->p2; p->pc = pcx; if (p->rc) { - if (p->rc == SQL_TARANTOOL_ERROR) { - if (pOp->p4.z == NULL) { - assert(! diag_is_empty(diag_get())); - } else { - box_error_set(__FILE__, __LINE__, pOp->p5, - pOp->p4.z); - } - } else if (pOp->p5 != 0) { - static const char * const azType[] = { "NOT NULL", "UNIQUE", "CHECK", - "FOREIGN KEY" }; - testcase( pOp->p5==1); - testcase( pOp->p5==2); - testcase( pOp->p5==3); - testcase( pOp->p5==4); - sqlVdbeError(p, "%s constraint failed", azType[pOp->p5-1]); - if (pOp->p4.z) { - p->zErrMsg = sqlMPrintf(db, "%z: %s", p->zErrMsg, pOp->p4.z); - } - } else { - sqlVdbeError(p, "%s", pOp->p4.z); - } - sql_log(pOp->p1, "abort at %d in [%s]: %s", pcx, p->zSql, p->zErrMsg); + assert(p->rc == SQL_TARANTOOL_ERROR); + if (pOp->p4.z != NULL) + box_error_set(__FILE__, __LINE__, pOp->p3, pOp->p4.z); + assert(! diag_is_empty(diag_get())); } rc = sqlVdbeHalt(p); assert(rc==SQL_BUSY || rc==SQL_OK || rc==SQL_ERROR); diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua index 6f17471..9968038 100755 --- a/test/sql-tap/e_select1.test.lua +++ b/test/sql-tap/e_select1.test.lua @@ -2170,7 +2170,7 @@ for _, val in ipairs({ "e_select-9.2."..tn, select, { - 1, "Only positive integers are allowed in the LIMIT clause"}) + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause"}) end -- EVIDENCE-OF: R-03014-26414 If the LIMIT expression evaluates to a @@ -2224,7 +2224,7 @@ for _, val in ipairs({ test:do_catchsql_test( "e_select-9.7."..tn, select, { - 1, "Only positive integers are allowed in the OFFSET clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the OFFSET clause" }) end diff --git a/test/sql-tap/limit.test.lua b/test/sql-tap/limit.test.lua index 9b728d8..40b787b 100755 --- a/test/sql-tap/limit.test.lua +++ b/test/sql-tap/limit.test.lua @@ -84,7 +84,7 @@ test:do_catchsql_test( SELECT x FROM t1 ORDER BY x+1 LIMIT 5 OFFSET -2 ]], { -- - 1 ,"Only positive integers are allowed in the OFFSET clause" + 1 ,"Failed to execute SQL statement: Only positive integers are allowed in the OFFSET clause" -- }) @@ -94,7 +94,7 @@ test:do_catchsql_test( SELECT x FROM t1 ORDER BY x+1 LIMIT 2, -5 ]], { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) @@ -115,7 +115,7 @@ test:do_catchsql_test( SELECT x FROM t1 ORDER BY x+1 LIMIT -2, 5 ]], { -- - 1, "Only positive integers are allowed in the OFFSET clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the OFFSET clause" -- }) @@ -135,7 +135,7 @@ test:do_catchsql_test( SELECT x FROM t1 ORDER BY x+1 LIMIT -2, -5 ]], { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) @@ -384,7 +384,7 @@ test:do_catchsql_test( SELECT * FROM t6 LIMIT -1 OFFSET -1; ]], { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) @@ -394,7 +394,7 @@ test:do_catchsql_test( SELECT * FROM t6 LIMIT 2 OFFSET -123; ]], { -- - 1, "Only positive integers are allowed in the OFFSET clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the OFFSET clause" -- }) @@ -414,7 +414,7 @@ test:do_catchsql_test( SELECT * FROM t6 LIMIT -432 OFFSET 2; ]], { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) @@ -434,7 +434,7 @@ test:do_catchsql_test( SELECT * FROM t6 LIMIT -1 ]], { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) @@ -454,7 +454,7 @@ test:do_catchsql_test( SELECT * FROM t6 LIMIT -1 OFFSET 1 ]], { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) @@ -734,7 +734,7 @@ test:do_test( return test:catchsql("SELECT x FROM t1 WHERE x<10 LIMIT "..limit) end, { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) @@ -745,7 +745,7 @@ test:do_test( return test:catchsql("SELECT x FROM t1 WHERE x<10 LIMIT "..limit) end, { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) @@ -1320,7 +1320,7 @@ test:do_catchsql_test( SELECT 123 LIMIT -1 OFFSET 0 ]], { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) @@ -1340,7 +1340,7 @@ test:do_catchsql_test( SELECT 123 LIMIT -1 OFFSET 1 ]], { -- - 1, "Only positive integers are allowed in the LIMIT clause" + 1, "Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) diff --git a/test/sql-tap/select4.test.lua b/test/sql-tap/select4.test.lua index b78091b..1c0804b 100755 --- a/test/sql-tap/select4.test.lua +++ b/test/sql-tap/select4.test.lua @@ -990,7 +990,7 @@ test:do_catchsql_test( SELECT DISTINCT log FROM t1 ORDER BY log LIMIT -1 ]], { -- - 1,"Only positive integers are allowed in the LIMIT clause" + 1,"Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) test:do_execsql_test( @@ -1009,7 +1009,7 @@ test:do_catchsql_test( SELECT DISTINCT log FROM t1 ORDER BY log LIMIT -1 OFFSET 2 ]], { -- - 1,"Only positive integers are allowed in the LIMIT clause" + 1,"Failed to execute SQL statement: Only positive integers are allowed in the LIMIT clause" -- }) test:do_execsql_test( @@ -1402,7 +1402,7 @@ test:do_catchsql_test( SELECT (VALUES(1),(2),(3),(4)) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -1412,7 +1412,7 @@ test:do_catchsql_test( SELECT (SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3 UNION ALL SELECT 4) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) diff --git a/test/sql-tap/subselect.test.lua b/test/sql-tap/subselect.test.lua index 5b71390..ebfdf43 100755 --- a/test/sql-tap/subselect.test.lua +++ b/test/sql-tap/subselect.test.lua @@ -350,7 +350,7 @@ test:do_catchsql_test( SELECT (SELECT a FROM t5); ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -360,7 +360,7 @@ test:do_catchsql_test( SELECT b FROM t5 WHERE a = (SELECT a FROM t5 WHERE b=6); ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -380,7 +380,7 @@ test:do_catchsql_test( SELECT b FROM t1 WHERE a = (SELECT a FROM t1 WHERE b=6 LIMIT (SELECT b FROM t1 WHERE a =1)); ]], { -- - 1, "SQL error: Expression subquery could be limited only with 1" + 1, "Failed to execute SQL statement: Expression subquery could be limited only with 1" -- }) diff --git a/test/sql-tap/tkt1473.test.lua b/test/sql-tap/tkt1473.test.lua index 3e93203..ada18d0 100755 --- a/test/sql-tap/tkt1473.test.lua +++ b/test/sql-tap/tkt1473.test.lua @@ -125,7 +125,7 @@ test:do_catchsql_test( SELECT (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=0) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -145,7 +145,7 @@ test:do_catchsql_test( SELECT (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=4) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -155,7 +155,7 @@ test:do_catchsql_test( SELECT (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -165,7 +165,7 @@ test:do_catchsql_test( SELECT (SELECT 1 FROM t1 WHERE a=0 UNION ALL SELECT 2 FROM t1 WHERE b=4) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -206,7 +206,7 @@ test:do_catchsql_test( (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=0) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -228,7 +228,7 @@ test:do_catchsql_test( (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=4) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -239,7 +239,7 @@ test:do_catchsql_test( (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -250,7 +250,7 @@ test:do_catchsql_test( (SELECT 1 FROM t1 WHERE a=0 UNION ALL SELECT 2 FROM t1 WHERE b=4) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -359,7 +359,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -389,7 +389,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -419,7 +419,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -449,7 +449,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -509,7 +509,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -539,7 +539,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -569,7 +569,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -599,7 +599,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -659,7 +659,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) @@ -689,7 +689,7 @@ test:do_catchsql_test( ) ]], { -- - 1, "SQL error: Expression subquery returned more than 1 row" + 1, "Failed to execute SQL statement: Expression subquery returned more than 1 row" -- }) diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 73497b4..9639ba7 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -164,15 +164,18 @@ cn:execute('select * from test limit ?', {2}) ... cn:execute('select * from test limit ?', {-2}) --- -- error: Only positive integers are allowed in the LIMIT clause +- error: 'Failed to execute SQL statement: Only positive integers are allowed in the + LIMIT clause' ... cn:execute('select * from test limit ?', {2.7}) --- -- error: Only positive integers are allowed in the LIMIT clause +- error: 'Failed to execute SQL statement: Only positive integers are allowed in the + LIMIT clause' ... cn:execute('select * from test limit ?', {'Hello'}) --- -- error: Only positive integers are allowed in the LIMIT clause +- error: 'Failed to execute SQL statement: Only positive integers are allowed in the + LIMIT clause' ... cn:execute('select * from test limit 1 offset ?', {2}) --- @@ -188,15 +191,18 @@ cn:execute('select * from test limit 1 offset ?', {2}) ... cn:execute('select * from test limit 1 offset ?', {-2}) --- -- error: Only positive integers are allowed in the OFFSET clause +- error: 'Failed to execute SQL statement: Only positive integers are allowed in the + OFFSET clause' ... cn:execute('select * from test limit 1 offset ?', {2.7}) --- -- error: Only positive integers are allowed in the OFFSET clause +- error: 'Failed to execute SQL statement: Only positive integers are allowed in the + OFFSET clause' ... cn:execute('select * from test limit 1 offset ?', {'Hello'}) --- -- error: Only positive integers are allowed in the OFFSET clause +- error: 'Failed to execute SQL statement: Only positive integers are allowed in the + OFFSET clause' ... -- gh-2608 SQL iproto DDL cn:execute('create table test2(id int primary key, a int, b int, c int)') diff --git a/test/sql/types.result b/test/sql/types.result index a53d6f7..572a83c 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -383,11 +383,13 @@ box.execute("SELECT true IN (1, 'abc', false)") ... box.execute("SELECT 1 LIMIT true;") --- -- error: Only positive integers are allowed in the LIMIT clause +- error: 'Failed to execute SQL statement: Only positive integers are allowed in the + LIMIT clause' ... box.execute("SELECT 1 LIMIT 1 OFFSET true;") --- -- error: Only positive integers are allowed in the OFFSET clause +- error: 'Failed to execute SQL statement: Only positive integers are allowed in the + OFFSET clause' ... box.execute("SELECT 'abc' || true;") --- @@ -551,7 +553,8 @@ box.execute("SELECT b FROM t GROUP BY b LIMIT 1;") ... box.execute("SELECT b FROM t LIMIT true;") --- -- error: Only positive integers are allowed in the LIMIT clause +- error: 'Failed to execute SQL statement: Only positive integers are allowed in the + LIMIT clause' ... -- Most of aggregates don't accept boolean arguments. --