From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 054CA2DAFE for ; Mon, 3 Jun 2019 08:20:25 -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 FCP3SNCjlUJ2 for ; Mon, 3 Jun 2019 08:20:24 -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 8711B2DAAE for ; Mon, 3 Jun 2019 08:20:23 -0400 (EDT) Date: Mon, 3 Jun 2019 15:20:20 +0300 From: Mergen Imeev Subject: [tarantool-patches] Re: [PATCH v1 9/9] sql: set errors in VDBE using diag_set() Message-ID: <20190603122019.GD24317@tarantool.org> References: <2fb295d4-473a-eb96-1f32-c12b2a0e96aa@tarantool.org> <20190603121010.GC24317@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20190603121010.GC24317@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Vladislav Shpilevoy Cc: tarantool-patches@freelists.org Did not include a new patch in the previous letter, sorry. New patch below. On Mon, Jun 03, 2019 at 03:10:10PM +0300, Mergen Imeev wrote: > On Sun, Jun 02, 2019 at 06:34:12PM +0200, Vladislav Shpilevoy wrote: > > > > > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > > > index 43d7329..5bf5e6e 100644 > > > --- a/src/box/sql/vdbe.c > > > +++ b/src/box/sql/vdbe.c > > > @@ -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; > > > > In all the similar places you remove SQL_TARANTOOL_ERROR, > > but here you added it. Why? > > > I thought the next command was doing something useful and > should be executed in any case. Now I see that I was wrong. > > > > VdbeBranchTaken(res!=0,2); > > > if (rc) goto abort_due_to_error; > > > if (res) goto jump_to_p2; > > > @@ -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; > > > > The same. > > > As in the previous case, I thought that pC->cacheStatus > should be set, but now I see that it is checked only in > OP_Column, and in case of an error, it will not be > checked at all. > > > Consider my review fixes on the branch and below. They are motivated > > by several points: > > > > 1) in the original code there were places comparing 'rc' with 0, but > > you replaced them with 'rc ==/!= SQL_OK'. I rolled back such changes, > > because we move towards removal of SQL_OK. So when you need to choose > > between 0 and SQL_OK - use 0. > > > > 2) there were places using SQL_OK, but very easy to fix and compare > > with 0. I did it. > > > > 3) in a couple of places you kept 'rc = SQL_TARANTOOL_ERROR'. I don't > > know why, but I dropped it, and the tests passed. If I was wrong, tell > > me, please. > > > Thank you, I squashed your patch. Actually, all of SQL > errcodes are removed in the follow-up patch-set. > New patch: >From 1bbdbe48b600e93ef88d8cfb0b5264e5ec41c753 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(). Closes #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.c b/src/box/sql.c index 59a8bfb..d4d9d96 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -325,21 +325,19 @@ int tarantoolsqlMovetoUnpacked(BtCursor *pCur, UnpackedRecord *pIdxKey, * * @retval SQL_OK */ -int tarantoolsqlEphemeralCount(struct BtCursor *pCur, i64 *pnEntry) +int64_t +tarantoolsqlEphemeralCount(struct BtCursor *pCur) { assert(pCur->curFlags & BTCF_TEphemCursor); - struct index *primary_index = space_index(pCur->space, 0 /* PK */); - *pnEntry = index_count(primary_index, pCur->iter_type, NULL, 0); - return SQL_OK; + return index_count(primary_index, pCur->iter_type, NULL, 0); } -int tarantoolsqlCount(BtCursor *pCur, i64 *pnEntry) +int64_t +tarantoolsqlCount(struct BtCursor *pCur) { assert(pCur->curFlags & BTCF_TaCursor); - - *pnEntry = index_count(pCur->index, pCur->iter_type, NULL, 0); - return SQL_OK; + return index_count(pCur->index, pCur->iter_type, NULL, 0); } struct space * @@ -621,12 +619,12 @@ int tarantoolsqlRenameTrigger(const char *trig_name, char *key_begin = (char*) region_alloc(&fiber()->gc, key_len); if (key_begin == NULL) { diag_set(OutOfMemory, key_len, "region_alloc", "key_begin"); - return SQL_TARANTOOL_ERROR; + return -1; } char *key = mp_encode_array(key_begin, 1); key = mp_encode_str(key, trig_name, trig_name_len); if (box_index_get(BOX_TRIGGER_ID, 0, key_begin, key, &tuple) != 0) - return SQL_TARANTOOL_ERROR; + return -1; assert(tuple != NULL); assert(tuple_field_count(tuple) == 3); const char *field = tuple_field(tuple, BOX_TRIGGER_FIELD_SPACE_ID); @@ -645,7 +643,7 @@ int tarantoolsqlRenameTrigger(const char *trig_name, if (trigger_stmt == NULL) { diag_set(OutOfMemory, trigger_stmt_len + 1, "region_alloc", "trigger_stmt"); - return SQL_TARANTOOL_ERROR; + return -1; } memcpy(trigger_stmt, trigger_stmt_old, trigger_stmt_len); trigger_stmt[trigger_stmt_len] = '\0'; @@ -662,7 +660,7 @@ int tarantoolsqlRenameTrigger(const char *trig_name, char *new_tuple = (char*)region_alloc(&fiber()->gc, key_len); if (new_tuple == NULL) { diag_set(OutOfMemory, key_len, "region_alloc", "new_tuple"); - return SQL_TARANTOOL_ERROR; + return -1; } char *new_tuple_end = mp_encode_array(new_tuple, 3); new_tuple_end = mp_encode_str(new_tuple_end, trig_name, trig_name_len); @@ -672,15 +670,12 @@ int tarantoolsqlRenameTrigger(const char *trig_name, new_tuple_end = mp_encode_str(new_tuple_end, trigger_stmt, trigger_stmt_new_len); - if (box_replace(BOX_TRIGGER_ID, new_tuple, new_tuple_end, NULL) != 0) - return SQL_TARANTOOL_ERROR; - else - return SQL_OK; + return box_replace(BOX_TRIGGER_ID, new_tuple, new_tuple_end, NULL); rename_fail: diag_set(ClientError, ER_SQL_EXECUTE, "can't modify name of space " "created not via SQL facilities"); - return SQL_TARANTOOL_ERROR; + return -1; } int @@ -695,7 +690,7 @@ sql_rename_table(uint32_t space_id, const char *new_name) char *raw = (char *) region_alloc(region, size); if (raw == NULL) { diag_set(OutOfMemory, size, "region_alloc", "raw"); - return SQL_TARANTOOL_ERROR; + return -1; } /* Encode key. */ char *pos = mp_encode_array(raw, 1); @@ -709,7 +704,7 @@ sql_rename_table(uint32_t space_id, const char *new_name) pos = mp_encode_uint(pos, BOX_SPACE_FIELD_NAME); pos = mp_encode_str(pos, new_name, name_len); if (box_update(BOX_SPACE_ID, 0, raw, ops, ops, pos, 0, NULL) != 0) - return SQL_TARANTOOL_ERROR; + return -1; return 0; } @@ -834,8 +829,8 @@ tarantoolsqlIncrementMaxid(uint64_t *space_max_id) request.space_id = space_schema->def->id; if (box_process_rw(&request, space_schema, &res) != 0 || res == NULL || tuple_field_u64(res, 1, space_max_id) != 0) - return SQL_TARANTOOL_ERROR; - return SQL_OK; + return -1; + return 0; } /* diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c index 9795429..4106bce 100644 --- a/src/box/sql/analyze.c +++ b/src/box/sql/analyze.c @@ -1691,7 +1691,7 @@ sql_analysis_load(struct sql *db) assert(stat_space != NULL); ssize_t index_count = box_index_len(stat_space->def->id, 0); if (index_count < 0) - return SQL_TARANTOOL_ERROR; + return -1; if (box_txn_begin() != 0) goto fail; size_t stats_size = index_count * sizeof(struct index_stat); @@ -1712,7 +1712,7 @@ sql_analysis_load(struct sql *db) goto fail; if (info.index_count == 0) { box_txn_commit(); - return SQL_OK; + return 0; } /* * This query is used to allocate enough memory for @@ -1768,12 +1768,9 @@ sql_analysis_load(struct sql *db) */ const char *order_query = "SELECT \"tbl\",\"idx\" FROM " "\"_sql_stat4\" GROUP BY \"tbl\",\"idx\""; - if (load_stat_to_index(db, order_query, heap_stats) != 0) - goto fail; - if (box_txn_commit() != 0) - return SQL_TARANTOOL_ERROR; - return SQL_OK; + if (load_stat_to_index(db, order_query, heap_stats) == 0) + return box_txn_commit(); fail: box_txn_rollback(); - return SQL_TARANTOOL_ERROR; + return -1; } diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 9c0659b..7775278 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4591,7 +4591,8 @@ sql_index_tuple_size(struct space *space, struct index *idx); * samples[] arrays. * * @param db Database handler. - * @retval sql_OK on success, smth else otherwise. + * @retval 0 Success. + * @retval -1 Error. */ int sql_analysis_load(struct sql *db); diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h index f15e147..0474f29 100644 --- a/src/box/sql/tarantoolInt.h +++ b/src/box/sql/tarantoolInt.h @@ -34,7 +34,8 @@ int tarantoolsqlNext(BtCursor * pCur, int *pRes); int tarantoolsqlPrevious(BtCursor * pCur, int *pRes); int tarantoolsqlMovetoUnpacked(BtCursor * pCur, UnpackedRecord * pIdxKey, int *pRes); -int tarantoolsqlCount(BtCursor * pCur, i64 * pnEntry); +int64_t +tarantoolsqlCount(struct BtCursor *pCur); int tarantoolsqlInsert(struct space *space, const char *tuple, const char *tuple_end); int tarantoolsqlReplace(struct space *space, const char *tuple, @@ -62,7 +63,8 @@ int tarantoolsqlClearTable(struct space *space, uint32_t *tuple_count); * @param space_id Table's space identifier. * @param new_name new name of table * - * @retval 0 on success, SQL_TARANTOOL_ERROR otherwise. + * @retval 0 Success. + * @retval -1 Error. */ int sql_rename_table(uint32_t space_id, const char *new_name); @@ -100,7 +102,8 @@ sql_ephemeral_space_create(uint32_t filed_count, struct sql_key_info *key_info); int tarantoolsqlEphemeralInsert(struct space *space, const char *tuple, const char *tuple_end); int tarantoolsqlEphemeralDelete(BtCursor * pCur); -int tarantoolsqlEphemeralCount(BtCursor * pCur, i64 * pnEntry); +int64_t +tarantoolsqlEphemeralCount(struct BtCursor *pCur); int tarantoolsqlEphemeralDrop(BtCursor * pCur); int tarantoolsqlEphemeralClearTable(BtCursor * pCur); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index b5909b6..86ad180 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -961,7 +961,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; } @@ -1073,17 +1073,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; @@ -1310,7 +1306,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; } @@ -1402,7 +1398,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; } @@ -1410,10 +1405,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) != 0 || ExpandBlob(pIn2) != 0) + goto abort_due_to_error; nByte = pIn1->n + pIn2->n; if (nByte>db->aLimit[SQL_LIMIT_LENGTH]) { goto too_big; @@ -1526,13 +1521,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) { @@ -1572,11 +1565,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; } @@ -1690,10 +1681,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)) { @@ -1754,13 +1743,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; @@ -1829,7 +1816,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; @@ -1875,8 +1861,7 @@ case OP_Realify: { /* in1 */ */ case OP_Cast: { /* in1 */ pIn1 = &aMem[pOp->p1]; - rc = ExpandBlob(pIn1); - if (rc != 0) + if (ExpandBlob(pIn1) != 0) goto abort_due_to_error; rc = sqlVdbeMemCast(pIn1, pOp->p2); UPDATE_MAX_BLOBSIZE(pIn1); @@ -1884,7 +1869,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 */ @@ -2037,7 +2021,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); @@ -2056,7 +2039,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; } @@ -2298,7 +2280,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]; @@ -2309,7 +2290,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) { @@ -2343,7 +2323,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); @@ -2367,7 +2346,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; @@ -2415,7 +2393,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); @@ -2555,8 +2532,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; } @@ -2615,10 +2593,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. */ @@ -2652,7 +2628,11 @@ case OP_Column: { if ((pDest->flags & (MEM_Ephem | MEM_Str)) == (MEM_Ephem | MEM_Str)) { int len = pDest->n; if (pDest->szMallocaRow) + sqlVdbeMemRelease(&sMem); + goto abort_due_to_error; + } } else { pDest->z = memcpy(pDest->zMalloc, pDest->z, len); pDest->flags &= ~MEM_Ephem; @@ -2666,10 +2646,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 * @@ -2694,7 +2670,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++; @@ -2767,10 +2742,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; @@ -2786,7 +2759,8 @@ case OP_MakeRecord: { * routine. */ if (bIsEphemeral) { - rc = sqlVdbeMemClearAndResize(pOut, tuple_size); + if (sqlVdbeMemClearAndResize(pOut, tuple_size) != 0) + goto abort_due_to_error; pOut->flags = MEM_Blob; pOut->n = tuple_size; memcpy(pOut->z, tuple, tuple_size); @@ -2801,8 +2775,6 @@ case OP_MakeRecord: { pOut->n = tuple_size; pOut->z = tuple; } - if (rc) - goto no_mem; assert(sqlVdbeCheckMemInvariants(pOut)); assert(pOp->p3>0 && pOp->p3<=(p->nMem+1 - p->nCursor)); REGISTER_TRACE(pOp->p3, pOut); @@ -2823,15 +2795,12 @@ case OP_Count: { /* out2 */ assert(p->apCsr[pOp->p1]->eCurType==CURTYPE_TARANTOOL); pCrsr = p->apCsr[pOp->p1]->uc.pCursor; assert(pCrsr); - nEntry = 0; /* Not needed. Only used to silence a warning. */ if (pCrsr->curFlags & BTCF_TaCursor) { - rc = tarantoolsqlCount(pCrsr, &nEntry); - } else if (pCrsr->curFlags & BTCF_TEphemCursor) { - rc = tarantoolsqlEphemeralCount(pCrsr, &nEntry); + nEntry = tarantoolsqlCount(pCrsr); } else { - unreachable(); + assert((pCrsr->curFlags & BTCF_TEphemCursor) != 0); + nEntry = tarantoolsqlEphemeralCount(pCrsr); } - if (rc) goto abort_due_to_error; pOut = out2Prerelease(p, pOp); pOut->u.i = nEntry; break; @@ -2855,7 +2824,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; @@ -2883,8 +2851,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, @@ -2901,7 +2869,8 @@ case OP_Savepoint: { p->rc = rc = SQL_BUSY; goto vdbe_return; } - rc = p->rc; + if (p->rc != 0) + goto abort_due_to_error; } else { if (p1==SAVEPOINT_ROLLBACK) box_txn_rollback_to_savepoint(pSavepoint->tnt_savepoint); @@ -2933,7 +2902,6 @@ case OP_Savepoint: { } } } - if (rc) goto abort_due_to_error; break; } @@ -2956,7 +2924,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; @@ -2969,10 +2936,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; } @@ -2988,13 +2953,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; @@ -3007,14 +2970,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; @@ -3032,16 +2992,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; } @@ -3080,9 +3036,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; @@ -3091,10 +3046,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); @@ -3117,8 +3070,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; } @@ -3140,10 +3091,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; @@ -3169,8 +3118,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) != 0) + goto abort_due_to_error; break; } @@ -3402,7 +3351,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; @@ -3483,10 +3431,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; @@ -3498,8 +3444,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; } @@ -3507,8 +3453,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. @@ -3650,10 +3596,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; @@ -3713,10 +3660,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 @@ -3725,7 +3670,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]; @@ -3871,10 +3815,9 @@ case OP_SorterCompare: { assert(pOp->p4type==P4_INT32); pIn3 = &aMem[pOp->p3]; nKeyCol = pOp->p4.i; - res = 0; - rc = sqlVdbeSorterCompare(pC, pIn3, nKeyCol, &res); + if (sqlVdbeSorterCompare(pC, pIn3, nKeyCol, &res) != 0) + goto abort_due_to_error; VdbeBranchTaken(res!=0,2); - if (rc) goto abort_due_to_error; if (res) goto jump_to_p2; break; }; @@ -3897,10 +3840,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) != 0) + 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; } @@ -3967,11 +3910,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) != 0 || + sqlCursorPayload(pCrsr, 0, n, pOut->z) != 0) + goto abort_due_to_error; UPDATE_MAX_BLOBSIZE(pOut); REGISTER_TRACE(pOp->p2, pOut); break; @@ -4034,10 +3975,10 @@ case OP_Last: { /* jump */ pC->seekOp = OP_Last; #endif if (pOp->p3==0 || !sqlCursorIsValidNN(pCrsr)) { - rc = tarantoolsqlLast(pCrsr, &res); + if (tarantoolsqlLast(pCrsr, &res) != 0) + goto abort_due_to_error; pC->nullRow = (u8)res; pC->cacheStatus = CACHE_STALE; - if (rc) goto abort_due_to_error; if (pOp->p2>0) { VdbeBranchTaken(res!=0,2); if (res) goto jump_to_p2; @@ -4106,15 +4047,16 @@ case OP_Rewind: { /* jump */ pC->seekOp = OP_Rewind; #endif if (isSorter(pC)) { - rc = sqlVdbeSorterRewind(pC, &res); + if (sqlVdbeSorterRewind(pC, &res) != 0) + 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) != 0) + goto abort_due_to_error; pC->cacheStatus = CACHE_STALE; } - if (rc) goto abort_due_to_error; pC->nullRow = (u8)res; assert(pOp->p2>0 && pOp->p2nOp); VdbeBranchTaken(res!=0,2); @@ -4197,7 +4139,8 @@ case OP_SorterNext: { /* jump */ pC = p->apCsr[pOp->p1]; assert(isSorter(pC)); res = 0; - rc = sqlVdbeSorterNext(db, pC, &res); + if (sqlVdbeSorterNext(db, pC, &res) != 0) + goto abort_due_to_error; goto next_tail; case OP_PrevIfOpen: /* jump */ case OP_NextIfOpen: /* jump */ @@ -4228,11 +4171,11 @@ case OP_Next: /* jump */ || pC->seekOp==OP_SeekLT || pC->seekOp==OP_SeekLE || pC->seekOp==OP_Last); - rc = pOp->p4.xAdvance(pC->uc.pCursor, &res); + if (pOp->p4.xAdvance(pC->uc.pCursor, &res) != 0) + goto abort_due_to_error; next_tail: pC->cacheStatus = CACHE_STALE; VdbeBranchTaken(res==0,2); - if (rc) goto abort_due_to_error; if (res==0) { pC->nullRow = 0; p->aCounter[pOp->p5]++; @@ -4260,11 +4203,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) != 0 || + sqlVdbeSorterWrite(cursor, pIn2) != 0) goto abort_due_to_error; break; } @@ -4294,8 +4234,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) != 0) goto abort_due_to_error; struct space *space; if (pOp->p4type == P4_SPACEPTR) @@ -4329,6 +4268,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; @@ -4396,14 +4336,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; } @@ -4429,6 +4367,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; @@ -4479,8 +4418,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) != 0) goto abort_due_to_error; if (pOp->p5 & OPFLAG_NCHANGE) p->nChange++; @@ -4514,18 +4452,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) != 0) + 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) != 0) + goto abort_due_to_error; } else if (pCrsr->curFlags & BTCF_TEphemCursor) { - rc = tarantoolsqlEphemeralDelete(pCrsr); + if (tarantoolsqlEphemeralDelete(pCrsr) != 0) + goto abort_due_to_error; } else { unreachable(); } - if (rc) goto abort_due_to_error; } pC->cacheStatus = CACHE_STALE; pC->seekResult = 0; @@ -4634,17 +4573,16 @@ case OP_Clear: { uint32_t space_id = pOp->p1; struct space *space = space_by_id(space_id); assert(space != NULL); - 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) != 0) + goto abort_due_to_error; + if ((pOp->p5 & OPFLAG_NCHANGE) != 0) p->nChange += tuple_count; } - if (rc) goto abort_due_to_error; break; } @@ -4667,8 +4605,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) != 0) + goto abort_due_to_error; } break; } @@ -4701,8 +4639,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) != 0) + goto abort_due_to_error; /* * Rebuild 'CREATE TRIGGER' expressions of all triggers * created on this table. Sure, this action is not atomic @@ -4712,20 +4650,15 @@ 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) != 0) goto abort_due_to_error; - } trigger = next_trigger; } sqlDbFree(db, (void*)zOldTableName); @@ -4740,8 +4673,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) != 0) + goto abort_due_to_error; break; } @@ -4797,8 +4730,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; } @@ -5126,11 +5059,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; @@ -5157,11 +5088,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; @@ -5270,10 +5198,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) != 0) goto abort_due_to_error; - } pOut->flags = MEM_Int; break; } @@ -5337,28 +5263,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: @@ -5367,21 +5273,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 312c661..ea6afa6 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(); ---