[tarantool-patches] [PATCH v1 0/3] sql: set errors in VDBE using diag

Tarantool development patches archive
 help / color / mirror / Atom feed

* [tarantool-patches] [PATCH v1 0/3] sql: set errors in VDBE using diag_set()
@ 2019-04-26 12:23 imeevma
  2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 1/3] sql: remove error SQL_INTERRUPT imeevma
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: imeevma @ 2019-04-26 12:23 UTC (permalink / raw)
  To: korablev; +Cc: tarantool-patches

The main purpose of this patch is to make sure that after
executing VDBE with an error, diag will be set.

https://github.com/tarantool/tarantool/issues/4074
https://github.com/tarantool/tarantool/tree/imeevma/gh-4074-diag_set-in-vdbe

Mergen Imeev (3):
  sql: remove error SQL_INTERRUPT
  sql: remove error SQL_MISMATCH
  sql: set errors in VDBE using diag_set()

 src/box/execute.c             |  23 +--
 src/box/sql/main.c            |   2 -
 src/box/sql/malloc.c          |   4 -
 src/box/sql/sqlInt.h          |  13 --
 src/box/sql/tokenize.c        |   3 -
 src/box/sql/vdbe.c            | 419 +++++++++++++++---------------------------
 src/box/sql/vdbeInt.h         |  10 -
 src/box/sql/vdbeapi.c         |  41 +----
 src/box/sql/vdbeaux.c         |  61 ++----
 test/sql-tap/autoinc.test.lua |   2 +-
 10 files changed, 171 insertions(+), 407 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] [PATCH v1 1/3] sql: remove error SQL_INTERRUPT
  2019-04-26 12:23 [tarantool-patches] [PATCH v1 0/3] sql: set errors in VDBE using diag_set() imeevma
