From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tarantool-patches-bounce@freelists.org>
Received: from localhost (localhost [127.0.0.1])
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 11F292DEA8
	for <tarantool-patches@freelists.org>; Sat, 25 May 2019 06:36:04 -0400 (EDT)
Received: from turing.freelists.org ([127.0.0.1])
	by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id 3ElV3T_StUE8 for <tarantool-patches@freelists.org>;
	Sat, 25 May 2019 06:36:03 -0400 (EDT)
Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 1FC5E2DD43
	for <tarantool-patches@freelists.org>; Sat, 25 May 2019 06:36:03 -0400 (EDT)
Subject: [tarantool-patches] Re: [PATCH v1 07/12] sql: set errors in VDBE
 using diag_set()
From: Imeev Mergen <imeevma@tarantool.org>
References: <cover.1557056617.git.imeevma@gmail.com>
 <8b2d92d24172d1cd5934afa638b92232a93fba19.1557056617.git.imeevma@gmail.com>
 <8C6EC917-D1A6-4171-B203-97CA432CF8F3@tarantool.org>
 <20190525102431.GB15670@tarantool.org>
Message-ID: <a09dd97d-f456-23e0-101b-aebbb2f99677@tarantool.org>
Date: Sat, 25 May 2019 13:36:00 +0300
MIME-Version: 1.0
In-Reply-To: <20190525102431.GB15670@tarantool.org>
Content-Type: multipart/alternative;
 boundary="------------BBA6F7D93F42705AAF51EAFB"
Content-Language: en-US
Sender: tarantool-patches-bounce@freelists.org
Errors-to: tarantool-patches-bounce@freelists.org
Reply-To: tarantool-patches@freelists.org
List-Help: <mailto:ecartis@freelists.org?Subject=help>
List-Unsubscribe: <mailto:tarantool-patches-request@freelists.org?Subject=unsubscribe>
List-software: Ecartis version 1.0.0
List-Id: tarantool-patches <tarantool-patches.freelists.org>
List-Subscribe: <mailto:tarantool-patches-request@freelists.org?Subject=subscribe>
List-Owner: <mailto:>
List-post: <mailto:tarantool-patches@freelists.org>
List-Archive: <http://www.freelists.org/archives/tarantool-patches>
To: "n.pettik" <korablev@tarantool.org>
Cc: tarantool-patches@freelists.org

This is a multi-part message in MIME format.
--------------BBA6F7D93F42705AAF51EAFB
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Transfer-Encoding: 8bit

Fixed commit-message:

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

Closes  #4074  <https://github.com/tarantool/tarantool/issues/4074>

