Tarantool development patches archive
 help / color / mirror / Atom feed
From: imeevma@tarantool.org
To: korablev@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] [PATCH v1 04/12] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt
Date: Sun,  5 May 2019 15:17:08 +0300	[thread overview]
Message-ID: <15369cd674c3a454e66470edfa19f540c81b9dd0.1557056617.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1557056617.git.imeevma@gmail.com>

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 28dcbc3..32c101d 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2972,26 +2972,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 f725478..dcadd7c 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -865,7 +865,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;
@@ -895,20 +894,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,
@@ -951,11 +948,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 9f0d760..85cec85 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

  parent reply	other threads:[~2019-05-05 12:17 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-05 12:17 [tarantool-patches] [PATCH v1 00/12] sql: set errors in VDBE using diag_set() imeevma
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 01/12] sql: remove errors SQL_TARANTOOL_*_FAIL imeevma
2019-05-15 13:18   ` [tarantool-patches] " n.pettik
2019-05-25  9:16     ` Imeev Mergen
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 02/12] sql: remove error ER_SQL imeevma
2019-05-15 13:18   ` [tarantool-patches] " n.pettik
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 03/12] sql: rework diag_set() in OP_Halt imeevma
2019-05-15 13:18   ` [tarantool-patches] " n.pettik
2019-05-05 12:17 ` imeevma [this message]
2019-05-15 13:18   ` [tarantool-patches] Re: [PATCH v1 04/12] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt n.pettik
2019-05-25  9:18     ` Imeev Mergen
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 05/12] sql: remove error SQL_INTERRUPT imeevma
2019-05-15 13:18   ` [tarantool-patches] " n.pettik
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 06/12] sql: remove error SQL_MISMATCH imeevma
2019-05-15 13:19   ` [tarantool-patches] " n.pettik
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 07/12] sql: set errors in VDBE using diag_set() imeevma
2019-05-15 13:26   ` [tarantool-patches] " n.pettik
2019-05-25 10:24     ` Mergen Imeev
2019-05-25 10:36       ` Imeev Mergen
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 08/12] sql: remove field zErrMsg from struct Vdbe imeevma
2019-05-15 13:30   ` [tarantool-patches] " n.pettik
2019-05-25  9:25     ` Imeev Mergen
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 09/12] sql: remove field pErr from struct sql imeevma
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 10/12] sql: remove field errCode " imeevma
2019-05-15 13:32   ` [tarantool-patches] " n.pettik
2019-05-25  9:25     ` Imeev Mergen
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 11/12] sql: remove sqlError() and remove sqlErrorWithMsg() imeevma
2019-05-05 12:17 ` [tarantool-patches] [PATCH v1 12/12] sql: use diag_set() to set an error in SQL functions imeevma
2019-05-15 14:12   ` [tarantool-patches] " n.pettik
2019-05-25  9:45     ` Mergen Imeev
2019-05-25 10:36       ` Imeev Mergen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=15369cd674c3a454e66470edfa19f540c81b9dd0.1557056617.git.imeevma@gmail.com \
    --to=imeevma@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v1 04/12] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox