[tarantool-patches] Re: [PATCH v1 9/9] sql: set errors in VDBE using diag_set()
Mergen Imeev
imeevma at tarantool.org
Mon Jun 3 15:20:20 MSK 2019
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->szMalloc<len+1) {
- if (sqlVdbeMemGrow(pDest, len+1, 1)) goto op_column_error;
+ if (sqlVdbeMemGrow(pDest, len + 1, 1) != 0) {
+ if (zData != pC->aRow)
+ 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->p1<p->nCursor);
- 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->p2<p->nOp);
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();
---
More information about the Tarantool-patches
mailing list