On 5/25/19 1:24 PM, Mergen Imeev wrote:
> I moved patch "sql: use diag_set() to set an error in SQL
> functions" to position before this patch. It allowed to
> simplify this patch. New patch below.
>
> On Wed, May 15, 2019 at 04:26:54PM +0300, n.pettik wrote:
>>
>>> On 5 May 2019, at 15:17, imeevma@tarantool.org wrote:
>>>
>>> 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(-)
>> Please, remove whole sqlErrStr(), tarantoolErrorMessage() -
>> they are unused now. The same concerns sql_ret_code() -
>> I see no reason keeping it. Please, remove all legacy routines
>> connected with error codes.
>>
> These functions are removed in the following patches.
>
>>> 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
>>>
>>> @@ -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) {
>> -> if (sqlVdbeCheckFk() != 0)
>>
> Fixed. I used SQL_OK but it will be replaced by 0 in the
> following patches.
>
>>> 		assert(user_session->sql_flags&SQL_CountRows);
>>> 		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;
>> != 0
>> Still need to call sqlOomFault(db); by jumping to no_mem label.
>>
> Fixed.
>
>>> 	nByte = pIn1->n + pIn2->n;
>>> 	if (nByte>db->aLimit[SQL_LIMIT_LENGTH]) {
>>> 		goto too_big;
>>>
>>>
>>> @@ -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) {
>> How it can be different from SQL_TARANTOOL_ERROR?
>>
> It was possible, but this part of code was completely
> changed after i moved patch "sql: use diag_set() to set an
> error in SQL functions" to position before this patch.
>
>>> +				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;
>> This diff seems to be redundant
>>
> Fixed.
>
>>> @@ -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)
>> != 0
>> Please, don’t use SQL_OK value anywhere.
>>
> I think it's better to use SQL_OK here for integrity. It is
> replaced with 0 in one of the following patches.
>
>>> @@ -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;
>>>
>>>
>>> @@ -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);
>> We already have ER_NO_SUCH_SAVEPOINT.
>>
> Fixed.
>
>>> @@ -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)
>> != NULL
>>
> Fixed.
>>> @@ -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;
>> Looks like redundant diff
>>
> Fixed.
>
>>> 	} 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));
>> Could you please clarify what does this error mean?
>> It would just print value of memory to string…
>> The same relates to error in OP_AggStep.
>>
> If result was returned using sql_result_error() then it was
> possible that diag is not set. This was fixed in patch that
> now right before this patch.
>
>>> @@ -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;
>>> }
>>>
>>> too_big:
>>> -	sqlVdbeError(p, "string or blob too big");
>>> -	rc = SQL_TOOBIG;
>>> +	diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big”);
>> -> is too big
>>
> Fixed here. Still, it is possible to find this error in
> other places. I think it will be fixed when we create error
> code for such errors.
>
>>> 	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
>>>
>>>
>>> @@ -550,13 +545,8 @@ void sqlVdbeMemPrettyPrint(Mem * pMem, char *zBuf);
>>> #endif
>>> int sqlVdbeMemHandleBom(Mem * pMem);
>>>
>>> -#ifndef SQL_OMIT_INCRBLOB
>> Still see usage of this macro in code: vdbemem.c : 213
>>
> Fixed.
>
>>> 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/vdbeaux.c b/src/box/sql/vdbeaux.c
>>> index 27fa5b2..48c2a81 100644
>>> --- a/src/box/sql/vdbeaux.c
>>> +++ b/src/box/sql/vdbeaux.c
>>> 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”);
>> Please, reserve separate error code for this violation.
>>
> I am going to do this a bit later, in a different patch.
>
>>> +		return SQL_TARANTOOL_ERROR;
>>> 	}
>>> 	return SQL_OK;
>> Return 0/-1
>>
>>
> Left as it as for now, will be fixed in following patches.
>
>
> New patch:
>
>  From 85cfbe96609b66379631c8d4534c8a9329fb3a47 Mon Sep 17 00:00:00 2001
> Date: Mon, 22 Apr 2019 19:41:46 +0300
> Subject: [PATCH] sql: set errors in VDBE using diag_set()
>
> After this patch, all errors in VDBE will be set using diag_set().
>
> Part of #4074
>
> 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 7d85959..b64293a 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,7 @@ 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))) {
> +	if (sqlVdbeCheckFk(p, 0) != SQL_OK) {
>   		assert(user_session->sql_flags&SQL_CountRows);
>   		goto abort_due_to_error;
>   	}
> @@ -1427,7 +1423,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 +1430,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 +1546,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 +1590,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;
>   }
>   
> @@ -1721,10 +1712,8 @@ case OP_Function: {
>   	(*pCtx->pFunc->xSFunc)(pCtx, pCtx->argc, pCtx->argv);/* IMP: R-24505-23230 */
>   
>   	/* If the function returned an error, throw an exception */
> -	if (pCtx->is_aborted) {
> -		rc = SQL_TARANTOOL_ERROR;
> +	if (pCtx->is_aborted)
>   		goto abort_due_to_error;
> -	}
>   
>   	/* Copy the result of the function into register P3 */
>   	if (pOut->flags & (MEM_Str|MEM_Blob)) {
> @@ -1785,13 +1774,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;
> @@ -1860,7 +1847,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;
> @@ -1906,8 +1892,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);
> @@ -1915,7 +1900,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 */
> @@ -2068,7 +2052,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);
> @@ -2087,7 +2070,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;
>   					}
>   
> @@ -2329,7 +2311,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];
> @@ -2340,7 +2321,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) {
> @@ -2374,7 +2354,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);
> @@ -2398,7 +2377,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;
> @@ -2446,7 +2424,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);
> @@ -2586,8 +2563,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;
>   	}
> @@ -2646,10 +2624,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.
>   	 */
> @@ -2683,7 +2659,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;
> @@ -2697,10 +2677,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 *
> @@ -2725,7 +2701,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++;
> @@ -2798,10 +2773,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;
>   
> @@ -2856,13 +2829,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;
> @@ -2886,7 +2860,6 @@ case OP_Savepoint: {
>   	if (psql_txn == NULL) {
>   		assert(!box_txn());
>   		diag_set(ClientError, ER_NO_TRANSACTION);
> -		rc = SQL_TARANTOOL_ERROR;
>   		goto abort_due_to_error;
>   	}
>   	p1 = pOp->p1;
> @@ -2914,8 +2887,8 @@ case OP_Savepoint: {
>   			pSavepoint = pSavepoint->pNext
>   			);
>   		if (!pSavepoint) {
> -			sqlVdbeError(p, "no such savepoint: %s", zName);
> -			rc = SQL_ERROR;
> +			diag_set(ClientError, ER_NO_SUCH_SAVEPOINT);
> +			goto abort_due_to_error;
>   		} else {
>   
>   			/* Determine whether or not this is a transaction savepoint. If so,
> @@ -2932,7 +2905,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);
> @@ -2964,7 +2938,6 @@ case OP_Savepoint: {
>   			}
>   		}
>   	}
> -	if (rc) goto abort_due_to_error;
>   
>   	break;
>   }
> @@ -2987,7 +2960,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;
> @@ -3000,10 +2972,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;
>   }
> @@ -3019,13 +2989,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;
> @@ -3038,14 +3006,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;
> @@ -3063,16 +3028,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;
>   }
> @@ -3111,9 +3072,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;
> @@ -3122,10 +3082,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);
> @@ -3148,8 +3106,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;
>   }
>   
> @@ -3171,10 +3127,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;
> @@ -3200,8 +3154,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;
>   }
>   
> @@ -3433,7 +3387,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;
> @@ -3514,10 +3467,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;
> @@ -3529,8 +3480,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;
>   		}
> @@ -3538,8 +3489,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.
> @@ -3681,10 +3632,11 @@ case OP_Found: {        /* jump, in3 */
>   		}
>   	}
>   	rc = sqlCursorMovetoUnpacked(pC->uc.pCursor, pIdxKey, &res);
> -	if (pFree) sqlDbFree(db, pFree);
> -	if (rc!=SQL_OK) {
> +	if (pFree != NULL)
> +		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;
> @@ -3744,10 +3696,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
> @@ -3756,7 +3706,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];
> @@ -3903,7 +3852,8 @@ 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;
>   			if (res) goto jump_to_p2;
> @@ -3928,10 +3878,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;
>   }
> @@ -3998,11 +3948,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;
> @@ -4137,15 +4085,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);
> @@ -4291,11 +4242,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;
>   }
> @@ -4325,8 +4273,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)
> @@ -4360,6 +4307,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;
> @@ -4427,14 +4375,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;
>   	}
>   
> @@ -4460,6 +4406,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;
> @@ -4510,8 +4457,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++;
> @@ -4545,18 +4491,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;
> @@ -4668,14 +4615,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;
>   }
>   
> @@ -4698,8 +4645,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;
>   }
> @@ -4732,8 +4679,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
> @@ -4743,20 +4690,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);
> @@ -4771,8 +4716,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;
>   }
>   
> @@ -4828,8 +4773,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;
>   	}
>   
> @@ -5157,11 +5102,9 @@ case OP_AggStep: {
>   	(pCtx->pFunc->xSFunc)(pCtx,pCtx->argc,pCtx->argv); /* IMP: R-24505-23230 */
>   	if (pCtx->is_aborted) {
>   		sqlVdbeMemRelease(&t);
> -		rc = SQL_TARANTOOL_ERROR;
>   		goto abort_due_to_error;
> -	} else {
> -		assert(t.flags==MEM_Null);
>   	}
> +	assert(t.flags==MEM_Null);
>   	if (pCtx->skipFlag) {
>   		assert(pOp[-1].opcode==OP_CollSeq);
>   		i = pOp[-1].p1;
> @@ -5188,11 +5131,8 @@ case OP_AggFinal: {
>   	assert(pOp->p1>0 && pOp->p1<=(p->nMem+1 - p->nCursor));
>   	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 (sqlVdbeMemFinalize(pMem, pOp->p4.pFunc) != 0)
>   		goto abort_due_to_error;
> -	}
>   	UPDATE_MAX_BLOBSIZE(pMem);
>   	if (sqlVdbeMemTooBig(pMem)) {
>   		goto too_big;
> @@ -5301,10 +5241,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;
>   }
> @@ -5368,28 +5306,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:
> @@ -5398,21 +5316,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 ee14510..9e6a426 100644
> --- a/src/box/sql/vdbeInt.h
> +++ b/src/box/sql/vdbeInt.h
> @@ -244,10 +244,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
> @@ -450,7 +446,6 @@ struct Vdbe {
>   /*
>    * Function prototypes
>    */
> -void sqlVdbeError(Vdbe *, const char *, ...);
>   void sqlVdbeFreeCursor(Vdbe *, VdbeCursor *);
>   void sqlVdbePopStack(Vdbe *, int);
>   int sqlVdbeCursorRestore(VdbeCursor *);
> @@ -532,13 +527,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 673ccd1..3bdfa7d 100644
> --- a/src/box/sql/vdbeapi.c
> +++ b/src/box/sql/vdbeapi.c
> @@ -536,15 +536,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
> @@ -564,20 +555,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);
> @@ -585,26 +573,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 619b211..3f573d0 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
> @@ -2115,10 +2102,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;
>   }
> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
> index 585dc21..d6375a6 100644
> --- a/src/box/sql/vdbemem.c
> +++ b/src/box/sql/vdbemem.c
> @@ -210,7 +210,6 @@ sqlVdbeMemMakeWriteable(Mem * pMem)
>    * If the given Mem* has a zero-filled tail, turn it into an ordinary
>    * blob stored in dynamically allocated space.
>    */
> -#ifndef SQL_OMIT_INCRBLOB
>   int
>   sqlVdbeMemExpandBlob(Mem * pMem)
>   {
> @@ -232,7 +231,6 @@ sqlVdbeMemExpandBlob(Mem * pMem)
>   	pMem->flags &= ~(MEM_Zero | MEM_Term);
>   	return SQL_OK;
>   }
> -#endif
>   
>   /*
>    * It is already known that pMem contains an unterminated string.
> @@ -315,8 +313,7 @@ sqlVdbeMemStringify(Mem * pMem, u8 bForce)
>    * This routine calls the finalize method for that function.  The
>    * result of the aggregate is stored back into pMem.
>    *
> - * Return SQL_ERROR if the finalizer reports an error.  SQL_OK
> - * otherwise.
> + * Return -1 if the finalizer reports an error. 0 otherwise.
>    */
>   int
>   sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
> @@ -337,9 +334,10 @@ sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
>   		if (pMem->szMalloc > 0)
>   			sqlDbFree(pMem->db, pMem->zMalloc);
>   		memcpy(pMem, &t, sizeof(t));
> -		return ctx.is_aborted ? SQL_TARANTOOL_ERROR : SQL_OK;
> +		if (ctx.is_aborted)
> +			return -1;
>   	}
> -	return SQL_OK;
> +	return 0;
>   }
>   
>   /*
> diff --git a/test/sql-tap/gh-2931-savepoints.test.lua b/test/sql-tap/gh-2931-savepoints.test.lua
> index 6123fc4..be499b6 100755
> --- a/test/sql-tap/gh-2931-savepoints.test.lua
> +++ b/test/sql-tap/gh-2931-savepoints.test.lua
> @@ -39,7 +39,7 @@ local testcases = {
>   		{0,{1,1}}},
>   	{"5",
>   		[[rollback to savepoint s1_2;]],
> -		{1, "Failed to execute SQL statement: no such savepoint: S1_2"}},
> +		{1, "Can not rollback to savepoint: the savepoint does not exist"}},
>   	{"6",
>   		[[insert into t1 values(2);
>   		select * from t1 union all select * from t2;]],
> diff --git a/test/sql/savepoints.result b/test/sql/savepoints.result
> index bb4a296..d20e0ed 100644
> --- a/test/sql/savepoints.result
> +++ b/test/sql/savepoints.result
> @@ -67,7 +67,7 @@ end;
>   ...
>   release_sv_fail();
>   ---
> -- error: 'Failed to execute SQL statement: no such savepoint: T1'
> +- error: 'Can not rollback to savepoint: the savepoint does not exist'
>   ...
>   box.commit();
>   ---
>
>
>
>

--------------BBA6F7D93F42705AAF51EAFB
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>Fixed commit-message:</p>
    <pre class="text-small">After this patch, all errors in VDBE will be set using diag_set().

<span class="issue-keyword tooltipped tooltipped-se">Closes</span> <a class="issue-link js-issue-link tooltipped tooltipped-ne" data-error-text="Failed to load issue title" data-id="424499021" data-permission-text="Issue title is private" data-hovercard-type="issue" data-hovercard-url="/tarantool/tarantool/issues/4074/hovercard" href="https://github.com/tarantool/tarantool/issues/4074">#4074</a>

</pre>
    <div class="moz-cite-prefix">On 5/25/19 1:24 PM, Mergen Imeev wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:20190525102431.GB15670@tarantool.org">
      <pre class="moz-quote-pre" wrap="">I moved patch "sql: use diag_set() to set an error in SQL
functions" to position before this patch. It allowed to
simplify this patch. New patch below.

On Wed, May 15, 2019 at 04:26:54PM +0300, n.pettik wrote:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">

</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">On 5 May 2019, at 15:17, <a class="moz-txt-link-abbreviated" href="mailto:imeevma@tarantool.org">imeevma@tarantool.org</a> wrote:

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(-)
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
Please, remove whole sqlErrStr(), tarantoolErrorMessage() -
they are unused now. The same concerns sql_ret_code() -
I see no reason keeping it. Please, remove all legacy routines
connected with error codes.

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">These functions are removed in the following patches.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">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

@@ -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) {
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
-&gt; if (sqlVdbeCheckFk() != 0)

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Fixed. I used SQL_OK but it will be replaced by 0 in the
following patches.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">		assert(user_session-&gt;sql_flags&amp;SQL_CountRows);
		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;
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
!= 0
Still need to call sqlOomFault(db); by jumping to no_mem label.

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Fixed.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">	nByte = pIn1-&gt;n + pIn2-&gt;n;
	if (nByte&gt;db-&gt;aLimit[SQL_LIMIT_LENGTH]) {
		goto too_big;


@@ -1723,11 +1715,15 @@ case OP_Function: {
	/* If the function returned an error, throw an exception */
	if (pCtx-&gt;fErrorOrAux) {
		if (pCtx-&gt;isError) {
-			sqlVdbeError(p, "%s", sql_value_text(pCtx-&gt;pOut));
-			rc = pCtx-&gt;isError;
+			if (pCtx-&gt;isError != SQL_TARANTOOL_ERROR) {
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
How it can be different from SQL_TARANTOOL_ERROR?

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">It was possible, but this part of code was completely
changed after i moved patch "sql: use diag_set() to set an
error in SQL functions" to position before this patch.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">+				diag_set(ClientError, ER_SQL_EXECUTE,
+					 sql_value_text(pCtx-&gt;pOut));
+			}
+			rc = SQL_TARANTOOL_ERROR;
		}
		sqlVdbeDeleteAuxData(db, &amp;p-&gt;pAuxData, pCtx-&gt;iOp, pOp-&gt;p1);
-		if (rc) goto abort_due_to_error;
+		if (rc != SQL_OK)
+			goto abort_due_to_error;
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
This diff seems to be redundant

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Fixed.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">@@ -1910,8 +1903,7 @@ case OP_Realify: {                  /* in1 */
 */
case OP_Cast: {                  /* in1 */
	pIn1 = &amp;aMem[pOp-&gt;p1];
-	rc = ExpandBlob(pIn1);
-	if (rc != 0)
+	if (ExpandBlob(pIn1) != SQL_OK)
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
!= 0
Please, don’t use SQL_OK value anywhere.

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">I think it's better to use SQL_OK here for integrity. It is
replaced with 0 in one of the following patches.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">
@@ -2802,10 +2784,8 @@ case OP_MakeRecord: {
	uint32_t tuple_size;
	char *tuple =
		sql_vdbe_mem_encode_tuple(pData0, nField, &amp;tuple_size, region);
-	if (tuple == NULL) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (tuple == NULL)
		goto abort_due_to_error;
-	}
	if ((int64_t)tuple_size &gt; db-&gt;aLimit[SQL_LIMIT_LENGTH])
		goto too_big;


@@ -2918,8 +2898,10 @@ case OP_Savepoint: {
			pSavepoint = pSavepoint-&gt;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);
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
We already have ER_NO_SUCH_SAVEPOINT.

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Fixed.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">@@ -3685,10 +3645,11 @@ case OP_Found: {        /* jump, in3 */
		}
	}
	rc = sqlCursorMovetoUnpacked(pC-&gt;uc.pCursor, pIdxKey, &amp;res);