@ 2019-04-26 12:23 ` imeevma
  2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 2/3] sql: remove error SQL_MISMATCH imeevma
  2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 3/3] sql: set errors in VDBE using diag_set() imeevma
  2 siblings, 0 replies; 4+ messages in thread
From: imeevma @ 2019-04-26 12:23 UTC (permalink / raw)
  To: korablev; +Cc: tarantool-patches

Since the interrupt system is no longer used in SQL, the
SQL_INTERRUPT error is out of date and should be removed. Also
this patch removes currently unused progress callback system.

Part of #4074
---
 src/box/sql/main.c     |  1 -
 src/box/sql/malloc.c   |  4 ---
 src/box/sql/sqlInt.h   | 11 -------
 src/box/sql/tokenize.c |  3 --
 src/box/sql/vdbe.c     | 84 ++------------------------------------------------
 src/box/sql/vdbeapi.c  |  7 -----
 src/box/sql/vdbeaux.c  | 41 ++++++++++--------------
 7 files changed, 19 insertions(+), 132 deletions(-)

diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index ed07553..0c6ad47 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -371,7 +371,6 @@ sqlErrStr(int rc)
 		/* SQL_BUSY        */ "database is locked",
 		/* SQL_LOCKED      */ "database table is locked",
 		/* SQL_NOMEM       */ "out of memory",
-		/* SQL_INTERRUPT   */ "interrupted",
 		/* SQL_IOERR       */ "disk I/O error",
 		/* SQL_NOTFOUND    */ "unknown operation",
 		/* SQL_FULL        */ "database or disk is full",
diff --git a/src/box/sql/malloc.c b/src/box/sql/malloc.c
index d6f99b4..dbc6846 100644
--- a/src/box/sql/malloc.c
+++ b/src/box/sql/malloc.c
@@ -898,9 +898,6 @@ sqlOomFault(sql * db)
 {
 	if (db->mallocFailed == 0 && db->bBenignMalloc == 0) {
 		db->mallocFailed = 1;
-		if (db->nVdbeExec > 0) {
-			db->u1.isInterrupted = 1;
-		}
 		db->lookaside.bDisable++;
 	}
 }
@@ -917,7 +914,6 @@ sqlOomClear(sql * db)
 {
 	if (db->mallocFailed && db->nVdbeExec == 0) {
 		db->mallocFailed = 0;
-		db->u1.isInterrupted = 0;
 		assert(db->lookaside.bDisable > 0);
 		db->lookaside.bDisable--;
 	}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 311adc9..88d1cc5 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -370,8 +370,6 @@ enum sql_ret_code {
 	SQL_LOCKED,
 	/** A malloc() failed. */
 	SQL_NOMEM,
-	/** Operation terminated by sql_interrupt(). */
-	SQL_INTERRUPT,
 	/** Some kind of disk I/O error occurred. */
 	SQL_IOERR,
 	/** Unknown opcode in sql_file_control(). */
@@ -1457,16 +1455,7 @@ struct sql {
 	void (*xUpdateCallback) (void *, int, const char *, const char *,
 				 sql_int64);
 	sql_value *pErr;	/* Most recent error message */
-	union {
-		volatile int isInterrupted;	/* True if sql_interrupt has been called */
-		double notUsed1;	/* Spacer */
-	} u1;
 	Lookaside lookaside;	/* Lookaside malloc configuration */
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	int (*xProgress) (void *);	/* The progress callback */
-	void *pProgressArg;	/* Argument to the progress callback */
-	unsigned nProgressOps;	/* Number of opcodes for progress callback */
-#endif
 	Hash aFunc;		/* Hash table of connection functions */
 	int *pnBytesFreed;	/* If not NULL, increment this in DbFree() */
 };
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 8cc3532..5cb4b7a 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -457,9 +457,6 @@ sqlRunParser(Parse * pParse, const char *zSql)
 
 	assert(zSql != 0);
 	mxSqlLen = db->aLimit[SQL_LIMIT_SQL_LENGTH];
-	if (db->nVdbeActive == 0) {
-		db->u1.isInterrupted = 0;
-	}
 	pParse->zTail = zSql;
 	i = 0;
 	/* sqlParserTrace(stdout, "parser: "); */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 85cec85..c2eec93 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -92,18 +92,6 @@ int sql_xfer_count = 0;
 #endif
 
 /*
- * When this global variable is positive, it gets decremented once before
- * each instruction in the VDBE.  When it reaches zero, the u1.isInterrupted
- * field of the sql structure is set in order to simulate an interrupt.
- *
- * This facility is used for testing purposes only.  It does not function
- * in an ordinary build.
- */
-#ifdef SQL_TEST
-int sql_interrupt_count = 0;
-#endif
-
-/*
  * The next global variable is incremented each type the OP_Sort opcode
  * is executed.  The test procedures use this information to make sure that
  * sorting is occurring or not occurring at appropriate times.   This variable
@@ -631,9 +619,6 @@ int sqlVdbeExec(Vdbe *p)
 	sql *db = p->db;       /* The database */
 	int iCompare = 0;          /* Result of last comparison */
 	unsigned nVmStep = 0;      /* Number of virtual machine steps */
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	unsigned nProgressLimit = 0;/* Invoke xProgress() when nVmStep reaches this */
-#endif
 	Mem *aMem = p->aMem;       /* Copy of p->aMem */
 	Mem *pIn1 = 0;             /* 1st input operand */
 	Mem *pIn2 = 0;             /* 2nd input operand */
@@ -658,15 +643,7 @@ int sqlVdbeExec(Vdbe *p)
 	p->iCurrentTime = 0;
 	assert(p->explain==0);
 	p->pResultSet = 0;
-	if (db->u1.isInterrupted) goto abort_due_to_interrupt;
 	sqlVdbeIOTraceSql(p);
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	if (db->xProgress) {
-		u32 iPrior = p->aCounter[SQL_STMTSTATUS_VM_STEP];
-		assert(0 < db->nProgressOps);
-		nProgressLimit = db->nProgressOps - (iPrior % db->nProgressOps);
-	}
-#endif
 #ifdef SQL_DEBUG
 	sqlBeginBenignMalloc();
 	if (p->pc==0
@@ -810,40 +787,7 @@ int sqlVdbeExec(Vdbe *p)
  * to the current line should be indented for EXPLAIN output.
  */
 case OP_Goto: {             /* jump */
-			jump_to_p2_and_check_for_interrupt:
-	pOp = &aOp[pOp->p2 - 1];
-
-	/* Opcodes that are used as the bottom of a loop (OP_Next, OP_Prev,
-	 * OP_RowSetNext, or OP_SorterNext) all jump here upon
-	 * completion.  Check to see if sql_interrupt() has been called
-	 * or if the progress callback needs to be invoked.
-	 *
-	 * This code uses unstructured "goto" statements and does not look clean.
-	 * But that is not due to sloppy coding habits. The code is written this
-	 * way for performance, to avoid having to run the interrupt and progress
-	 * checks on every opcode.  This helps sql_step() to run about 1.5%
-	 * faster according to "valgrind --tool=cachegrind"
-	 */
-			check_for_interrupt:
-	if (db->u1.isInterrupted) goto abort_due_to_interrupt;
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	/* Call the progress callback if it is configured and the required number
-	 * of VDBE ops have been executed (either since this invocation of
-	 * sqlVdbeExec() or since last time the progress callback was called).
-	 * If the progress callback returns non-zero, exit the virtual machine with
-	 * a return code SQL_ABORT.
-	 */
-	if (db->xProgress!=0 && nVmStep>=nProgressLimit) {
-		assert(db->nProgressOps!=0);
-		nProgressLimit = nVmStep + db->nProgressOps - (nVmStep%db->nProgressOps);
-		if (db->xProgress(db->pProgressArg)) {
-			rc = SQL_INTERRUPT;
-			goto abort_due_to_error;
-		}
-	}
-#endif
-
-	break;
+	goto jump_to_p2;
 }
 
 /* Opcode:  Gosub P1 P2 * * *
@@ -1387,18 +1331,6 @@ case OP_ResultRow: {
 	assert(pOp->p1>0);
 	assert(pOp->p1+pOp->p2<=(p->nMem+1 - p->nCursor)+1);
 
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	/* Run the progress counter just before returning.
-	 */
-	if (db->xProgress!=0
-	    && nVmStep>=nProgressLimit
-	    && db->xProgress(db->pProgressArg)!=0
-		) {
-		rc = SQL_INTERRUPT;
-		goto abort_due_to_error;
-	}
-#endif
-
 	/* If this statement has violated immediate foreign key constraints, do
 	 * not return the number of rows modified. And do not RELEASE the statement
 	 * transaction. It needs to be rolled back.
@@ -4340,11 +4272,11 @@ case OP_Next:          /* jump */
 #ifdef SQL_TEST
 		sql_search_count++;
 #endif
-		goto jump_to_p2_and_check_for_interrupt;
+		goto jump_to_p2;
 	} else {
 		pC->nullRow = 1;
 	}
-	goto check_for_interrupt;
+	break;
 }
 
 /* Opcode: SorterInsert P1 P2 * * *
@@ -5491,14 +5423,4 @@ no_mem:
 	sqlVdbeError(p, "out of memory");
 	rc = SQL_NOMEM;
 	goto abort_due_to_error;
-
-	/* Jump to here if the sql_interrupt() API sets the interrupt
-	 * flag.
-	 */
-abort_due_to_interrupt:
-	assert(db->u1.isInterrupted);
-	rc = db->mallocFailed ? SQL_NOMEM : SQL_INTERRUPT;
-	p->rc = rc;
-	sqlVdbeError(p, "%s", sqlErrStr(rc));
-	goto abort_due_to_error;
 }
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 4175f7e..a8037f7 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -537,13 +537,6 @@ sqlStep(Vdbe * p)
 		goto end_of_step;
 	}
 	if (p->pc < 0) {
-		/* If there are no other statements currently running, then
-		 * reset the interrupt flag.  This prevents a call to sql_interrupt
-		 * from interrupting a statement that has not yet started.
-		 */
-		if (db->nVdbeActive == 0) {
-			db->u1.isInterrupted = 0;
-		}
 
 #ifndef SQL_OMIT_TRACE
 		if ((db->xProfile || (db->mTrace & SQL_TRACE_PROFILE) != 0)
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 140bb97..27fa5b2 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -1486,10 +1486,6 @@ sqlVdbeList(Vdbe * p)
 	if (i >= nRow) {
 		p->rc = SQL_OK;
 		rc = SQL_DONE;
-	} else if (db->u1.isInterrupted) {
-		p->rc = SQL_INTERRUPT;
-		rc = SQL_ERROR;
-		sqlVdbeError(p, sqlErrStr(p->rc));
 	} else {
 		char *zP4;
 		Op *pOp;
@@ -2208,7 +2204,6 @@ sqlVdbeHalt(Vdbe * p)
 	 *     SQL_NOMEM
 	 *     SQL_IOERR
 	 *     SQL_FULL
-	 *     SQL_INTERRUPT
 	 *
 	 * Then the internal cache might have been left in an inconsistent
 	 * state.  We need to rollback the statement transaction, if there is
@@ -2234,13 +2229,11 @@ sqlVdbeHalt(Vdbe * p)
 
 		/* Check for one of the special errors */
 		mrc = p->rc & 0xff;
-		isSpecialError = mrc == SQL_NOMEM || mrc == SQL_IOERR
-		    || mrc == SQL_INTERRUPT || mrc == SQL_FULL;
+		isSpecialError = mrc == SQL_NOMEM || mrc == SQL_IOERR ||
+				 mrc == SQL_FULL;
 		if (isSpecialError) {
-			/* If the query was read-only and the error code is SQL_INTERRUPT,
-			 * no rollback is necessary. Otherwise, at least a savepoint
-			 * transaction must be rolled back to restore the database to a
-			 * consistent state.
+			/* At least a savepoint transaction must be rolled back
+			 * to restore the database to a consistent state.
 			 *
 			 * Even if the statement is read-only, it is important to perform
 			 * a statement or transaction rollback operation. If the error
@@ -2249,20 +2242,18 @@ sqlVdbeHalt(Vdbe * p)
 			 * pagerStress() in pager.c), the rollback is required to restore
 			 * the pager to a consistent state.
 			 */
-			if (mrc != SQL_INTERRUPT) {
-				if ((mrc == SQL_NOMEM || mrc == SQL_FULL)
-				    && box_txn()) {
-					eStatementOp = SAVEPOINT_ROLLBACK;
-				} else {
-					/* We are forced to roll back the active transaction. Before doing
-					 * so, abort any other statements this handle currently has active.
-					 */
-					box_txn_rollback();
-					closeCursorsAndFree(p);
-					sqlRollbackAll(p);
-					sqlCloseSavepoints(p);
-					p->nChange = 0;
-				}
+			if ((mrc == SQL_NOMEM || mrc == SQL_FULL)
+			    && box_txn()) {
+				eStatementOp = SAVEPOINT_ROLLBACK;
+			} else {
+				/* We are forced to roll back the active transaction. Before doing
+				 * so, abort any other statements this handle currently has active.
+				 */
+				box_txn_rollback();
+				closeCursorsAndFree(p);
+				sqlRollbackAll(p);
+				sqlCloseSavepoints(p);
+				p->nChange = 0;
 			}
 		}
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] [PATCH v1 2/3] sql: remove error SQL_MISMATCH
  2019-04-26 12:23 [tarantool-patches] [PATCH v1 0/3] sql: set errors in VDBE using diag_set() imeevma
  2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 1/3] sql: remove error SQL_INTERRUPT imeevma
@ 2019-04-26 12:23 ` imeevma
  2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 3/3] sql: set errors in VDBE using diag_set() imeevma
  2 siblings, 0 replies; 4+ messages in thread
From: imeevma @ 2019-04-26 12:23 UTC (permalink / raw)
  To: korablev; +Cc: tarantool-patches

This patch replaces SQL error SQL_MISMATCH by Tarantool error
ER_SQL_TYPE_MISMATCH.

Part of #4074
---
 src/box/sql/main.c            | 1 -
 src/box/sql/sqlInt.h          | 2 --
 src/box/sql/vdbe.c            | 6 ++++--
 test/sql-tap/autoinc.test.lua | 2 +-
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 0c6ad47..a3c6aa1 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -378,7 +378,6 @@ sqlErrStr(int rc)
 		/* SQL_SCHEMA      */ "database schema has changed",
 		/* SQL_TOOBIG      */ "string or blob too big",
 		/* SQL_CONSTRAINT  */ "constraint failed",
