Fixed commit-message: After this patch, all errors in VDBE will be set using diag_set(). Closes #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 - 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->p1nCursor); > - 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->p2nOp); > 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(); > --- > > > >