[tarantool-patches] Re: [PATCH v1 07/12] sql: set errors in VDBE using diag_set()
Imeev Mergen
imeevma at tarantool.org
Sat May 25 13:36:00 MSK 2019
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 at 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();
> ---
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20190525/ef13f954/attachment.html>
More information about the Tarantool-patches
mailing list