-	if (pFree) sqlDbFree(db, pFree);
-	if (rc!=SQL_OK) {
+	if (pFree)
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
!= NULL

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Fixed.
</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">
@@ -5164,11 +5126,16 @@ case OP_AggStep: {
	(pCtx-&gt;pFunc-&gt;xSFunc)(pCtx,pCtx-&gt;argc,pCtx-&gt;argv); /* IMP: R-24505-23230 */
	if (pCtx-&gt;fErrorOrAux) {
		if (pCtx-&gt;isError) {
-			sqlVdbeError(p, "%s", sql_value_text(&amp;t));
-			rc = pCtx-&gt;isError;
+			if (pCtx-&gt;isError != SQL_TARANTOOL_ERROR) {
+				diag_set(ClientError, ER_SQL_EXECUTE,
+					 sql_value_text(&amp;t));
+			}
+			rc = SQL_TARANTOOL_ERROR;
		}
		sqlVdbeMemRelease(&amp;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;
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
Looks like redundant diff

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Fixed.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">	} else {
		assert(t.flags==MEM_Null);
	}
@@ -5199,8 +5166,11 @@ case OP_AggFinal: {
	pMem = &amp;aMem[pOp-&gt;p1];
	assert((pMem-&gt;flags &amp; ~(MEM_Null|MEM_Agg))==0);
	rc = sqlVdbeMemFinalize(pMem, pOp-&gt;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));
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
Could you please clarify what does this error mean?
It would just print value of memory to string…
The same relates to error in OP_AggStep.

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">If result was returned using sql_result_error() then it was
possible that diag is not set. This was fixed in patch that
now right before this patch.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">@@ -5311,10 +5281,8 @@ case OP_IncMaxid: {
	assert(pOp-&gt;p1 &gt; 0);
	pOut = &amp;aMem[pOp-&gt;p1];

-	rc = tarantoolsqlIncrementMaxid((uint64_t*) &amp;pOut-&gt;u.i);
-	if (rc!=SQL_OK) {
+	if (tarantoolsqlIncrementMaxid((uint64_t*) &amp;pOut-&gt;u.i) != SQL_OK)
		goto abort_due_to_error;
-	}
	pOut-&gt;flags = MEM_Int;
	break;
}

too_big:
-	sqlVdbeError(p, "string or blob too big");
-	rc = SQL_TOOBIG;
+	diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big”);
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
-&gt; is too big

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Fixed here. Still, it is possible to find this error in
other places. I think it will be fixed when we create error
code for such errors.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">	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


@@ -550,13 +545,8 @@ void sqlVdbeMemPrettyPrint(Mem * pMem, char *zBuf);
#endif
int sqlVdbeMemHandleBom(Mem * pMem);

-#ifndef SQL_OMIT_INCRBLOB
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
Still see usage of this macro in code: vdbemem.c : 213

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Fixed.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">int sqlVdbeMemExpandBlob(Mem *);
#define ExpandBlob(P) (((P)-&gt;flags&amp;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/vdbeaux.c b/src/box/sql/vdbeaux.c
index 27fa5b2..48c2a81 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
void
@@ -2124,10 +2111,11 @@ sqlVdbeCheckFk(Vdbe * p, int deferred)
	if ((deferred &amp;&amp; txn != NULL &amp;&amp; txn-&gt;psql_txn != NULL &amp;&amp;
	     txn-&gt;psql_txn-&gt;fk_deferred_count &gt; 0) ||
	    (!deferred &amp;&amp; p-&gt;nFkConstraint &gt; 0)) {
-		p-&gt;rc = SQL_CONSTRAINT_FOREIGNKEY;
+		p-&gt;rc = SQL_TARANTOOL_ERROR;
		p-&gt;errorAction = ON_CONFLICT_ACTION_ABORT;
-		sqlVdbeError(p, "FOREIGN KEY constraint failed");
-		return SQL_ERROR;
+		diag_set(ClientError, ER_SQL_EXECUTE, "FOREIGN KEY constraint "\
+			 "failed”);
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
Please, reserve separate error code for this violation.

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">I am going to do this a bit later, in a different patch.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">+		return SQL_TARANTOOL_ERROR;
	}
	return SQL_OK;
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
Return 0/-1


</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Left as it as for now, will be fixed in following patches.


New patch:

>From 85cfbe96609b66379631c8d4534c8a9329fb3a47 Mon Sep 17 00:00:00 2001
Date: Mon, 22 Apr 2019 19:41:46 +0300
Subject: [PATCH] sql: set errors in VDBE using diag_set()

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

Part of #4074

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 &gt; 0) {
@@ -427,15 +426,8 @@ sql_execute(sql *db, struct sql_stmt *stmt, struct port *port,
 		rc = sql_step(stmt);
 		assert(rc != SQL_ROW &amp;&amp; rc != SQL_OK);
 	}
-	if (rc != SQL_DONE) {
-		if (db-&gt;errCode != SQL_TARANTOOL_ERROR) {
-			const char *err = (char *)sql_value_text(db-&gt;pErr);
-			if (err == NULL)
-				err = sqlErrStr(db-&gt;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, &amp;stmt, NULL) != SQL_OK) {
-		if (db-&gt;errCode != SQL_TARANTOOL_ERROR) {
-			const char *err = (char *)sql_value_text(db-&gt;pErr);
-			if (err == NULL)
-				err = sqlErrStr(db-&gt;errCode);
-			diag_set(ClientError, ER_SQL_EXECUTE, err);
-		}
+	if (sql_prepare_v2(db, sql, len, &amp;stmt, NULL) != SQL_OK)
 		return -1;
-	}
 	assert(stmt != NULL);
 	port_sql_create(port, stmt);
 	if (sql_bind(stmt, bind, bind_count) == 0 &amp;&amp;
-	    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 7d85959..b64293a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -986,7 +986,7 @@ case OP_Halt: {
 		p-&gt;rc = SQL_BUSY;
 	} else {
 		assert(rc==SQL_OK || (p-&gt;rc&amp;0xff)==SQL_CONSTRAINT);
-		rc = p-&gt;rc ? SQL_ERROR : SQL_DONE;
+		rc = p-&gt;rc ? SQL_TARANTOOL_ERROR : SQL_DONE;
 	}
 	goto vdbe_return;
 }
@@ -1098,17 +1098,13 @@ case OP_NextAutoincValue: {
 	assert(pOp-&gt;p2 &gt; 0);
 
 	struct space *space = space_by_id(pOp-&gt;p1);
-	if (space == NULL) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (space == NULL)
 		goto abort_due_to_error;
-	}
 
 	int64_t value;
 	struct sequence *sequence = space-&gt;sequence;
-	if (sequence == NULL || sequence_next(sequence, &amp;value) != 0) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (sequence == NULL || sequence_next(sequence, &amp;value) != 0)
 		goto abort_due_to_error;
-	}
 
 	pOut = out2Prerelease(p, pOp);
 	pOut-&gt;flags = MEM_Int;
@@ -1335,7 +1331,7 @@ 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))) {
+	if (sqlVdbeCheckFk(p, 0) != SQL_OK) {
 		assert(user_session-&gt;sql_flags&amp;SQL_CountRows);
 		goto abort_due_to_error;
 	}
@@ -1427,7 +1423,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 +1430,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-&gt;n + pIn2-&gt;n;
 	if (nByte&gt;db-&gt;aLimit[SQL_LIMIT_LENGTH]) {
 		goto too_big;
@@ -1551,13 +1546,11 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		if (sqlVdbeRealValue(pIn1, &amp;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, &amp;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-&gt;opcode) {
@@ -1597,11 +1590,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;
 }
 
@@ -1721,10 +1712,8 @@ case OP_Function: {
 	(*pCtx-&gt;pFunc-&gt;xSFunc)(pCtx, pCtx-&gt;argc, pCtx-&gt;argv);/* IMP: R-24505-23230 */
 
 	/* If the function returned an error, throw an exception */
-	if (pCtx-&gt;is_aborted) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (pCtx-&gt;is_aborted)
 		goto abort_due_to_error;
-	}
 
 	/* Copy the result of the function into register P3 */
 	if (pOut-&gt;flags &amp; (MEM_Str|MEM_Blob)) {
@@ -1785,13 +1774,11 @@ case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
 	if (sqlVdbeIntValue(pIn2, (int64_t *) &amp;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 *) &amp;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-&gt;opcode;
@@ -1860,7 +1847,6 @@ case OP_MustBeInt: {            /* jump, in1 */
 			if (pOp-&gt;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;
@@ -1906,8 +1892,7 @@ case OP_Realify: {                  /* in1 */
  */
 case OP_Cast: {                  /* in1 */
 	pIn1 = &amp;aMem[pOp-&gt;p1];
-	rc = ExpandBlob(pIn1);
-	if (rc != 0)
+	if (ExpandBlob(pIn1) != SQL_OK)
 		goto abort_due_to_error;
 	rc = sqlVdbeMemCast(pIn1, pOp-&gt;p2);
 	UPDATE_MAX_BLOBSIZE(pIn1);
@@ -1915,7 +1900,6 @@ case OP_Cast: {                  /* in1 */
 		break;
 	diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_text(pIn1),
 		 field_type_strs[pOp-&gt;p2]);
-	rc = SQL_TARANTOOL_ERROR;
 	goto abort_due_to_error;
 }
 #endif /* SQL_OMIT_CAST */
@@ -2068,7 +2052,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);
@@ -2087,7 +2070,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;
 					}
 
@@ -2329,7 +2311,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 = &amp;aMem[pOp-&gt;p2];
@@ -2340,7 +2321,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-&gt;opcode==OP_And) {
@@ -2374,7 +2354,6 @@ case OP_Not: {                /* same as TK_NOT, in1, out2 */
 		if ((pIn1-&gt;flags &amp; 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-&gt;u.b);
@@ -2398,7 +2377,6 @@ case OP_BitNot: {             /* same as TK_BITNOT, in1, out2 */
 		if (sqlVdbeIntValue(pIn1, &amp;i) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn1), "integer");
-			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
 		pOut-&gt;flags = MEM_Int;
@@ -2446,7 +2424,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);
@@ -2586,8 +2563,9 @@ case OP_Column: {
 		zEnd = zData + pC-&gt;payloadSize;
 	} else {
 		memset(&amp;sMem, 0, sizeof(sMem));
-		rc = sqlVdbeMemFromBtree(pC-&gt;uc.pCursor, 0, pC-&gt;payloadSize, &amp;sMem);
-		if (rc!=SQL_OK) goto abort_due_to_error;
+		if (sqlVdbeMemFromBtree(pC-&gt;uc.pCursor, 0, pC-&gt;payloadSize,
+					&amp;sMem) != SQL_OK)
+			goto abort_due_to_error;
 		zData = (u8*)sMem.z;
 		zEnd = zData + pC-&gt;payloadSize;
 	}
@@ -2646,10 +2624,8 @@ case OP_Column: {
 	}
 	uint32_t unused;
 	if (vdbe_decode_msgpack_into_mem((const char *)(zData + aOffset[p2]),
-					 pDest, &amp;unused) != 0) {
-		rc = SQL_TARANTOOL_ERROR;
+					 pDest, &amp;unused) != 0)
 		goto abort_due_to_error;
-	}
 	/* MsgPack map, array or extension (unsupported in sql).
 	 * Wrap it in a blob verbatim.
 	 */
@@ -2683,7 +2659,11 @@ case OP_Column: {
 	if ((pDest-&gt;flags &amp; (MEM_Ephem | MEM_Str)) == (MEM_Ephem | MEM_Str)) {
 		int len = pDest-&gt;n;
 		if (pDest-&gt;szMalloc&lt;len+1) {
-			if (sqlVdbeMemGrow(pDest, len+1, 1)) goto op_column_error;
+			if (sqlVdbeMemGrow(pDest, len + 1, 1)) {
+				if (zData != pC-&gt;aRow)
+					sqlVdbeMemRelease(&amp;sMem);
+				goto abort_due_to_error;
+			}
 		} else {
 			pDest-&gt;z = memcpy(pDest-&gt;zMalloc, pDest-&gt;z, len);
 			pDest-&gt;flags &amp;= ~MEM_Ephem;
@@ -2697,10 +2677,6 @@ case OP_Column: {
 	UPDATE_MAX_BLOBSIZE(pDest);
 	REGISTER_TRACE(pOp-&gt;p3, pDest);
 	break;
-
-			op_column_error:
-	if (zData!=pC-&gt;aRow) sqlVdbeMemRelease(&amp;sMem);
-	goto abort_due_to_error;
 }
 
 /* Opcode: ApplyType P1 P2 * P4 *
@@ -2725,7 +2701,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++;
@@ -2798,10 +2773,8 @@ case OP_MakeRecord: {
 	uint32_t tuple_size;
 	char *tuple =
 		sql_vdbe_mem_encode_tuple(pData0, nField, &amp;tuple_size, region);
-	if (tuple == NULL) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (tuple == NULL)
 		goto abort_due_to_error;
-	}
 	if ((int64_t)tuple_size &gt; db-&gt;aLimit[SQL_LIMIT_LENGTH])
 		goto too_big;
 
@@ -2856,13 +2829,14 @@ case OP_Count: {         /* out2 */
 	assert(pCrsr);
 	nEntry = 0;  /* Not needed.  Only used to silence a warning. */
 	if (pCrsr-&gt;curFlags &amp; BTCF_TaCursor) {
-		rc = tarantoolsqlCount(pCrsr, &amp;nEntry);
+		if (tarantoolsqlCount(pCrsr, &amp;nEntry) != SQL_OK)
+			goto abort_due_to_error;
 	} else if (pCrsr-&gt;curFlags &amp; BTCF_TEphemCursor) {
-		rc = tarantoolsqlEphemeralCount(pCrsr, &amp;nEntry);
+		if (tarantoolsqlEphemeralCount(pCrsr, &amp;nEntry) != SQL_OK)
+			goto abort_due_to_error;
 	} else {
 		unreachable();
 	}
-	if (rc) goto abort_due_to_error;
 	pOut = out2Prerelease(p, pOp);
 	pOut-&gt;u.i = nEntry;
 	break;
@@ -2886,7 +2860,6 @@ case OP_Savepoint: {
 	if (psql_txn == NULL) {
 		assert(!box_txn());
 		diag_set(ClientError, ER_NO_TRANSACTION);
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	p1 = pOp-&gt;p1;
@@ -2914,8 +2887,8 @@ case OP_Savepoint: {
 			pSavepoint = pSavepoint-&gt;pNext
 			);
 		if (!pSavepoint) {
-			sqlVdbeError(p, "no such savepoint: %s", zName);
-			rc = SQL_ERROR;
+			diag_set(ClientError, ER_NO_SUCH_SAVEPOINT);
+			goto abort_due_to_error;
 		} else {
 
 			/* Determine whether or not this is a transaction savepoint. If so,
@@ -2932,7 +2905,8 @@ case OP_Savepoint: {
 					p-&gt;rc = rc = SQL_BUSY;
 					goto vdbe_return;
 				}
-				rc = p-&gt;rc;
+				if (p-&gt;rc != SQL_OK)
+					goto abort_due_to_error;
 			} else {
 				if (p1==SAVEPOINT_ROLLBACK)
 					box_txn_rollback_to_savepoint(pSavepoint-&gt;tnt_savepoint);
@@ -2964,7 +2938,6 @@ case OP_Savepoint: {
 			}
 		}
 	}
-	if (rc) goto abort_due_to_error;
 
 	break;
 }
@@ -2987,7 +2960,6 @@ case OP_CheckViewReferences: {
 	if (space-&gt;def-&gt;view_ref_count &gt; 0) {
 		diag_set(ClientError, ER_DROP_SPACE, space-&gt;def-&gt;name,
 			 "other views depend on this space");
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	break;
@@ -3000,10 +2972,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-&gt;auto_commit = false	;
 	break;
 }
@@ -3019,13 +2989,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;
@@ -3038,14 +3006,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;
@@ -3063,16 +3028,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-&gt;anonymous_savepoint = sql_savepoint(p, NULL);
-		if (p-&gt;anonymous_savepoint == NULL) {
-			rc = SQL_TARANTOOL_ERROR;
+		if (p-&gt;anonymous_savepoint == NULL)
 			goto abort_due_to_error;
-		}
 	}
 	break;
 }
@@ -3111,9 +3072,8 @@ case OP_IteratorOpen:
 	if (box_schema_version() != p-&gt;schema_ver &amp;&amp;
 	    (pOp-&gt;p5 &amp; OPFLAG_SYSTEMSP) == 0) {
 		p-&gt;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;
@@ -3122,10 +3082,8 @@ case OP_IteratorOpen:
 	else
 		space = aMem[pOp-&gt;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-&gt;p2);
 	assert(index != NULL);
@@ -3148,8 +3106,6 @@ case OP_IteratorOpen:
 	cur-&gt;nullRow = 1;
 open_cursor_set_hints:
 	cur-&gt;uc.pCursor-&gt;hints = pOp-&gt;p5 &amp; OPFLAG_SEEKEQ;
-	if (rc != 0)
-		goto abort_due_to_error;
 	break;
 }
 
@@ -3171,10 +3127,8 @@ case OP_OpenTEphemeral: {
 	struct space *space = sql_ephemeral_space_create(pOp-&gt;p2,
 							 pOp-&gt;p4.key_info);
 
-	if (space == NULL) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (space == NULL)
 		goto abort_due_to_error;
-	}
 	aMem[pOp-&gt;p1].u.p = space;
 	aMem[pOp-&gt;p1].flags = MEM_Ptr;
 	break;
@@ -3200,8 +3154,8 @@ case OP_SorterOpen: {
 	pCx = allocateCursor(p, pOp-&gt;p1, pOp-&gt;p2, CURTYPE_SORTER);
 	if (pCx==0) goto no_mem;
 	pCx-&gt;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;
 }
 
@@ -3433,7 +3387,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;
@@ -3514,10 +3467,8 @@ case OP_SeekGT: {       /* jump, in3 */
 #endif
 	r.eqSeen = 0;
 	r.opcode = oc;
-	rc = sqlCursorMovetoUnpacked(pC-&gt;uc.pCursor, &amp;r, &amp;res);
-	if (rc!=SQL_OK) {
+	if (sqlCursorMovetoUnpacked(pC-&gt;uc.pCursor, &amp;r, &amp;res) != SQL_OK)
 		goto abort_due_to_error;
-	}
 	if (eqOnly &amp;&amp; r.eqSeen==0) {
 		assert(res!=0);
 		goto seek_not_found;
@@ -3529,8 +3480,8 @@ case OP_SeekGT: {       /* jump, in3 */
 	if (oc&gt;=OP_SeekGE) {  assert(oc==OP_SeekGE || oc==OP_SeekGT);
 		if (res&lt;0 || (res==0 &amp;&amp; oc==OP_SeekGT)) {
 			res = 0;
-			rc = sqlCursorNext(pC-&gt;uc.pCursor, &amp;res);
-			if (rc!=SQL_OK) goto abort_due_to_error;
+			if (sqlCursorNext(pC-&gt;uc.pCursor, &amp;res) != SQL_OK)
+				goto abort_due_to_error;
 		} else {
 			res = 0;
 		}
@@ -3538,8 +3489,8 @@ case OP_SeekGT: {       /* jump, in3 */
 		assert(oc==OP_SeekLT || oc==OP_SeekLE);
 		if (res&gt;0 || (res==0 &amp;&amp; oc==OP_SeekLT)) {
 			res = 0;
-			rc = sqlCursorPrevious(pC-&gt;uc.pCursor, &amp;res);
-			if (rc!=SQL_OK) goto abort_due_to_error;
+			if (sqlCursorPrevious(pC-&gt;uc.pCursor, &amp;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.
@@ -3681,10 +3632,11 @@ case OP_Found: {        /* jump, in3 */
 		}
 	}
 	rc = sqlCursorMovetoUnpacked(pC-&gt;uc.pCursor, pIdxKey, &amp;res);
-	if (pFree) sqlDbFree(db, pFree);
-	if (rc!=SQL_OK) {
+	if (pFree != NULL)
+		sqlDbFree(db, pFree);
+	assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
+	if (rc != SQL_OK)
 		goto abort_due_to_error;
-	}
 	pC-&gt;seekResult = res;
 	alreadyExists = (res==0);
 	pC-&gt;nullRow = 1-alreadyExists;
@@ -3744,10 +3696,8 @@ case OP_NextIdEphemeral: {
 	struct space *space = (struct space*)p-&gt;aMem[pOp-&gt;p1].u.p;
 	assert(space-&gt;def-&gt;id == 0);
 	uint64_t rowid;
-	if (space-&gt;vtab-&gt;ephemeral_rowid_next(space, &amp;rowid) != 0) {
-		rc = SQL_TARANTOOL_ERROR;
+	if (space-&gt;vtab-&gt;ephemeral_rowid_next(space, &amp;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
@@ -3756,7 +3706,6 @@ case OP_NextIdEphemeral: {
 	 */
 	if (rowid &gt; INT32_MAX) {
 		diag_set(ClientError, ER_ROWID_OVERFLOW);
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
 	pOut = &amp;aMem[pOp-&gt;p2];
@@ -3903,7 +3852,8 @@ case OP_SorterCompare: {
 			pIn3 = &amp;aMem[pOp-&gt;p3];
 			nKeyCol = pOp-&gt;p4.i;
 			res = 0;
-			rc = sqlVdbeSorterCompare(pC, pIn3, nKeyCol, &amp;res);
+			if (sqlVdbeSorterCompare(pC, pIn3, nKeyCol, &amp;res) != 0)
+				rc = SQL_TARANTOOL_ERROR;
 			VdbeBranchTaken(res!=0,2);
 			if (rc) goto abort_due_to_error;
 			if (res) goto jump_to_p2;
@@ -3928,10 +3878,10 @@ case OP_SorterData: {
 	pOut = &amp;aMem[pOp-&gt;p2];
 	pC = p-&gt;apCsr[pOp-&gt;p1];
 	assert(isSorter(pC));
-	rc = sqlVdbeSorterRowkey(pC, pOut);
-	assert(rc!=SQL_OK || (pOut-&gt;flags &amp; MEM_Blob));
+	if (sqlVdbeSorterRowkey(pC, pOut) != SQL_OK)
+		goto abort_due_to_error;
+	assert(pOut-&gt;flags &amp; MEM_Blob);
 	assert(pOp-&gt;p1&gt;=0 &amp;&amp; pOp-&gt;p1&lt;p-&gt;nCursor);
-	if (rc) goto abort_due_to_error;
 	p-&gt;apCsr[pOp-&gt;p3]-&gt;cacheStatus = CACHE_STALE;
 	break;
 }
@@ -3998,11 +3948,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-&gt;z);
-	if (rc) goto abort_due_to_error;
+	if (sql_vdbe_mem_alloc_region(pOut, n) != SQL_OK ||
+	    sqlCursorPayload(pCrsr, 0, n, pOut-&gt;z) != SQL_OK)
+		goto abort_due_to_error;
 	UPDATE_MAX_BLOBSIZE(pOut);
 	REGISTER_TRACE(pOp-&gt;p2, pOut);
 	break;
@@ -4137,15 +4085,18 @@ case OP_Rewind: {        /* jump */
 	pC-&gt;seekOp = OP_Rewind;
 #endif
 	if (isSorter(pC)) {
-		rc = sqlVdbeSorterRewind(pC, &amp;res);
+		if (sqlVdbeSorterRewind(pC, &amp;res) != SQL_OK)
+			goto abort_due_to_error;
 	} else {
 		assert(pC-&gt;eCurType==CURTYPE_TARANTOOL);
 		pCrsr = pC-&gt;uc.pCursor;
 		assert(pCrsr);
-		rc = tarantoolsqlFirst(pCrsr, &amp;res);
+		if (tarantoolsqlFirst(pCrsr, &amp;res) != SQL_OK)
+			rc = SQL_TARANTOOL_ERROR;
 		pC-&gt;cacheStatus = CACHE_STALE;
+		if (rc != SQL_OK)
+			goto abort_due_to_error;
 	}
-	if (rc) goto abort_due_to_error;
 	pC-&gt;nullRow = (u8)res;
 	assert(pOp-&gt;p2&gt;0 &amp;&amp; pOp-&gt;p2&lt;p-&gt;nOp);
 	VdbeBranchTaken(res!=0,2);
@@ -4291,11 +4242,8 @@ case OP_SorterInsert: {      /* in2 */
 	assert(isSorter(cursor));
 	pIn2 = &amp;aMem[pOp-&gt;p2];
 	assert((pIn2-&gt;flags &amp; 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;
 }
@@ -4325,8 +4273,7 @@ case OP_IdxInsert: {
 	assert((pIn2-&gt;flags &amp; MEM_Blob) != 0);
 	if (pOp-&gt;p5 &amp; OPFLAG_NCHANGE)
 		p-&gt;nChange++;
-	rc = ExpandBlob(pIn2);
-	if (rc != 0)
+	if (ExpandBlob(pIn2) != SQL_OK)
 		goto abort_due_to_error;
 	struct space *space;
 	if (pOp-&gt;p4type == P4_SPACEPTR)
@@ -4360,6 +4307,7 @@ case OP_IdxInsert: {
 	} else if (pOp-&gt;p5 &amp; OPFLAG_OE_ROLLBACK) {
 		p-&gt;errorAction = ON_CONFLICT_ACTION_ROLLBACK;
 	}
+	assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
 	if (rc != 0)
 		goto abort_due_to_error;
 	break;
@@ -4427,14 +4375,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;
 	}
 
@@ -4460,6 +4406,7 @@ case OP_Update: {
 	} else if (pOp-&gt;p5 &amp; OPFLAG_OE_ROLLBACK) {
 		p-&gt;errorAction = ON_CONFLICT_ACTION_ROLLBACK;
 	}
+	assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
 	if (rc != 0)
 		goto abort_due_to_error;
 	break;
@@ -4510,8 +4457,7 @@ case OP_SDelete: {
 	struct space *space = space_by_id(pOp-&gt;p1);
 	assert(space != NULL);
 	assert(space_is_system(space));
-	rc = sql_delete_by_key(space, 0, pIn2-&gt;z, pIn2-&gt;n);
-	if (rc)
+	if (sql_delete_by_key(space, 0, pIn2-&gt;z, pIn2-&gt;n) != SQL_OK)
 		goto abort_due_to_error;
 	if (pOp-&gt;p5 &amp; OPFLAG_NCHANGE)
 		p-&gt;nChange++;
@@ -4545,18 +4491,19 @@ case OP_IdxDelete: {
 	r.default_rc = 0;
 	r.aMem = &amp;aMem[pOp-&gt;p2];
 	r.opcode = OP_IdxDelete;
-	rc = sqlCursorMovetoUnpacked(pCrsr, &amp;r, &amp;res);
-	if (rc) goto abort_due_to_error;
+	if (sqlCursorMovetoUnpacked(pCrsr, &amp;r, &amp;res) != SQL_OK)
+		goto abort_due_to_error;
 	if (res==0) {
 		assert(pCrsr-&gt;eState == CURSOR_VALID);
 		if (pCrsr-&gt;curFlags &amp; BTCF_TaCursor) {
-			rc = tarantoolsqlDelete(pCrsr, 0);
+			if (tarantoolsqlDelete(pCrsr, 0) != SQL_OK)
+				goto abort_due_to_error;
 		} else if (pCrsr-&gt;curFlags &amp; 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-&gt;cacheStatus = CACHE_STALE;
 	pC-&gt;seekResult = 0;
@@ -4668,14 +4615,14 @@ case OP_Clear: {
 	rc = 0;
 	if (pOp-&gt;p2 &gt; 0) {
 		if (box_truncate(space_id) != 0)
-			rc = SQL_TARANTOOL_ERROR;
+			goto abort_due_to_error;
 	} else {
 		uint32_t tuple_count;
-		rc = tarantoolsqlClearTable(space, &amp;tuple_count);
-		if (rc == 0 &amp;&amp; (pOp-&gt;p5 &amp; OPFLAG_NCHANGE) != 0)
+		if (tarantoolsqlClearTable(space, &amp;tuple_count) != SQL_OK)
+			goto abort_due_to_error;
+		if ((pOp-&gt;p5 &amp; OPFLAG_NCHANGE) != 0)
 			p-&gt;nChange += tuple_count;
 	}
-	if (rc) goto abort_due_to_error;
 	break;
 }
 
@@ -4698,8 +4645,8 @@ case OP_ResetSorter: {
 	} else {
 		assert(pC-&gt;eCurType==CURTYPE_TARANTOOL);
 		assert(pC-&gt;uc.pCursor-&gt;curFlags &amp; BTCF_TEphemCursor);
-		rc = tarantoolsqlEphemeralClearTable(pC-&gt;uc.pCursor);
-		if (rc) goto abort_due_to_error;
+		if (tarantoolsqlEphemeralClearTable(pC-&gt;uc.pCursor) != SQL_OK)
+			goto abort_due_to_error;
 	}
 	break;
 }
@@ -4732,8 +4679,8 @@ case OP_RenameTable: {
 	zNewTableName = pOp-&gt;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
@@ -4743,20 +4690,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-&gt;next;
-		rc = tarantoolsqlRenameTrigger(trigger-&gt;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-&gt;zName, zOldTableName,
+					      zNewTableName) != SQL_OK)
 			goto abort_due_to_error;
-		}
 		trigger = next_trigger;
 	}
 	sqlDbFree(db, (void*)zOldTableName);
@@ -4771,8 +4716,8 @@ case OP_RenameTable: {
  */
 case OP_LoadAnalysis: {
 	assert(pOp-&gt;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;
 }
 
@@ -4828,8 +4773,8 @@ case OP_Program: {        /* jump */
 	}
 
 	if (p-&gt;nFrame&gt;=db-&gt;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;
 	}
 
@@ -5157,11 +5102,9 @@ case OP_AggStep: {
 	(pCtx-&gt;pFunc-&gt;xSFunc)(pCtx,pCtx-&gt;argc,pCtx-&gt;argv); /* IMP: R-24505-23230 */
 	if (pCtx-&gt;is_aborted) {
 		sqlVdbeMemRelease(&amp;t);
-		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
-	} else {
-		assert(t.flags==MEM_Null);
 	}
+	assert(t.flags==MEM_Null);
 	if (pCtx-&gt;skipFlag) {
 		assert(pOp[-1].opcode==OP_CollSeq);
 		i = pOp[-1].p1;
@@ -5188,11 +5131,8 @@ case OP_AggFinal: {
 	assert(pOp-&gt;p1&gt;0 &amp;&amp; pOp-&gt;p1&lt;=(p-&gt;nMem+1 - p-&gt;nCursor));
 	pMem = &amp;aMem[pOp-&gt;p1];
 	assert((pMem-&gt;flags &amp; ~(MEM_Null|MEM_Agg))==0);
-	rc = sqlVdbeMemFinalize(pMem, pOp-&gt;p4.pFunc);
-	if (rc) {
-		sqlVdbeError(p, "%s", sql_value_text(pMem));
+	if (sqlVdbeMemFinalize(pMem, pOp-&gt;p4.pFunc) != 0)
 		goto abort_due_to_error;
-	}
 	UPDATE_MAX_BLOBSIZE(pMem);
 	if (sqlVdbeMemTooBig(pMem)) {
 		goto too_big;
@@ -5301,10 +5241,8 @@ case OP_IncMaxid: {
 	assert(pOp-&gt;p1 &gt; 0);
 	pOut = &amp;aMem[pOp-&gt;p1];
 
-	rc = tarantoolsqlIncrementMaxid((uint64_t*) &amp;pOut-&gt;u.i);
-	if (rc!=SQL_OK) {
+	if (tarantoolsqlIncrementMaxid((uint64_t*) &amp;pOut-&gt;u.i) != SQL_OK)
 		goto abort_due_to_error;
-	}
 	pOut-&gt;flags = MEM_Int;
 	break;
 }
@@ -5368,28 +5306,8 @@ default: {          /* This is really OP_Noop and OP_Explain */
 	 * an error of some kind.
 	 */
 abort_due_to_error:
-	if (db-&gt;mallocFailed) rc = SQL_NOMEM;
-	assert(rc);
-	if (p-&gt;zErrMsg==0 &amp;&amp; rc!=SQL_IOERR_NOMEM) {
-		const char *msg;
-		/* Avoiding situation when Tarantool error is set,
-		 * but error message isn't.
-		 */
-		if (rc == SQL_TARANTOOL_ERROR &amp;&amp; tarantoolErrorMessage()) {
-			msg = tarantoolErrorMessage();
-		} else {
-			msg = sqlErrStr(rc);
-		}
-		sqlVdbeError(p, "%s", msg);
-	}
+	rc = SQL_TARANTOOL_ERROR;
 	p-&gt;rc = rc;
-	sqlSystemError(db, rc);
-	testcase( sqlGlobalConfig.xLog!=0);
-	sql_log(rc, "statement aborts at %d: [%s] %s",
-		    (int)(pOp - aOp), p-&gt;zSql, p-&gt;zErrMsg);
-	sqlVdbeHalt(p);
-	if (rc==SQL_IOERR_NOMEM) sqlOomFault(db);
-	rc = SQL_ERROR;
 
 	/* This is the only way out of this procedure. */
 vdbe_return:
@@ -5398,21 +5316,20 @@ vdbe_return:
 	assert(rc!=SQL_OK || nExtraDelete==0
 		|| sql_strlike_ci("DELETE%", p-&gt;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 ee14510..9e6a426 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -244,10 +244,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
@@ -450,7 +446,6 @@ struct Vdbe {
 /*
  * Function prototypes
  */
-void sqlVdbeError(Vdbe *, const char *, ...);
 void sqlVdbeFreeCursor(Vdbe *, VdbeCursor *);
 void sqlVdbePopStack(Vdbe *, int);
 int sqlVdbeCursorRestore(VdbeCursor *);
@@ -532,13 +527,8 @@ void sqlVdbeMemPrettyPrint(Mem * pMem, char *zBuf);
 #endif
 int sqlVdbeMemHandleBom(Mem * pMem);
 
-#ifndef SQL_OMIT_INCRBLOB
 int sqlVdbeMemExpandBlob(Mem *);
 #define ExpandBlob(P) (((P)-&gt;flags&amp;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 673ccd1..3bdfa7d 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -536,15 +536,6 @@ sqlStep(Vdbe * p)
 		p-&gt;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-&gt;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 &amp; 0xff) == SQL_BUSY || rc == SQL_MISUSE);
 	if (p-&gt;isPrepareV2 &amp;&amp; rc != SQL_ROW &amp;&amp; rc != SQL_DONE) {
 		/* If this statement was prepared using sql_prepare_v2(), and an
 		 * error has occurred, then return the error code in p-&gt;rc to the
@@ -564,20 +555,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-&gt;db;
 	v-&gt;doingRerun = 0;
 	while ((rc = sqlStep(v)) == SQL_SCHEMA
 	       &amp;&amp; cnt++ &lt; SQL_MAX_SCHEMA_RETRY) {
 		int savedPc = v-&gt;pc;
-		rc2 = rc = sqlReprepare(v);
+		rc = sqlReprepare(v);
 		if (rc != SQL_OK)
 			break;
 		sql_reset(pStmt);
@@ -585,26 +573,6 @@ sql_step(sql_stmt * pStmt)
 			v-&gt;doingRerun = 1;
 		assert(v-&gt;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-&gt;pErr);
-		sqlDbFree(db, v-&gt;zErrMsg);
-		if (!db-&gt;mallocFailed) {
-			v-&gt;zErrMsg = sqlDbStrDup(db, zErr);
-			v-&gt;rc = rc2;
-		} else {
-			v-&gt;zErrMsg = 0;
-			v-&gt;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 619b211..3f573d0 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-&gt;db, p-&gt;zErrMsg);
-	va_start(ap, zFormat);
-	p-&gt;zErrMsg = sqlVMPrintf(p-&gt;db, zFormat, ap);
-	va_end(ap);
-}
-
-/*
  * Remember the SQL string for a prepared statement.
  */
 void
@@ -2115,10 +2102,11 @@ sqlVdbeCheckFk(Vdbe * p, int deferred)
 	if ((deferred &amp;&amp; txn != NULL &amp;&amp; txn-&gt;psql_txn != NULL &amp;&amp;
 	     txn-&gt;psql_txn-&gt;fk_deferred_count &gt; 0) ||
 	    (!deferred &amp;&amp; p-&gt;nFkConstraint &gt; 0)) {
-		p-&gt;rc = SQL_CONSTRAINT_FOREIGNKEY;
+		p-&gt;rc = SQL_TARANTOOL_ERROR;
 		p-&gt;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;
 }
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 585dc21..d6375a6 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -210,7 +210,6 @@ sqlVdbeMemMakeWriteable(Mem * pMem)
  * If the given Mem* has a zero-filled tail, turn it into an ordinary
  * blob stored in dynamically allocated space.
  */
-#ifndef SQL_OMIT_INCRBLOB
 int
 sqlVdbeMemExpandBlob(Mem * pMem)
 {
@@ -232,7 +231,6 @@ sqlVdbeMemExpandBlob(Mem * pMem)
 	pMem-&gt;flags &amp;= ~(MEM_Zero | MEM_Term);
 	return SQL_OK;
 }
-#endif
 
 /*
  * It is already known that pMem contains an unterminated string.
@@ -315,8 +313,7 @@ sqlVdbeMemStringify(Mem * pMem, u8 bForce)
  * This routine calls the finalize method for that function.  The
  * result of the aggregate is stored back into pMem.
  *
- * Return SQL_ERROR if the finalizer reports an error.  SQL_OK
- * otherwise.
+ * Return -1 if the finalizer reports an error. 0 otherwise.
  */
 int
 sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
@@ -337,9 +334,10 @@ sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
 		if (pMem-&gt;szMalloc &gt; 0)
 			sqlDbFree(pMem-&gt;db, pMem-&gt;zMalloc);
 		memcpy(pMem, &amp;t, sizeof(t));
-		return ctx.is_aborted ? SQL_TARANTOOL_ERROR : SQL_OK;
+		if (ctx.is_aborted)
+			return -1;
 	}
-	return SQL_OK;
+	return 0;
 }
 
 /*
diff --git a/test/sql-tap/gh-2931-savepoints.test.lua b/test/sql-tap/gh-2931-savepoints.test.lua
index 6123fc4..be499b6 100755
--- a/test/sql-tap/gh-2931-savepoints.test.lua
+++ b/test/sql-tap/gh-2931-savepoints.test.lua
@@ -39,7 +39,7 @@ local testcases = {
 		{0,{1,1}}},
 	{"5",
 		[[rollback to savepoint s1_2;]],
-		{1, "Failed to execute SQL statement: no such savepoint: S1_2"}},
+		{1, "Can not rollback to savepoint: the savepoint does not exist"}},
 	{"6",
 		[[insert into t1 values(2);
 		select * from t1 union all select * from t2;]],
diff --git a/test/sql/savepoints.result b/test/sql/savepoints.result
index bb4a296..d20e0ed 100644
--- a/test/sql/savepoints.result
+++ b/test/sql/savepoints.result
@@ -67,7 +67,7 @@ end;
 ...
 release_sv_fail();
 ---
-- error: 'Failed to execute SQL statement: no such savepoint: T1'
+- error: 'Can not rollback to savepoint: the savepoint does not exist'
 ...
 box.commit();
 ---




</pre>
    </blockquote>
  </body>
</html>

--------------BBA6F7D93F42705AAF51EAFB--