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 BF5E72DD10 for ; Tue, 28 May 2019 07:39:40 -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 xipKE6kTILhA for ; Tue, 28 May 2019 07:39:40 -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 83A1C2D813 for ; Tue, 28 May 2019 07:39:40 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v1 5/9] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt Date: Tue, 28 May 2019 14:39:38 +0300 Message-Id: In-Reply-To: References: 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: v.shpilevoy@tarantool.org Cc: tarantool-patches@freelists.org 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 --- src/box/sql/build.c | 20 -------------------- src/box/sql/expr.c | 7 ++++--- src/box/sql/fk_constraint.c | 7 +++---- src/box/sql/insert.c | 29 ++++++++++++++--------------- src/box/sql/sqlInt.h | 1 - src/box/sql/vdbe.c | 23 ++++------------------- 6 files changed, 25 insertions(+), 62 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 327f6a8..a0d0088 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2977,26 +2977,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 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); } break; } diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c index 7d36edc..602f439 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -287,10 +287,9 @@ 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); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, 0, "FOREIGN "\ + "KEY constraint failed", P4_STATIC); + sqlVdbeChangeP5(v, ER_SQL_EXECUTE); } 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 6f50726..a28fcde 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -853,7 +853,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; @@ -883,20 +882,18 @@ 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("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, + err, P4_STATIC); + sqlVdbeChangeP5(v, ER_SQL_EXECUTE); break; case ON_CONFLICT_ACTION_IGNORE: sqlVdbeAddOp2(v, OP_IsNull, new_tuple_reg + i, @@ -939,11 +936,13 @@ 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("CHECK constraint failed: "\ + "%s", name); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, + on_conflict_check, 0, err, + P4_STATIC); + sqlVdbeChangeP5(v, ER_SQL_EXECUTE); } sqlVdbeResolveLabel(v, all_ok); } 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 3c2cd2b..30ea549 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1031,25 +1031,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) - diag_set(ClientError, pOp->p5, pOp->p4.z); - assert(! diag_is_empty(diag_get())); - } 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) + diag_set(ClientError, pOp->p5, pOp->p4.z); + assert(! diag_is_empty(diag_get())); } rc = sqlVdbeHalt(p); assert(rc==SQL_BUSY || rc==SQL_OK || rc==SQL_ERROR); -- 2.7.4