-		/* SQL_MISMATCH    */ "datatype mismatch",
 		/* SQL_MISUSE      */
 		    "library routine called out of sequence",
 		/* SQL_RANGE       */ "bind or column index out of range",
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 88d1cc5..655ea62 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -384,8 +384,6 @@ enum sql_ret_code {
 	SQL_TOOBIG,
 	/** Abort due to constraint violation. */
 	SQL_CONSTRAINT,
-	/** Data type mismatch. */
-	SQL_MISMATCH,
 	/** Library used incorrectly. */
 	SQL_MISUSE,
 	/** 2nd parameter to sql_bind out of range. */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index c2eec93..bdf7429 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1853,7 +1853,7 @@ case OP_AddImm: {            /* in1 */
  * Force the value in register P1 to be an integer.  If the value
  * in P1 is not an integer and cannot be converted into an integer
  * without data loss, then jump immediately to P2, or if P2==0
- * raise an SQL_MISMATCH exception.
+ * raise an ER_SQL_TYPE_MISMATCH error.
  */
 case OP_MustBeInt: {            /* jump, in1 */
 	pIn1 = &aMem[pOp->p1];
@@ -1862,7 +1862,9 @@ case OP_MustBeInt: {            /* jump, in1 */
 		VdbeBranchTaken((pIn1->flags&MEM_Int)==0, 2);
 		if ((pIn1->flags & MEM_Int)==0) {
 			if (pOp->p2==0) {
-				rc = SQL_MISMATCH;
+				diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+					 sql_value_text(pIn1), "integer");
+				rc = SQL_TARANTOOL_ERROR;
 				goto abort_due_to_error;
 			} else {
 				goto jump_to_p2;
diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua
index 257621b..3e3eaad 100755
--- a/test/sql-tap/autoinc.test.lua
+++ b/test/sql-tap/autoinc.test.lua
@@ -810,7 +810,7 @@ test:do_catchsql_test(
         INSERT INTO t1 SELECT s2, s2 FROM t1;
     ]], {
         -- <autoinc-gh-3670>
-        1, "Failed to execute SQL statement: datatype mismatch"
+        1, "Type mismatch: can not convert a to integer"
         -- </autoinc-gh-3670>
     })
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] [PATCH v1 3/3] sql: set errors in VDBE using diag_set()
  2019-04-26 12:23 [tarantool-patches] [PATCH v1 0/3] sql: set errors in VDBE using diag_set() imeevma
  2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 1/3] sql: remove error SQL_INTERRUPT imeevma
  2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 2/3] sql: remove error SQL_MISMATCH imeevma
