[tarantool-patches] Re: [PATCH v1 5/9] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt

Mergen Imeev imeevma at tarantool.org
Mon Jun 3 14:53:51 MSK 2019


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
     ]], {
         -- <limit-1.2.13>
-        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"
         -- </limit-1.2.13>
     })
 
@@ -94,7 +94,7 @@ test:do_catchsql_test(
         SELECT x FROM t1 ORDER BY x+1 LIMIT 2, -5
     ]], {
         -- <limit-1.2.4>
-        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"
         -- </limit-1.2.4>
     })
 
@@ -115,7 +115,7 @@ test:do_catchsql_test(
         SELECT x FROM t1 ORDER BY x+1 LIMIT -2, 5
     ]], {
         -- <limit-1.2.6>
-        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"
         -- </limit-1.2.6>
     })
 
@@ -135,7 +135,7 @@ test:do_catchsql_test(
         SELECT x FROM t1 ORDER BY x+1 LIMIT -2, -5
     ]], {
         -- <limit-1.2.8>
-        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"
         -- </limit-1.2.8>
     })
 
@@ -384,7 +384,7 @@ test:do_catchsql_test(
         SELECT * FROM t6 LIMIT -1 OFFSET -1;
     ]], {
         -- <limit-6.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"
         -- </limit-6.2>
     })
 
@@ -394,7 +394,7 @@ test:do_catchsql_test(
         SELECT * FROM t6 LIMIT 2 OFFSET -123;
     ]], {
         -- <limit-6.3>
-        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"
         -- </limit-6.3>
     })
 
@@ -414,7 +414,7 @@ test:do_catchsql_test(
         SELECT * FROM t6 LIMIT -432 OFFSET 2;
     ]], {
         -- <limit-6.4>
-        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"
         -- </limit-6.4>
     })
 
@@ -434,7 +434,7 @@ test:do_catchsql_test(
         SELECT * FROM t6 LIMIT -1
     ]], {
         -- <limit-6.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"
         -- </limit-6.5>
     })
 
@@ -454,7 +454,7 @@ test:do_catchsql_test(
         SELECT * FROM t6 LIMIT -1 OFFSET 1
     ]], {
         -- <limit-6.6>
-        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"
         -- </limit-6.6>
     })
 
@@ -734,7 +734,7 @@ test:do_test(
         return test:catchsql("SELECT x FROM t1 WHERE x<10 LIMIT "..limit)
     end, {
         -- <limit-10.4>
-        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"
         -- </limit-10.4>
     })
 
@@ -745,7 +745,7 @@ test:do_test(
         return test:catchsql("SELECT x FROM t1 WHERE x<10 LIMIT "..limit)
     end, {
         -- <limit-10.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"
         -- </limit-10.5>
     })
 
@@ -1320,7 +1320,7 @@ test:do_catchsql_test(
         SELECT 123 LIMIT -1 OFFSET 0
     ]], {
         -- <limit-14.6.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"
         -- </limit-14.6.1>
     })
 
@@ -1340,7 +1340,7 @@ test:do_catchsql_test(
         SELECT 123 LIMIT -1 OFFSET 1
     ]], {
         -- <limit-14.7.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"
         -- </limit-14.7.1>
     })
 
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
     ]], {
         -- <select4-10.4.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"
         -- </select4-10.4.1>
     })
 test:do_execsql_test(
@@ -1009,7 +1009,7 @@ test:do_catchsql_test(
         SELECT DISTINCT log FROM t1 ORDER BY log LIMIT -1 OFFSET 2
     ]], {
         -- <select4-10.5.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"
         -- </select4-10.5.1>
     })
 test:do_execsql_test(
@@ -1402,7 +1402,7 @@ test:do_catchsql_test(
         SELECT (VALUES(1),(2),(3),(4))
     ]], {
         -- <select4-14.10>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </select4-14.10>
     })
 
@@ -1412,7 +1412,7 @@ test:do_catchsql_test(
         SELECT (SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3 UNION ALL SELECT 4)
     ]], {
         -- <select4-14.11>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </select4-14.11>
     })
 
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);
     ]], {
     -- <subselect-5.1>
-    1, "SQL error: Expression subquery returned more than 1 row"
+    1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
     -- </subselect-5.1>
 })
 
@@ -360,7 +360,7 @@ test:do_catchsql_test(
         SELECT b FROM t5 WHERE a = (SELECT a FROM t5 WHERE b=6);
     ]], {
     -- <subselect-5.2>
-    1, "SQL error: Expression subquery returned more than 1 row"
+    1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
     -- </subselect-5.2>
 })
 
@@ -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));
     ]], {
     -- <subselect-5.2>
-    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"
     -- </subselect-5.2>
 })
 
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)
     ]], {
         -- <tkt1473-2.2>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-2.2>
     })
 
@@ -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)
     ]], {
         -- <tkt1473-2.4>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-2.4>
     })
 
@@ -155,7 +155,7 @@ test:do_catchsql_test(
         SELECT (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-2.5>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-2.5>
     })
 
@@ -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)
     ]], {
         -- <tkt1473-2.6>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-2.6>
     })
 
@@ -206,7 +206,7 @@ test:do_catchsql_test(
           (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=0)
     ]], {
         -- <tkt1473-3.2>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-3.2>
     })
 
@@ -228,7 +228,7 @@ test:do_catchsql_test(
           (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-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"
         -- </tkt1473-3.4>
     })
 
@@ -239,7 +239,7 @@ test:do_catchsql_test(
           (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-3.5>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-3.5>
     })
 
@@ -250,7 +250,7 @@ test:do_catchsql_test(
           (SELECT 1 FROM t1 WHERE a=0 UNION ALL SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-3.6>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-3.6>
     })
 
@@ -359,7 +359,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-4.3>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-4.3>
     })
 
@@ -389,7 +389,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-4.4>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-4.4>
     })
 
@@ -419,7 +419,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-4.5>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-4.5>
     })
 
@@ -449,7 +449,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-4.6>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-4.6>
     })
 
@@ -509,7 +509,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-5.3>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-5.3>
     })
 
@@ -539,7 +539,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-5.4>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-5.4>
     })
 
@@ -569,7 +569,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-5.5>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-5.5>
     })
 
@@ -599,7 +599,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-5.6>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-5.6>
     })
 
@@ -659,7 +659,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-6.3>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-6.3>
     })
 
@@ -689,7 +689,7 @@ test:do_catchsql_test(
         )
     ]], {
         -- <tkt1473-6.4>
-        1, "SQL error: Expression subquery returned more than 1 row"
+        1, "Failed to execute SQL statement: Expression subquery returned more than 1 row"
         -- </tkt1473-6.4>
     })
 
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.
 --







More information about the Tarantool-patches mailing list