@ 2019-04-26 12:23 ` imeevma
  2 siblings, 0 replies; 4+ messages in thread
From: imeevma @ 2019-04-26 12:23 UTC (permalink / raw)
  To: korablev; +Cc: tarantool-patches

After this patch, all errors in VDBE will be set using diag_set().

Part of #4074
---
 src/box/execute.c     |  23 +---
 src/box/sql/vdbe.c    | 331 +++++++++++++++++++++-----------------------------
 src/box/sql/vdbeInt.h |  10 --
 src/box/sql/vdbeapi.c |  34 +-----
 src/box/sql/vdbeaux.c |  20 +--
 5 files changed, 148 insertions(+), 270 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index a3d4a92..e81cc32 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -410,8 +410,7 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
  * @retval -1 Error.
  */
 static inline int
-sql_execute(sql *db, struct sql_stmt *stmt, struct port *port,
-	    struct region *region)
+sql_execute(struct sql_stmt *stmt, struct port *port, struct region *region)
 {
 	int rc, column_count = sql_column_count(stmt);
 	if (column_count > 0) {
@@ -427,15 +426,8 @@ sql_execute(sql *db, struct sql_stmt *stmt, struct port *port,
 		rc = sql_step(stmt);
 		assert(rc != SQL_ROW && rc != SQL_OK);
 	}
-	if (rc != SQL_DONE) {
-		if (db->errCode != SQL_TARANTOOL_ERROR) {
-			const char *err = (char *)sql_value_text(db->pErr);
-			if (err == NULL)
-				err = sqlErrStr(db->errCode);
-			diag_set(ClientError, ER_SQL_EXECUTE, err);
-		}
+	if (rc != SQL_DONE)
 		return -1;
-	}
 	return 0;
 }
 
@@ -446,19 +438,12 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 {
 	struct sql_stmt *stmt;
 	struct sql *db = sql_get();
-	if (sql_prepare_v2(db, sql, len, &stmt, NULL) != SQL_OK) {
-		if (db->errCode != SQL_TARANTOOL_ERROR) {
-			const char *err = (char *)sql_value_text(db->pErr);
-			if (err == NULL)
-				err = sqlErrStr(db->errCode);
-			diag_set(ClientError, ER_SQL_EXECUTE, err);
-		}
+	if (sql_prepare_v2(db, sql, len, &stmt, NULL) != SQL_OK)
 		return -1;
-	}
 	assert(stmt != NULL);
 	port_sql_create(port, stmt);
 	if (sql_bind(stmt, bind, bind_count) == 0 &&
-	    sql_execute(db, stmt, port, region) == 0)
+	    sql_execute(stmt, port, region) == 0)
 		return 0;
 	port_destroy(port);
 	return -1;
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index bdf7429..115f22e 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -986,7 +986,7 @@ case OP_Halt: {
 		p->rc = SQL_BUSY;
 	} else {
 		assert(rc==SQL_OK || (p->rc&0xff)==SQL_CONSTRAINT);
-		rc = p->rc ? SQL_ERROR : SQL_DONE;
+		rc = p->rc ? SQL_TARANTOOL_ERROR : SQL_DONE;
 	}
 	goto vdbe_return;
 }
@@ -1098,17 +1098,13 @@ case OP_NextAutoincValue: {
 	assert(pOp->p2 > 0);
 
 	struct space *space = space_by_id(pOp->p1);
-	if (space == NULL) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (space == NULL)
 		goto abort_due_to_error;
-	}
 
 	int64_t value;
 	struct sequence *sequence = space->sequence;
-	if (sequence == NULL || sequence_next(sequence, &value) != 0) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (sequence == NULL || sequence_next(sequence, &value) != 0)
 		goto abort_due_to_error;
-	}
 
 	pOut = out2Prerelease(p, pOp);
 	pOut->flags = MEM_Int;
@@ -1335,7 +1331,8 @@ case OP_ResultRow: {
 	 * not return the number of rows modified. And do not RELEASE the statement
 	 * transaction. It needs to be rolled back.
 	 */
-	if (SQL_OK!=(rc = sqlVdbeCheckFk(p, 0))) {
+	rc = sqlVdbeCheckFk(p, 0);
+	if (rc != SQL_OK) {
 		assert(user_session->sql_flags&SQL_CountRows);
 		goto abort_due_to_error;
 	}
@@ -1427,7 +1424,6 @@ case OP_Concat: {           /* same as TK_CONCAT, in1, in2, out3 */
 					  mem_type_to_str(pIn2);
 		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT or BLOB",
 			 inconsistent_type);
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 
@@ -1435,10 +1431,10 @@ case OP_Concat: {           /* same as TK_CONCAT, in1, in2, out3 */
 	if (str_type_p1 != str_type_p2) {
 		diag_set(ClientError, ER_INCONSISTENT_TYPES,
 			 mem_type_to_str(pIn2), mem_type_to_str(pIn1));
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
-	if (ExpandBlob(pIn1) || ExpandBlob(pIn2)) goto no_mem;
+	if (ExpandBlob(pIn1) != SQL_OK || ExpandBlob(pIn2) != SQL_OK)
+		goto abort_due_to_error;
 	nByte = pIn1->n + pIn2->n;
 	if (nByte>db->aLimit[SQL_LIMIT_LENGTH]) {
 		goto too_big;
@@ -1551,13 +1547,11 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		if (sqlVdbeRealValue(pIn1, &rA) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn1), "numeric");
-			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
 		if (sqlVdbeRealValue(pIn2, &rB) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn2), "numeric");
-			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
 		switch( pOp->opcode) {
@@ -1597,11 +1591,9 @@ arithmetic_result_is_null:
 
 division_by_zero:
 	diag_set(ClientError, ER_SQL_EXECUTE, "division by zero");
-	rc = SQL_TARANTOOL_ERROR;
 	goto abort_due_to_error;
 integer_overflow:
 	diag_set(ClientError, ER_SQL_EXECUTE, "integer is overflowed");
-	rc = SQL_TARANTOOL_ERROR;
 	goto abort_due_to_error;
 }
 
@@ -1723,11 +1715,15 @@ case OP_Function: {
 	/* If the function returned an error, throw an exception */
 	if (pCtx->fErrorOrAux) {
 		if (pCtx->isError) {
-			sqlVdbeError(p, "%s", sql_value_text(pCtx->pOut));
-			rc = pCtx->isError;
+			if (pCtx->isError != SQL_TARANTOOL_ERROR) {
+				diag_set(ClientError, ER_SQL_EXECUTE,
+					 sql_value_text(pCtx->pOut));
+			}
+			rc = SQL_TARANTOOL_ERROR;
 		}
 		sqlVdbeDeleteAuxData(db, &p->pAuxData, pCtx->iOp, pOp->p1);
-		if (rc) goto abort_due_to_error;
+		if (rc != SQL_OK)
+			goto abort_due_to_error;
 	}
 
 	/* Copy the result of the function into register P3 */
@@ -1789,13 +1785,11 @@ case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
 	if (sqlVdbeIntValue(pIn2, (int64_t *) &iA) != 0) {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 			 sql_value_text(pIn2), "integer");
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	if (sqlVdbeIntValue(pIn1, (int64_t *) &iB) != 0) {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 			 sql_value_text(pIn1), "integer");
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	op = pOp->opcode;
@@ -1864,7 +1858,6 @@ case OP_MustBeInt: {            /* jump, in1 */
 			if (pOp->p2==0) {
 				diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 					 sql_value_text(pIn1), "integer");
-				rc = SQL_TARANTOOL_ERROR;
 				goto abort_due_to_error;
 			} else {
 				goto jump_to_p2;
@@ -1910,8 +1903,7 @@ case OP_Realify: {                  /* in1 */
  */
 case OP_Cast: {                  /* in1 */
 	pIn1 = &aMem[pOp->p1];
-	rc = ExpandBlob(pIn1);
-	if (rc != 0)
+	if (ExpandBlob(pIn1) != SQL_OK)
 		goto abort_due_to_error;
 	rc = sqlVdbeMemCast(pIn1, pOp->p2);
 	UPDATE_MAX_BLOBSIZE(pIn1);
@@ -1919,7 +1911,6 @@ case OP_Cast: {                  /* in1 */
 		break;
 	diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_text(pIn1),
 		 field_type_strs[pOp->p2]);
-	rc = SQL_TARANTOOL_ERROR;
 	goto abort_due_to_error;
 }
 #endif /* SQL_OMIT_CAST */
@@ -2072,7 +2063,6 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 						  mem_type_to_str(pIn3);
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 inconsistent_type, "boolean");
-			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
 		res = sqlMemCompare(pIn3, pIn1, NULL);
@@ -2091,7 +2081,6 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 							 ER_SQL_TYPE_MISMATCH,
 							 sql_value_text(pIn3),
 							 "numeric");
-						rc = SQL_TARANTOOL_ERROR;
 						goto abort_due_to_error;
 					}
 
@@ -2333,7 +2322,6 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 	} else {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 			 sql_value_text(pIn1), "boolean");
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	pIn2 = &aMem[pOp->p2];
@@ -2344,7 +2332,6 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 	} else {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 			 sql_value_text(pIn2), "boolean");
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	if (pOp->opcode==OP_And) {
@@ -2378,7 +2365,6 @@ case OP_Not: {                /* same as TK_NOT, in1, out2 */
 		if ((pIn1->flags & MEM_Bool) == 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn1), "boolean");
-			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
 		mem_set_bool(pOut, ! pIn1->u.b);
@@ -2402,7 +2388,6 @@ case OP_BitNot: {             /* same as TK_BITNOT, in1, out2 */
 		if (sqlVdbeIntValue(pIn1, &i) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn1), "integer");
-			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
 		pOut->flags = MEM_Int;
@@ -2450,7 +2435,6 @@ case OP_IfNot: {            /* jump, in1 */
 	} else {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 			 sql_value_text(pIn1), "boolean");
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	VdbeBranchTaken(c!=0, 2);
@@ -2590,8 +2574,9 @@ case OP_Column: {
 		zEnd = zData + pC->payloadSize;
 	} else {
 		memset(&sMem, 0, sizeof(sMem));
-		rc = sqlVdbeMemFromBtree(pC->uc.pCursor, 0, pC->payloadSize, &sMem);
-		if (rc!=SQL_OK) goto abort_due_to_error;
+		if (sqlVdbeMemFromBtree(pC->uc.pCursor, 0, pC->payloadSize,
+					&sMem) != SQL_OK)
+			goto abort_due_to_error;
 		zData = (u8*)sMem.z;
 		zEnd = zData + pC->payloadSize;
 	}
@@ -2650,10 +2635,8 @@ case OP_Column: {
 	}
 	uint32_t unused;
 	if (vdbe_decode_msgpack_into_mem((const char *)(zData + aOffset[p2]),
-					 pDest, &unused) != 0) {
-		rc = SQL_TARANTOOL_ERROR;
+					 pDest, &unused) != 0)
 		goto abort_due_to_error;
-	}
 	/* MsgPack map, array or extension (unsupported in sql).
 	 * Wrap it in a blob verbatim.
 	 */
@@ -2687,7 +2670,11 @@ case OP_Column: {
 	if ((pDest->flags & (MEM_Ephem | MEM_Str)) == (MEM_Ephem | MEM_Str)) {
 		int len = pDest->n;
 		if (pDest->szMalloc<len+1) {
-			if (sqlVdbeMemGrow(pDest, len+1, 1)) goto op_column_error;
+			if (sqlVdbeMemGrow(pDest, len + 1, 1)) {
+				if (zData != pC->aRow)
+					sqlVdbeMemRelease(&sMem);
+				goto abort_due_to_error;
+			}
 		} else {
 			pDest->z = memcpy(pDest->zMalloc, pDest->z, len);
 			pDest->flags &= ~MEM_Ephem;
@@ -2701,10 +2688,6 @@ case OP_Column: {
 	UPDATE_MAX_BLOBSIZE(pDest);
 	REGISTER_TRACE(pOp->p3, pDest);
 	break;
-
-			op_column_error:
-	if (zData!=pC->aRow) sqlVdbeMemRelease(&sMem);
-	goto abort_due_to_error;
 }
 
 /* Opcode: ApplyType P1 P2 * P4 *
@@ -2729,7 +2712,6 @@ case OP_ApplyType: {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn1),
 				 field_type_strs[type]);
-			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
 		pIn1++;
@@ -2802,10 +2784,8 @@ case OP_MakeRecord: {
 	uint32_t tuple_size;
 	char *tuple =
 		sql_vdbe_mem_encode_tuple(pData0, nField, &tuple_size, region);
-	if (tuple == NULL) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (tuple == NULL)
 		goto abort_due_to_error;
-	}
 	if ((int64_t)tuple_size > db->aLimit[SQL_LIMIT_LENGTH])
 		goto too_big;
 
@@ -2860,13 +2840,14 @@ case OP_Count: {         /* out2 */
 	assert(pCrsr);
 	nEntry = 0;  /* Not needed.  Only used to silence a warning. */
 	if (pCrsr->curFlags & BTCF_TaCursor) {
-		rc = tarantoolsqlCount(pCrsr, &nEntry);
+		if (tarantoolsqlCount(pCrsr, &nEntry) != SQL_OK)
+			goto abort_due_to_error;
 	} else if (pCrsr->curFlags & BTCF_TEphemCursor) {
-		rc = tarantoolsqlEphemeralCount(pCrsr, &nEntry);
+		if (tarantoolsqlEphemeralCount(pCrsr, &nEntry) != SQL_OK)
+			goto abort_due_to_error;
 	} else {
 		unreachable();
 	}
-	if (rc) goto abort_due_to_error;
 	pOut = out2Prerelease(p, pOp);
 	pOut->u.i = nEntry;
 	break;
@@ -2890,7 +2871,6 @@ case OP_Savepoint: {
 	if (psql_txn == NULL) {
 		assert(!box_txn());
 		diag_set(ClientError, ER_SAVEPOINT_NO_TRANSACTION);
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	p1 = pOp->p1;
@@ -2918,8 +2898,10 @@ case OP_Savepoint: {
 			pSavepoint = pSavepoint->pNext
 			);
 		if (!pSavepoint) {
-			sqlVdbeError(p, "no such savepoint: %s", zName);
-			rc = SQL_ERROR;
+			const char *err =
+				tt_sprintf("no such savepoint: %s", zName);
+			diag_set(ClientError, ER_SQL_EXECUTE, err);
+			goto abort_due_to_error;
 		} else {
 
 			/* Determine whether or not this is a transaction savepoint. If so,
@@ -2936,7 +2918,8 @@ case OP_Savepoint: {
 					p->rc = rc = SQL_BUSY;
 					goto vdbe_return;
 				}
-				rc = p->rc;
+				if (p->rc != SQL_OK)
+					goto abort_due_to_error;
 			} else {
 				if (p1==SAVEPOINT_ROLLBACK)
 					box_txn_rollback_to_savepoint(pSavepoint->tnt_savepoint);
@@ -2968,7 +2951,6 @@ case OP_Savepoint: {
 			}
 		}
 	}
-	if (rc) goto abort_due_to_error;
 
 	break;
 }
@@ -2991,7 +2973,6 @@ case OP_CheckViewReferences: {
 	if (space->def->view_ref_count > 0) {
 		diag_set(ClientError, ER_DROP_SPACE, space->def->name,
 			 "other views depend on this space");
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	break;
@@ -3004,10 +2985,8 @@ case OP_CheckViewReferences: {
  * Otherwise, raise an error with appropriate error message.
  */
 case OP_TransactionBegin: {
-	if (sql_txn_begin(p) != 0) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (sql_txn_begin(p) != 0)
 		goto abort_due_to_error;
-	}
 	p->auto_commit = false	;
 	break;
 }
@@ -3023,13 +3002,11 @@ case OP_TransactionBegin: {
 case OP_TransactionCommit: {
 	struct txn *txn = in_txn();
 	if (txn != NULL) {
-		if (txn_commit(txn) != 0) {
-			rc = SQL_TARANTOOL_ERROR;
+		if (txn_commit(txn) != 0)
 			goto abort_due_to_error;
-		}
 	} else {
-		sqlVdbeError(p, "cannot commit - no transaction is active");
-		rc = SQL_ERROR;
+		diag_set(ClientError, ER_SQL_EXECUTE, "cannot commit - no "\
+			 "transaction is active");
 		goto abort_due_to_error;
 	}
 	break;
@@ -3042,14 +3019,11 @@ case OP_TransactionCommit: {
  */
 case OP_TransactionRollback: {
 	if (box_txn()) {
-		if (box_txn_rollback() != 0) {
-			rc = SQL_TARANTOOL_ERROR;
+		if (box_txn_rollback() != 0)
 			goto abort_due_to_error;
-		}
 	} else {
-		sqlVdbeError(p, "cannot rollback - no "
-				    "transaction is active");
-		rc = SQL_ERROR;
+		diag_set(ClientError, ER_SQL_EXECUTE, "cannot rollback - no "\
+			 "transaction is active");
 		goto abort_due_to_error;
 	}
 	break;
@@ -3067,16 +3041,12 @@ case OP_TransactionRollback: {
  */
 case OP_TTransaction: {
 	if (!box_txn()) {
-		if (sql_txn_begin(p) != 0) {
-			rc = SQL_TARANTOOL_ERROR;
+		if (sql_txn_begin(p) != 0)
 			goto abort_due_to_error;
-		}
 	} else {
 		p->anonymous_savepoint = sql_savepoint(p, NULL);
-		if (p->anonymous_savepoint == NULL) {
-			rc = SQL_TARANTOOL_ERROR;
+		if (p->anonymous_savepoint == NULL)
 			goto abort_due_to_error;
-		}
 	}
 	break;
 }
@@ -3115,9 +3085,8 @@ case OP_IteratorOpen:
 	if (box_schema_version() != p->schema_ver &&
 	    (pOp->p5 & OPFLAG_SYSTEMSP) == 0) {
 		p->expired = 1;
-		rc = SQL_ERROR;
-		sqlVdbeError(p, "schema version has changed: " \
-				    "need to re-compile SQL statement");
+		diag_set(ClientError, ER_SQL_EXECUTE, "schema version has "\
+			 "changed: need to re-compile SQL statement");
 		goto abort_due_to_error;
 	}
 	struct space *space;
@@ -3126,10 +3095,8 @@ case OP_IteratorOpen:
 	else
 		space = aMem[pOp->p3].u.p;
 	assert(space != NULL);
-	if (access_check_space(space, PRIV_R) != 0) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (access_check_space(space, PRIV_R) != 0)
 		goto abort_due_to_error;
-	}
 
 	struct index *index = space_index(space, pOp->p2);
 	assert(index != NULL);
@@ -3152,8 +3119,6 @@ case OP_IteratorOpen:
 	cur->nullRow = 1;
 open_cursor_set_hints:
 	cur->uc.pCursor->hints = pOp->p5 & OPFLAG_SEEKEQ;
-	if (rc != 0)
-		goto abort_due_to_error;
 	break;
 }
 
@@ -3175,10 +3140,8 @@ case OP_OpenTEphemeral: {
 	struct space *space = sql_ephemeral_space_create(pOp->p2,
 							 pOp->p4.key_info);
 
-	if (space == NULL) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (space == NULL)
 		goto abort_due_to_error;
-	}
 	aMem[pOp->p1].u.p = space;
 	aMem[pOp->p1].flags = MEM_Ptr;
 	break;
@@ -3204,8 +3167,8 @@ case OP_SorterOpen: {
 	pCx = allocateCursor(p, pOp->p1, pOp->p2, CURTYPE_SORTER);
 	if (pCx==0) goto no_mem;
 	pCx->key_def = def;
-	rc = sqlVdbeSorterInit(db, pCx);
-	if (rc) goto abort_due_to_error;
+	if (sqlVdbeSorterInit(db, pCx) != SQL_OK)
+		goto abort_due_to_error;
 	break;
 }
 
@@ -3437,7 +3400,6 @@ case OP_SeekGT: {       /* jump, in3 */
 		} else {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn3), "integer");
-			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
 		iKey = i;
@@ -3518,10 +3480,8 @@ case OP_SeekGT: {       /* jump, in3 */
 #endif
 	r.eqSeen = 0;
 	r.opcode = oc;
-	rc = sqlCursorMovetoUnpacked(pC->uc.pCursor, &r, &res);
-	if (rc!=SQL_OK) {
+	if (sqlCursorMovetoUnpacked(pC->uc.pCursor, &r, &res) != SQL_OK)
 		goto abort_due_to_error;
-	}
 	if (eqOnly && r.eqSeen==0) {
 		assert(res!=0);
 		goto seek_not_found;
@@ -3533,8 +3493,8 @@ case OP_SeekGT: {       /* jump, in3 */
 	if (oc>=OP_SeekGE) {  assert(oc==OP_SeekGE || oc==OP_SeekGT);
 		if (res<0 || (res==0 && oc==OP_SeekGT)) {
 			res = 0;
-			rc = sqlCursorNext(pC->uc.pCursor, &res);
-			if (rc!=SQL_OK) goto abort_due_to_error;
+			if (sqlCursorNext(pC->uc.pCursor, &res) != SQL_OK)
+				goto abort_due_to_error;
 		} else {
 			res = 0;
 		}
@@ -3542,8 +3502,8 @@ case OP_SeekGT: {       /* jump, in3 */
 		assert(oc==OP_SeekLT || oc==OP_SeekLE);
 		if (res>0 || (res==0 && oc==OP_SeekLT)) {
 			res = 0;
-			rc = sqlCursorPrevious(pC->uc.pCursor, &res);
-			if (rc!=SQL_OK) goto abort_due_to_error;
+			if (sqlCursorPrevious(pC->uc.pCursor, &res) != SQL_OK)
+				goto abort_due_to_error;
 		} else {
 			/* res might be negative because the table is empty.  Check to
 			 * see if this is the case.
@@ -3685,10 +3645,11 @@ case OP_Found: {        /* jump, in3 */
 		}
 	}
 	rc = sqlCursorMovetoUnpacked(pC->uc.pCursor, pIdxKey, &res);
-	if (pFree) sqlDbFree(db, pFree);
-	if (rc!=SQL_OK) {
+	if (pFree)
+		sqlDbFree(db, pFree);
+	assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
+	if (rc != SQL_OK)
 		goto abort_due_to_error;
-	}
 	pC->seekResult = res;
 	alreadyExists = (res==0);
 	pC->nullRow = 1-alreadyExists;
@@ -3748,10 +3709,8 @@ case OP_NextIdEphemeral: {
 	struct space *space = (struct space*)p->aMem[pOp->p1].u.p;
 	assert(space->def->id == 0);
 	uint64_t rowid;
-	if (space->vtab->ephemeral_rowid_next(space, &rowid) != 0) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (space->vtab->ephemeral_rowid_next(space, &rowid) != 0)
 		goto abort_due_to_error;
-	}
 	/*
 	 * FIXME: since memory cell can comprise only 32-bit
 	 * integer, make sure it can fit in. This check should
@@ -3760,7 +3719,6 @@ case OP_NextIdEphemeral: {
 	 */
 	if (rowid > INT32_MAX) {
 		diag_set(ClientError, ER_ROWID_OVERFLOW);
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	pOut = &aMem[pOp->p2];
@@ -3860,7 +3818,9 @@ case OP_Delete: {
 	}
 	pC->cacheStatus = CACHE_STALE;
 	pC->seekResult = 0;
-	if (rc) goto abort_due_to_error;
+	assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
+	if (rc != SQL_OK)
+		goto abort_due_to_error;
 
 	if (opflags & OPFLAG_NCHANGE)
 		p->nChange++;
@@ -3907,9 +3867,12 @@ case OP_SorterCompare: {
 			pIn3 = &aMem[pOp->p3];
 			nKeyCol = pOp->p4.i;
 			res = 0;
-			rc = sqlVdbeSorterCompare(pC, pIn3, nKeyCol, &res);
+			if (sqlVdbeSorterCompare(pC, pIn3, nKeyCol, &res) != 0)
+				rc = SQL_TARANTOOL_ERROR;
 			VdbeBranchTaken(res!=0,2);
-			if (rc) goto abort_due_to_error;
+			assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
+			if (rc != SQL_OK)
+				goto abort_due_to_error;
 			if (res) goto jump_to_p2;
 			break;
 		};
@@ -3932,10 +3895,10 @@ case OP_SorterData: {
 	pOut = &aMem[pOp->p2];
 	pC = p->apCsr[pOp->p1];
 	assert(isSorter(pC));
-	rc = sqlVdbeSorterRowkey(pC, pOut);
-	assert(rc!=SQL_OK || (pOut->flags & MEM_Blob));
+	if (sqlVdbeSorterRowkey(pC, pOut) != SQL_OK)
+		goto abort_due_to_error;
+	assert(pOut->flags & MEM_Blob);
 	assert(pOp->p1>=0 && pOp->p1<p->nCursor);
-	if (rc) goto abort_due_to_error;
 	p->apCsr[pOp->p3]->cacheStatus = CACHE_STALE;
 	break;
 }
@@ -4002,11 +3965,9 @@ case OP_RowData: {
 	testcase( n==0);
 
 	sqlVdbeMemRelease(pOut);
-	rc = sql_vdbe_mem_alloc_region(pOut, n);
-	if (rc)
-		goto no_mem;
-	rc = sqlCursorPayload(pCrsr, 0, n, pOut->z);
-	if (rc) goto abort_due_to_error;
+	if (sql_vdbe_mem_alloc_region(pOut, n) != SQL_OK ||
+	    sqlCursorPayload(pCrsr, 0, n, pOut->z) != SQL_OK)
+		goto abort_due_to_error;
 	UPDATE_MAX_BLOBSIZE(pOut);
 	REGISTER_TRACE(pOp->p2, pOut);
 	break;
@@ -4072,7 +4033,9 @@ case OP_Last: {        /* jump */
 		rc = tarantoolsqlLast(pCrsr, &res);
 		pC->nullRow = (u8)res;
 		pC->cacheStatus = CACHE_STALE;
-		if (rc) goto abort_due_to_error;
+		assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
+		if (rc != SQL_OK)
+			goto abort_due_to_error;
 		if (pOp->p2>0) {
 			VdbeBranchTaken(res!=0,2);
 			if (res) goto jump_to_p2;
@@ -4141,15 +4104,18 @@ case OP_Rewind: {        /* jump */
 	pC->seekOp = OP_Rewind;
 #endif
 	if (isSorter(pC)) {
-		rc = sqlVdbeSorterRewind(pC, &res);
+		if (sqlVdbeSorterRewind(pC, &res) != SQL_OK)
+			goto abort_due_to_error;
 	} else {
 		assert(pC->eCurType==CURTYPE_TARANTOOL);
 		pCrsr = pC->uc.pCursor;
 		assert(pCrsr);
-		rc = tarantoolsqlFirst(pCrsr, &res);
+		if (tarantoolsqlFirst(pCrsr, &res) != SQL_OK)
+			rc = SQL_TARANTOOL_ERROR;
 		pC->cacheStatus = CACHE_STALE;
+		if (rc != SQL_OK)
+			goto abort_due_to_error;
 	}
-	if (rc) goto abort_due_to_error;
 	pC->nullRow = (u8)res;
 	assert(pOp->p2>0 && pOp->p2<p->nOp);
 	VdbeBranchTaken(res!=0,2);
@@ -4267,7 +4233,9 @@ case OP_Next:          /* jump */
 			next_tail:
 	pC->cacheStatus = CACHE_STALE;
 	VdbeBranchTaken(res==0,2);
-	if (rc) goto abort_due_to_error;
+	assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
+	if (rc != SQL_OK)
+		goto abort_due_to_error;
 	if (res==0) {
 		pC->nullRow = 0;
 		p->aCounter[pOp->p5]++;
@@ -4295,11 +4263,8 @@ case OP_SorterInsert: {      /* in2 */
 	assert(isSorter(cursor));
 	pIn2 = &aMem[pOp->p2];
 	assert((pIn2->flags & MEM_Blob) != 0);
-	rc = ExpandBlob(pIn2);
-	if (rc != 0)
-		goto abort_due_to_error;
-	rc = sqlVdbeSorterWrite(cursor, pIn2);
-	if (rc != 0)
+	if (ExpandBlob(pIn2) != SQL_OK ||
+	    sqlVdbeSorterWrite(cursor, pIn2) != SQL_OK)
 		goto abort_due_to_error;
 	break;
 }
@@ -4329,8 +4294,7 @@ case OP_IdxInsert: {
 	assert((pIn2->flags & MEM_Blob) != 0);
 	if (pOp->p5 & OPFLAG_NCHANGE)
 		p->nChange++;
-	rc = ExpandBlob(pIn2);
-	if (rc != 0)
+	if (ExpandBlob(pIn2) != SQL_OK)
 		goto abort_due_to_error;
 	struct space *space;
 	if (pOp->p4type == P4_SPACEPTR)
@@ -4364,6 +4328,7 @@ case OP_IdxInsert: {
 	} else if (pOp->p5 & OPFLAG_OE_ROLLBACK) {
 		p->errorAction = ON_CONFLICT_ACTION_ROLLBACK;
 	}
+	assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
 	if (rc != 0)
 		goto abort_due_to_error;
 	break;
@@ -4431,14 +4396,12 @@ case OP_Update: {
 	if (is_error) {
 		diag_set(OutOfMemory, stream.pos - stream.buf,
 			"mpstream_flush", "stream");
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	uint32_t ops_size = region_used(region) - used;
 	const char *ops = region_join(region, ops_size);
 	if (ops == NULL) {
 		diag_set(OutOfMemory, ops_size, "region_join", "raw");
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 
@@ -4464,6 +4427,7 @@ case OP_Update: {
 	} else if (pOp->p5 & OPFLAG_OE_ROLLBACK) {
 		p->errorAction = ON_CONFLICT_ACTION_ROLLBACK;
 	}
+	assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
 	if (rc != 0)
 		goto abort_due_to_error;
 	break;
@@ -4514,8 +4478,7 @@ case OP_SDelete: {
 	struct space *space = space_by_id(pOp->p1);
 	assert(space != NULL);
 	assert(space_is_system(space));
-	rc = sql_delete_by_key(space, 0, pIn2->z, pIn2->n);
-	if (rc)
+	if (sql_delete_by_key(space, 0, pIn2->z, pIn2->n) != SQL_OK)
 		goto abort_due_to_error;
 	if (pOp->p5 & OPFLAG_NCHANGE)
 		p->nChange++;
@@ -4549,18 +4512,19 @@ case OP_IdxDelete: {
 	r.default_rc = 0;
 	r.aMem = &aMem[pOp->p2];
 	r.opcode = OP_IdxDelete;
-	rc = sqlCursorMovetoUnpacked(pCrsr, &r, &res);
-	if (rc) goto abort_due_to_error;
+	if (sqlCursorMovetoUnpacked(pCrsr, &r, &res) != SQL_OK)
+		goto abort_due_to_error;
 	if (res==0) {
 		assert(pCrsr->eState == CURSOR_VALID);
 		if (pCrsr->curFlags & BTCF_TaCursor) {
-			rc = tarantoolsqlDelete(pCrsr, 0);
+			if (tarantoolsqlDelete(pCrsr, 0) != SQL_OK)
+				goto abort_due_to_error;
 		} else if (pCrsr->curFlags & BTCF_TEphemCursor) {
-			rc = tarantoolsqlEphemeralDelete(pCrsr);
+			if (tarantoolsqlEphemeralDelete(pCrsr) != SQL_OK)
+				goto abort_due_to_error;
 		} else {
 			unreachable();
 		}
-		if (rc) goto abort_due_to_error;
 	}
 	pC->cacheStatus = CACHE_STALE;
 	pC->seekResult = 0;
@@ -4672,14 +4636,14 @@ case OP_Clear: {
 	rc = 0;
 	if (pOp->p2 > 0) {
 		if (box_truncate(space_id) != 0)
-			rc = SQL_TARANTOOL_ERROR;
+			goto abort_due_to_error;
 	} else {
 		uint32_t tuple_count;
-		rc = tarantoolsqlClearTable(space, &tuple_count);
-		if (rc == 0 && (pOp->p5 & OPFLAG_NCHANGE) != 0)
+		if (tarantoolsqlClearTable(space, &tuple_count) != SQL_OK)
+			goto abort_due_to_error;
+		if ((pOp->p5 & OPFLAG_NCHANGE) != 0)
 			p->nChange += tuple_count;
 	}
-	if (rc) goto abort_due_to_error;
 	break;
 }
 
@@ -4702,8 +4666,8 @@ case OP_ResetSorter: {
 	} else {
 		assert(pC->eCurType==CURTYPE_TARANTOOL);
 		assert(pC->uc.pCursor->curFlags & BTCF_TEphemCursor);
-		rc = tarantoolsqlEphemeralClearTable(pC->uc.pCursor);
-		if (rc) goto abort_due_to_error;
+		if (tarantoolsqlEphemeralClearTable(pC->uc.pCursor) != SQL_OK)
+			goto abort_due_to_error;
 	}
 	break;
 }
@@ -4736,8 +4700,8 @@ case OP_RenameTable: {
 	zNewTableName = pOp->p4.z;
 	zOldTableName = sqlDbStrNDup(db, zOldTableName,
 					 sqlStrlen30(zOldTableName));
-	rc = sql_rename_table(space_id, zNewTableName);
-	if (rc) goto abort_due_to_error;
+	if (sql_rename_table(space_id, zNewTableName) != SQL_OK)
+		goto abort_due_to_error;
 	/*
 	 * Rebuild 'CREATE TRIGGER' expressions of all triggers
 	 * created on this table. Sure, this action is not atomic
@@ -4747,20 +4711,18 @@ case OP_RenameTable: {
 	for (struct sql_trigger *trigger = triggers; trigger != NULL; ) {
 		/* Store pointer as trigger will be destructed. */
 		struct sql_trigger *next_trigger = trigger->next;
-		rc = tarantoolsqlRenameTrigger(trigger->zName,
-						   zOldTableName, zNewTableName);
-		if (rc != SQL_OK) {
-			/*
-			 * FIXME: In the case of error,
-			 * part of triggers would have invalid
-			 * space name in tuple so can not been
-			 * persisted.
-			 * Server could be restarted.
-			 * In this case, rename table back and
-			 * try again.
-			 */
+		/*
+		 * FIXME: In the case of error,
+		 * part of triggers would have invalid
+		 * space name in tuple so can not been
+		 * persisted.
+		 * Server could be restarted.
+		 * In this case, rename table back and
+		 * try again.
+		 */
+		if (tarantoolsqlRenameTrigger(trigger->zName, zOldTableName,
+					      zNewTableName) != SQL_OK)
 			goto abort_due_to_error;
-		}
 		trigger = next_trigger;
 	}
 	sqlDbFree(db, (void*)zOldTableName);
@@ -4775,8 +4737,8 @@ case OP_RenameTable: {
  */
 case OP_LoadAnalysis: {
 	assert(pOp->p1==0 );
-	rc = sql_analysis_load(db);
-	if (rc) goto abort_due_to_error;
+	if (sql_analysis_load(db) != SQL_OK)
+		goto abort_due_to_error;
 	break;
 }
 
@@ -4832,8 +4794,8 @@ case OP_Program: {        /* jump */
 	}
 
 	if (p->nFrame>=db->aLimit[SQL_LIMIT_TRIGGER_DEPTH]) {
-		rc = SQL_ERROR;
-		sqlVdbeError(p, "too many levels of trigger recursion");
+		diag_set(ClientError, ER_SQL_EXECUTE, "too many levels of "\
+			 "trigger recursion");
 		goto abort_due_to_error;
 	}
 
@@ -5164,11 +5126,16 @@ case OP_AggStep: {
 	(pCtx->pFunc->xSFunc)(pCtx,pCtx->argc,pCtx->argv); /* IMP: R-24505-23230 */
 	if (pCtx->fErrorOrAux) {
 		if (pCtx->isError) {
-			sqlVdbeError(p, "%s", sql_value_text(&t));
-			rc = pCtx->isError;
+			if (pCtx->isError != SQL_TARANTOOL_ERROR) {
+				diag_set(ClientError, ER_SQL_EXECUTE,
+					 sql_value_text(&t));
+			}
+			rc = SQL_TARANTOOL_ERROR;
 		}
 		sqlVdbeMemRelease(&t);
-		if (rc) goto abort_due_to_error;
+		assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
+		if (rc != SQL_OK)
+			goto abort_due_to_error;
 	} else {
 		assert(t.flags==MEM_Null);
 	}
@@ -5199,8 +5166,11 @@ case OP_AggFinal: {
 	pMem = &aMem[pOp->p1];
 	assert((pMem->flags & ~(MEM_Null|MEM_Agg))==0);
 	rc = sqlVdbeMemFinalize(pMem, pOp->p4.pFunc);
-	if (rc) {
-		sqlVdbeError(p, "%s", sql_value_text(pMem));
+	if (rc != SQL_OK) {
+		if (rc != SQL_TARANTOOL_ERROR) {
+			diag_set(ClientError, ER_SQL_EXECUTE,
+				 sql_value_text(pMem));
+		}
 		goto abort_due_to_error;
 	}
 	UPDATE_MAX_BLOBSIZE(pMem);
@@ -5311,10 +5281,8 @@ case OP_IncMaxid: {
 	assert(pOp->p1 > 0);
 	pOut = &aMem[pOp->p1];
 
-	rc = tarantoolsqlIncrementMaxid((uint64_t*) &pOut->u.i);
-	if (rc!=SQL_OK) {
+	if (tarantoolsqlIncrementMaxid((uint64_t*) &pOut->u.i) != SQL_OK)
 		goto abort_due_to_error;
-	}
 	pOut->flags = MEM_Int;
 	break;
 }
@@ -5378,28 +5346,8 @@ default: {          /* This is really OP_Noop and OP_Explain */
 	 * an error of some kind.
 	 */
 abort_due_to_error:
-	if (db->mallocFailed) rc = SQL_NOMEM;
-	assert(rc);
-	if (p->zErrMsg==0 && rc!=SQL_IOERR_NOMEM) {
-		const char *msg;
-		/* Avoiding situation when Tarantool error is set,
-		 * but error message isn't.
-		 */
-		if (rc == SQL_TARANTOOL_ERROR && tarantoolErrorMessage()) {
-			msg = tarantoolErrorMessage();
-		} else {
-			msg = sqlErrStr(rc);
-		}
-		sqlVdbeError(p, "%s", msg);
-	}
+	rc = SQL_TARANTOOL_ERROR;
 	p->rc = rc;
-	sqlSystemError(db, rc);
-	testcase( sqlGlobalConfig.xLog!=0);
-	sql_log(rc, "statement aborts at %d: [%s] %s",
-		    (int)(pOp - aOp), p->zSql, p->zErrMsg);
-	sqlVdbeHalt(p);
-	if (rc==SQL_IOERR_NOMEM) sqlOomFault(db);
-	rc = SQL_ERROR;
 
 	/* This is the only way out of this procedure. */
 vdbe_return:
@@ -5408,21 +5356,20 @@ vdbe_return:
 	assert(rc!=SQL_OK || nExtraDelete==0
 		|| sql_strlike_ci("DELETE%", p->zSql, 0) != 0
 		);
+	assert(rc == SQL_OK || rc == SQL_BUSY || rc >= SQL_TARANTOOL_ERROR ||
+	       rc == SQL_ROW || rc == SQL_DONE);
 	return rc;
 
 	/* Jump to here if a string or blob larger than SQL_MAX_LENGTH
 	 * is encountered.
 	 */
 too_big:
-	sqlVdbeError(p, "string or blob too big");
-	rc = SQL_TOOBIG;
+	diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
 	goto abort_due_to_error;
 
 	/* Jump to here if a malloc() fails.
 	 */
 no_mem:
 	sqlOomFault(db);
-	sqlVdbeError(p, "out of memory");
-	rc = SQL_NOMEM;
 	goto abort_due_to_error;
 }
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index a3100e5..b655b5a 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -248,10 +248,6 @@ struct Mem {
 #define MEM_Agg       0x4000	/* Mem.z points to an agg function context */
 #define MEM_Zero      0x8000	/* Mem.i contains count of 0s appended to blob */
 #define MEM_Subtype   0x10000	/* Mem.eSubtype is valid */
-#ifdef SQL_OMIT_INCRBLOB
-#undef MEM_Zero
-#define MEM_Zero 0x0000
-#endif
 
 /**
  * In contrast to Mem_TypeMask, this one allows to get
@@ -467,7 +463,6 @@ struct Vdbe {
 /*
  * Function prototypes
  */
-void sqlVdbeError(Vdbe *, const char *, ...);
 void sqlVdbeFreeCursor(Vdbe *, VdbeCursor *);
 void sqlVdbePopStack(Vdbe *, int);
 int sqlVdbeCursorRestore(VdbeCursor *);
@@ -550,13 +545,8 @@ void sqlVdbeMemPrettyPrint(Mem * pMem, char *zBuf);
 #endif
 int sqlVdbeMemHandleBom(Mem * pMem);
 
-#ifndef SQL_OMIT_INCRBLOB
 int sqlVdbeMemExpandBlob(Mem *);
 #define ExpandBlob(P) (((P)->flags&MEM_Zero)?sqlVdbeMemExpandBlob(P):0)
-#else
-#define sqlVdbeMemExpandBlob(x) SQL_OK
-#define ExpandBlob(P) SQL_OK
-#endif
 
 /**
  * Perform comparison of two keys: one is packed and one is not.
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index a8037f7..0367bfd 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -569,15 +569,6 @@ sqlStep(Vdbe * p)
 		p->rc = SQL_NOMEM;
 	}
  end_of_step:
-	/* At this point local variable rc holds the value that should be
-	 * returned if this statement was compiled using the legacy
-	 * sql_prepare() interface. According to the docs, this can only
-	 * be one of the values in the first assert() below. Variable p->rc
-	 * contains the value that would be returned if sql_finalize()
-	 * were called on statement p.
-	 */
-	assert(rc == SQL_ROW || rc == SQL_DONE || rc == SQL_ERROR
-	       || (rc & 0xff) == SQL_BUSY || rc == SQL_MISUSE);
 	if (p->isPrepareV2 && rc != SQL_ROW && rc != SQL_DONE) {
 		/* If this statement was prepared using sql_prepare_v2(), and an
 		 * error has occurred, then return the error code in p->rc to the
@@ -597,20 +588,17 @@ int
 sql_step(sql_stmt * pStmt)
 {
 	int rc;			/* Result from sqlStep() */
-	int rc2 = SQL_OK;	/* Result from sqlReprepare() */
 	Vdbe *v = (Vdbe *) pStmt;	/* the prepared statement */
 	int cnt = 0;		/* Counter to prevent infinite loop of reprepares */
-	sql *db;		/* The database connection */
 
 	if (vdbeSafetyNotNull(v)) {
 		return SQL_MISUSE;
 	}
-	db = v->db;
 	v->doingRerun = 0;
 	while ((rc = sqlStep(v)) == SQL_SCHEMA
 	       && cnt++ < SQL_MAX_SCHEMA_RETRY) {
 		int savedPc = v->pc;
-		rc2 = rc = sqlReprepare(v);
+		rc = sqlReprepare(v);
 		if (rc != SQL_OK)
 			break;
 		sql_reset(pStmt);
@@ -618,26 +606,6 @@ sql_step(sql_stmt * pStmt)
 			v->doingRerun = 1;
 		assert(v->expired == 0);
 	}
-	if (rc2 != SQL_OK) {
-		/* This case occurs after failing to recompile an sql statement.
-		 * The error message from the SQL compiler has already been loaded
-		 * into the database handle. This block copies the error message
-		 * from the database handle into the statement and sets the statement
-		 * program counter to 0 to ensure that when the statement is
-		 * finalized or reset the parser error message is available via
-		 * sql_errmsg() and sql_errcode().
-		 */
-		const char *zErr = (const char *)sql_value_text(db->pErr);
-		sqlDbFree(db, v->zErrMsg);
-		if (!db->mallocFailed) {
-			v->zErrMsg = sqlDbStrDup(db, zErr);
-			v->rc = rc2;
-		} else {
-			v->zErrMsg = 0;
-			v->rc = rc = SQL_NOMEM;
-		}
-	}
-	rc = sqlApiExit(db, rc);
 	return rc;
 }
 
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 27fa5b2..48c2a81 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -116,19 +116,6 @@ sql_vdbe_prepare(struct Vdbe *vdbe)
 }
 
 /*
- * Change the error string stored in Vdbe.zErrMsg
- */
-void
-sqlVdbeError(Vdbe * p, const char *zFormat, ...)
-{
-	va_list ap;
-	sqlDbFree(p->db, p->zErrMsg);
-	va_start(ap, zFormat);
-	p->zErrMsg = sqlVMPrintf(p->db, zFormat, ap);
-	va_end(ap);
-}
-
-/*
  * Remember the SQL string for a prepared statement.
  */
 void
@@ -2124,10 +2111,11 @@ sqlVdbeCheckFk(Vdbe * p, int deferred)
 	if ((deferred && txn != NULL && txn->psql_txn != NULL &&
 	     txn->psql_txn->fk_deferred_count > 0) ||
 	    (!deferred && p->nFkConstraint > 0)) {
-		p->rc = SQL_CONSTRAINT_FOREIGNKEY;
+		p->rc = SQL_TARANTOOL_ERROR;
 		p->errorAction = ON_CONFLICT_ACTION_ABORT;
-		sqlVdbeError(p, "FOREIGN KEY constraint failed");
-		return SQL_ERROR;
+		diag_set(ClientError, ER_SQL_EXECUTE, "FOREIGN KEY constraint "\
+			 "failed");
+		return SQL_TARANTOOL_ERROR;
 	}
 	return SQL_OK;
 }
-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-04-26 12:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-26 12:23 [tarantool-patches] [PATCH v1 0/3] sql: set errors in VDBE using diag_set() imeevma
2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 1/3] sql: remove error SQL_INTERRUPT imeevma
2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 2/3] sql: remove error SQL_MISMATCH imeevma
2019-04-26 12:23 ` [tarantool-patches] [PATCH v1 3/3] sql: set errors in VDBE using diag_set() imeevma

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