<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Fixed commit-message:</p>
<pre class="text-small">After this patch, all errors in VDBE will be set using diag_set().
<span class="issue-keyword tooltipped tooltipped-se">Closes</span> <a class="issue-link js-issue-link tooltipped tooltipped-ne" data-error-text="Failed to load issue title" data-id="424499021" data-permission-text="Issue title is private" data-hovercard-type="issue" data-hovercard-url="/tarantool/tarantool/issues/4074/hovercard" href="https://github.com/tarantool/tarantool/issues/4074">#4074</a>
</pre>
<div class="moz-cite-prefix">On 5/25/19 1:24 PM, Mergen Imeev wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20190525102431.GB15670@tarantool.org">
<pre class="moz-quote-pre" wrap="">I moved patch "sql: use diag_set() to set an error in SQL
functions" to position before this patch. It allowed to
simplify this patch. New patch below.
On Wed, May 15, 2019 at 04:26:54PM +0300, n.pettik wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On 5 May 2019, at 15:17, <a class="moz-txt-link-abbreviated" href="mailto:imeevma@tarantool.org">imeevma@tarantool.org</a> wrote:
After this patch, all errors in VDBE will be set using diag_set().
Part of #4074
---
src/box/execute.c | 23 +---
src/box/sql/vdbe.c | 331 +++++++++++++++++++++-----------------------------
src/box/sql/vdbeInt.h | 10 --
src/box/sql/vdbeapi.c | 34 +-----
src/box/sql/vdbeaux.c | 20 +--
5 files changed, 148 insertions(+), 270 deletions(-)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Please, remove whole sqlErrStr(), tarantoolErrorMessage() -
they are unused now. The same concerns sql_ret_code() -
I see no reason keeping it. Please, remove all legacy routines
connected with error codes.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">These functions are removed in the following patches.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">diff --git a/src/box/execute.c b/src/box/execute.c
index a3d4a92..e81cc32 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -1335,7 +1331,8 @@ case OP_ResultRow: {
* not return the number of rows modified. And do not RELEASE the statement
* transaction. It needs to be rolled back.
*/
- if (SQL_OK!=(rc = sqlVdbeCheckFk(p, 0))) {
+ rc = sqlVdbeCheckFk(p, 0);
+ if (rc != SQL_OK) {
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
-> if (sqlVdbeCheckFk() != 0)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Fixed. I used SQL_OK but it will be replaced by 0 in the
following patches.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> assert(user_session->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;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
!= 0
Still need to call sqlOomFault(db); by jumping to no_mem label.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Fixed.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> nByte = pIn1->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) {
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
How it can be different from SQL_TARANTOOL_ERROR?
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">It was possible, but this part of code was completely
changed after i moved patch "sql: use diag_set() to set an
error in SQL functions" to position before this patch.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ diag_set(ClientError, ER_SQL_EXECUTE,
+ sql_value_text(pCtx->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;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
This diff seems to be redundant
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Fixed.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">@@ -1910,8 +1903,7 @@ case OP_Realify: { /* in1 */
*/
case OP_Cast: { /* in1 */
pIn1 = &aMem[pOp->p1];
- rc = ExpandBlob(pIn1);
- if (rc != 0)
+ if (ExpandBlob(pIn1) != SQL_OK)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
!= 0
Please, don’t use SQL_OK value anywhere.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">I think it's better to use SQL_OK here for integrity. It is
replaced with 0 in one of the following patches.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
@@ -2802,10 +2784,8 @@ case OP_MakeRecord: {
uint32_t tuple_size;
char *tuple =
sql_vdbe_mem_encode_tuple(pData0, nField, &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);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
We already have ER_NO_SUCH_SAVEPOINT.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Fixed.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">@@ -3685,10 +3645,11 @@ case OP_Found: { /* jump, in3 */
}
}
rc = sqlCursorMovetoUnpacked(pC->uc.pCursor, pIdxKey, &res);
- if (pFree) sqlDbFree(db, pFree);
- if (rc!=SQL_OK) {
+ if (pFree)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
!= NULL
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Fixed.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
@@ -5164,11 +5126,16 @@ case OP_AggStep: {
(pCtx->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;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Looks like redundant diff
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Fixed.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> } else {
assert(t.flags==MEM_Null);
}
@@ -5199,8 +5166,11 @@ case OP_AggFinal: {
pMem = &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));
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Could you please clarify what does this error mean?
It would just print value of memory to string…
The same relates to error in OP_AggStep.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">If result was returned using sql_result_error() then it was
possible that diag is not set. This was fixed in patch that
now right before this patch.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">@@ -5311,10 +5281,8 @@ case OP_IncMaxid: {
assert(pOp->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”);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
-> is too big
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Fixed here. Still, it is possible to find this error in
other places. I think it will be fixed when we create error
code for such errors.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> goto abort_due_to_error;
/* Jump to here if a malloc() fails.
*/
no_mem:
sqlOomFault(db);
- sqlVdbeError(p, "out of memory");
- rc = SQL_NOMEM;
goto abort_due_to_error;
}
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index a3100e5..b655b5a 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -248,10 +248,6 @@ struct Mem {
#define MEM_Agg 0x4000 /* Mem.z points to an agg function context */
#define MEM_Zero 0x8000 /* Mem.i contains count of 0s appended to blob */
#define MEM_Subtype 0x10000 /* Mem.eSubtype is valid */
-#ifdef SQL_OMIT_INCRBLOB
-#undef MEM_Zero
-#define MEM_Zero 0x0000
-#endif
@@ -550,13 +545,8 @@ void sqlVdbeMemPrettyPrint(Mem * pMem, char *zBuf);
#endif
int sqlVdbeMemHandleBom(Mem * pMem);
-#ifndef SQL_OMIT_INCRBLOB
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Still see usage of this macro in code: vdbemem.c : 213
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Fixed.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">int sqlVdbeMemExpandBlob(Mem *);
#define ExpandBlob(P) (((P)->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”);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Please, reserve separate error code for this violation.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">I am going to do this a bit later, in a different patch.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ return SQL_TARANTOOL_ERROR;
}
return SQL_OK;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Return 0/-1
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Left as it as for now, will be fixed in following patches.
New patch:
>From 85cfbe96609b66379631c8d4534c8a9329fb3a47 Mon Sep 17 00:00:00 2001
Date: Mon, 22 Apr 2019 19:41:46 +0300
Subject: [PATCH] sql: set errors in VDBE using diag_set()
After this patch, all errors in VDBE will be set using diag_set().
Part of #4074
diff --git a/src/box/execute.c b/src/box/execute.c
index a3d4a92..e81cc32 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -410,8 +410,7 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
* @retval -1 Error.
*/
static inline int
-sql_execute(sql *db, struct sql_stmt *stmt, struct port *port,
- struct region *region)
+sql_execute(struct sql_stmt *stmt, struct port *port, struct region *region)
{
int rc, column_count = sql_column_count(stmt);
if (column_count > 0) {
@@ -427,15 +426,8 @@ sql_execute(sql *db, struct sql_stmt *stmt, struct port *port,
rc = sql_step(stmt);
assert(rc != SQL_ROW && rc != SQL_OK);
}
- if (rc != SQL_DONE) {
- if (db->errCode != SQL_TARANTOOL_ERROR) {
- const char *err = (char *)sql_value_text(db->pErr);
- if (err == NULL)
- err = sqlErrStr(db->errCode);
- diag_set(ClientError, ER_SQL_EXECUTE, err);
- }
+ if (rc != SQL_DONE)
return -1;
- }
return 0;
}
@@ -446,19 +438,12 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
{
struct sql_stmt *stmt;
struct sql *db = sql_get();
- if (sql_prepare_v2(db, sql, len, &stmt, NULL) != SQL_OK) {
- if (db->errCode != SQL_TARANTOOL_ERROR) {
- const char *err = (char *)sql_value_text(db->pErr);
- if (err == NULL)
- err = sqlErrStr(db->errCode);
- diag_set(ClientError, ER_SQL_EXECUTE, err);
- }
+ if (sql_prepare_v2(db, sql, len, &stmt, NULL) != SQL_OK)
return -1;
- }
assert(stmt != NULL);
port_sql_create(port, stmt);
if (sql_bind(stmt, bind, bind_count) == 0 &&
- sql_execute(db, stmt, port, region) == 0)
+ sql_execute(stmt, port, region) == 0)
return 0;
port_destroy(port);
return -1;
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 7d85959..b64293a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -986,7 +986,7 @@ case OP_Halt: {
p->rc = SQL_BUSY;
} else {
assert(rc==SQL_OK || (p->rc&0xff)==SQL_CONSTRAINT);
- rc = p->rc ? SQL_ERROR : SQL_DONE;
+ rc = p->rc ? SQL_TARANTOOL_ERROR : SQL_DONE;
}
goto vdbe_return;
}
@@ -1098,17 +1098,13 @@ case OP_NextAutoincValue: {
assert(pOp->p2 > 0);
struct space *space = space_by_id(pOp->p1);
- if (space == NULL) {
- rc = SQL_TARANTOOL_ERROR;
+ if (space == NULL)
goto abort_due_to_error;
- }
int64_t value;
struct sequence *sequence = space->sequence;
- if (sequence == NULL || sequence_next(sequence, &value) != 0) {
- rc = SQL_TARANTOOL_ERROR;
+ if (sequence == NULL || sequence_next(sequence, &value) != 0)
goto abort_due_to_error;
- }
pOut = out2Prerelease(p, pOp);
pOut->flags = MEM_Int;
@@ -1335,7 +1331,7 @@ case OP_ResultRow: {
* not return the number of rows modified. And do not RELEASE the statement
* transaction. It needs to be rolled back.
*/
- if (SQL_OK!=(rc = sqlVdbeCheckFk(p, 0))) {
+ if (sqlVdbeCheckFk(p, 0) != SQL_OK) {
assert(user_session->sql_flags&SQL_CountRows);
goto abort_due_to_error;
}
@@ -1427,7 +1423,6 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */
mem_type_to_str(pIn2);
diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT or BLOB",
inconsistent_type);
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
@@ -1435,10 +1430,10 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */
if (str_type_p1 != str_type_p2) {
diag_set(ClientError, ER_INCONSISTENT_TYPES,
mem_type_to_str(pIn2), mem_type_to_str(pIn1));
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
- if (ExpandBlob(pIn1) || ExpandBlob(pIn2)) goto no_mem;
+ if (ExpandBlob(pIn1) != SQL_OK || ExpandBlob(pIn2) != SQL_OK)
+ goto abort_due_to_error;
nByte = pIn1->n + pIn2->n;
if (nByte>db->aLimit[SQL_LIMIT_LENGTH]) {
goto too_big;
@@ -1551,13 +1546,11 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */
if (sqlVdbeRealValue(pIn1, &rA) != 0) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn1), "numeric");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
if (sqlVdbeRealValue(pIn2, &rB) != 0) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn2), "numeric");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
switch( pOp->opcode) {
@@ -1597,11 +1590,9 @@ arithmetic_result_is_null:
division_by_zero:
diag_set(ClientError, ER_SQL_EXECUTE, "division by zero");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
integer_overflow:
diag_set(ClientError, ER_SQL_EXECUTE, "integer is overflowed");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
@@ -1721,10 +1712,8 @@ case OP_Function: {
(*pCtx->pFunc->xSFunc)(pCtx, pCtx->argc, pCtx->argv);/* IMP: R-24505-23230 */
/* If the function returned an error, throw an exception */
- if (pCtx->is_aborted) {
- rc = SQL_TARANTOOL_ERROR;
+ if (pCtx->is_aborted)
goto abort_due_to_error;
- }
/* Copy the result of the function into register P3 */
if (pOut->flags & (MEM_Str|MEM_Blob)) {
@@ -1785,13 +1774,11 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */
if (sqlVdbeIntValue(pIn2, (int64_t *) &iA) != 0) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn2), "integer");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
if (sqlVdbeIntValue(pIn1, (int64_t *) &iB) != 0) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn1), "integer");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
op = pOp->opcode;
@@ -1860,7 +1847,6 @@ case OP_MustBeInt: { /* jump, in1 */
if (pOp->p2==0) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn1), "integer");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
} else {
goto jump_to_p2;
@@ -1906,8 +1892,7 @@ case OP_Realify: { /* in1 */
*/
case OP_Cast: { /* in1 */
pIn1 = &aMem[pOp->p1];
- rc = ExpandBlob(pIn1);
- if (rc != 0)
+ if (ExpandBlob(pIn1) != SQL_OK)
goto abort_due_to_error;
rc = sqlVdbeMemCast(pIn1, pOp->p2);
UPDATE_MAX_BLOBSIZE(pIn1);
@@ -1915,7 +1900,6 @@ case OP_Cast: { /* in1 */
break;
diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_text(pIn1),
field_type_strs[pOp->p2]);
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
#endif /* SQL_OMIT_CAST */
@@ -2068,7 +2052,6 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
mem_type_to_str(pIn3);
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
inconsistent_type, "boolean");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
res = sqlMemCompare(pIn3, pIn1, NULL);
@@ -2087,7 +2070,6 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn3),
"numeric");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
@@ -2329,7 +2311,6 @@ case OP_Or: { /* same as TK_OR, in1, in2, out3 */
} else {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn1), "boolean");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
pIn2 = &aMem[pOp->p2];
@@ -2340,7 +2321,6 @@ case OP_Or: { /* same as TK_OR, in1, in2, out3 */
} else {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn2), "boolean");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
if (pOp->opcode==OP_And) {
@@ -2374,7 +2354,6 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */
if ((pIn1->flags & MEM_Bool) == 0) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn1), "boolean");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
mem_set_bool(pOut, ! pIn1->u.b);
@@ -2398,7 +2377,6 @@ case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */
if (sqlVdbeIntValue(pIn1, &i) != 0) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn1), "integer");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
pOut->flags = MEM_Int;
@@ -2446,7 +2424,6 @@ case OP_IfNot: { /* jump, in1 */
} else {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn1), "boolean");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
VdbeBranchTaken(c!=0, 2);
@@ -2586,8 +2563,9 @@ case OP_Column: {
zEnd = zData + pC->payloadSize;
} else {
memset(&sMem, 0, sizeof(sMem));
- rc = sqlVdbeMemFromBtree(pC->uc.pCursor, 0, pC->payloadSize, &sMem);
- if (rc!=SQL_OK) goto abort_due_to_error;
+ if (sqlVdbeMemFromBtree(pC->uc.pCursor, 0, pC->payloadSize,
+ &sMem) != SQL_OK)
+ goto abort_due_to_error;
zData = (u8*)sMem.z;
zEnd = zData + pC->payloadSize;
}
@@ -2646,10 +2624,8 @@ case OP_Column: {
}
uint32_t unused;
if (vdbe_decode_msgpack_into_mem((const char *)(zData + aOffset[p2]),
- pDest, &unused) != 0) {
- rc = SQL_TARANTOOL_ERROR;
+ pDest, &unused) != 0)
goto abort_due_to_error;
- }
/* MsgPack map, array or extension (unsupported in sql).
* Wrap it in a blob verbatim.
*/
@@ -2683,7 +2659,11 @@ case OP_Column: {
if ((pDest->flags & (MEM_Ephem | MEM_Str)) == (MEM_Ephem | MEM_Str)) {
int len = pDest->n;
if (pDest->szMalloc<len+1) {
- if (sqlVdbeMemGrow(pDest, len+1, 1)) goto op_column_error;
+ if (sqlVdbeMemGrow(pDest, len + 1, 1)) {
+ if (zData != pC->aRow)
+ sqlVdbeMemRelease(&sMem);
+ goto abort_due_to_error;
+ }
} else {
pDest->z = memcpy(pDest->zMalloc, pDest->z, len);
pDest->flags &= ~MEM_Ephem;
@@ -2697,10 +2677,6 @@ case OP_Column: {
UPDATE_MAX_BLOBSIZE(pDest);
REGISTER_TRACE(pOp->p3, pDest);
break;
-
- op_column_error:
- if (zData!=pC->aRow) sqlVdbeMemRelease(&sMem);
- goto abort_due_to_error;
}
/* Opcode: ApplyType P1 P2 * P4 *
@@ -2725,7 +2701,6 @@ case OP_ApplyType: {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn1),
field_type_strs[type]);
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
pIn1++;
@@ -2798,10 +2773,8 @@ case OP_MakeRecord: {
uint32_t tuple_size;
char *tuple =
sql_vdbe_mem_encode_tuple(pData0, nField, &tuple_size, region);
- if (tuple == NULL) {
- rc = SQL_TARANTOOL_ERROR;
+ if (tuple == NULL)
goto abort_due_to_error;
- }
if ((int64_t)tuple_size > db->aLimit[SQL_LIMIT_LENGTH])
goto too_big;
@@ -2856,13 +2829,14 @@ case OP_Count: { /* out2 */
assert(pCrsr);
nEntry = 0; /* Not needed. Only used to silence a warning. */
if (pCrsr->curFlags & BTCF_TaCursor) {
- rc = tarantoolsqlCount(pCrsr, &nEntry);
+ if (tarantoolsqlCount(pCrsr, &nEntry) != SQL_OK)
+ goto abort_due_to_error;
} else if (pCrsr->curFlags & BTCF_TEphemCursor) {
- rc = tarantoolsqlEphemeralCount(pCrsr, &nEntry);
+ if (tarantoolsqlEphemeralCount(pCrsr, &nEntry) != SQL_OK)
+ goto abort_due_to_error;
} else {
unreachable();
}
- if (rc) goto abort_due_to_error;
pOut = out2Prerelease(p, pOp);
pOut->u.i = nEntry;
break;
@@ -2886,7 +2860,6 @@ case OP_Savepoint: {
if (psql_txn == NULL) {
assert(!box_txn());
diag_set(ClientError, ER_NO_TRANSACTION);
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
p1 = pOp->p1;
@@ -2914,8 +2887,8 @@ case OP_Savepoint: {
pSavepoint = pSavepoint->pNext
);
if (!pSavepoint) {
- sqlVdbeError(p, "no such savepoint: %s", zName);
- rc = SQL_ERROR;
+ diag_set(ClientError, ER_NO_SUCH_SAVEPOINT);
+ goto abort_due_to_error;
} else {
/* Determine whether or not this is a transaction savepoint. If so,
@@ -2932,7 +2905,8 @@ case OP_Savepoint: {
p->rc = rc = SQL_BUSY;
goto vdbe_return;
}
- rc = p->rc;
+ if (p->rc != SQL_OK)
+ goto abort_due_to_error;
} else {
if (p1==SAVEPOINT_ROLLBACK)
box_txn_rollback_to_savepoint(pSavepoint->tnt_savepoint);
@@ -2964,7 +2938,6 @@ case OP_Savepoint: {
}
}
}
- if (rc) goto abort_due_to_error;
break;
}
@@ -2987,7 +2960,6 @@ case OP_CheckViewReferences: {
if (space->def->view_ref_count > 0) {
diag_set(ClientError, ER_DROP_SPACE, space->def->name,
"other views depend on this space");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
break;
@@ -3000,10 +2972,8 @@ case OP_CheckViewReferences: {
* Otherwise, raise an error with appropriate error message.
*/
case OP_TransactionBegin: {
- if (sql_txn_begin(p) != 0) {
- rc = SQL_TARANTOOL_ERROR;
+ if (sql_txn_begin(p) != 0)
goto abort_due_to_error;
- }
p->auto_commit = false ;
break;
}
@@ -3019,13 +2989,11 @@ case OP_TransactionBegin: {
case OP_TransactionCommit: {
struct txn *txn = in_txn();
if (txn != NULL) {
- if (txn_commit(txn) != 0) {
- rc = SQL_TARANTOOL_ERROR;
+ if (txn_commit(txn) != 0)
goto abort_due_to_error;
- }
} else {
- sqlVdbeError(p, "cannot commit - no transaction is active");
- rc = SQL_ERROR;
+ diag_set(ClientError, ER_SQL_EXECUTE, "cannot commit - no "\
+ "transaction is active");
goto abort_due_to_error;
}
break;
@@ -3038,14 +3006,11 @@ case OP_TransactionCommit: {
*/
case OP_TransactionRollback: {
if (box_txn()) {
- if (box_txn_rollback() != 0) {
- rc = SQL_TARANTOOL_ERROR;
+ if (box_txn_rollback() != 0)
goto abort_due_to_error;
- }
} else {
- sqlVdbeError(p, "cannot rollback - no "
- "transaction is active");
- rc = SQL_ERROR;
+ diag_set(ClientError, ER_SQL_EXECUTE, "cannot rollback - no "\
+ "transaction is active");
goto abort_due_to_error;
}
break;
@@ -3063,16 +3028,12 @@ case OP_TransactionRollback: {
*/
case OP_TTransaction: {
if (!box_txn()) {
- if (sql_txn_begin(p) != 0) {
- rc = SQL_TARANTOOL_ERROR;
+ if (sql_txn_begin(p) != 0)
goto abort_due_to_error;
- }
} else {
p->anonymous_savepoint = sql_savepoint(p, NULL);
- if (p->anonymous_savepoint == NULL) {
- rc = SQL_TARANTOOL_ERROR;
+ if (p->anonymous_savepoint == NULL)
goto abort_due_to_error;
- }
}
break;
}
@@ -3111,9 +3072,8 @@ case OP_IteratorOpen:
if (box_schema_version() != p->schema_ver &&
(pOp->p5 & OPFLAG_SYSTEMSP) == 0) {
p->expired = 1;
- rc = SQL_ERROR;
- sqlVdbeError(p, "schema version has changed: " \
- "need to re-compile SQL statement");
+ diag_set(ClientError, ER_SQL_EXECUTE, "schema version has "\
+ "changed: need to re-compile SQL statement");
goto abort_due_to_error;
}
struct space *space;
@@ -3122,10 +3082,8 @@ case OP_IteratorOpen:
else
space = aMem[pOp->p3].u.p;
assert(space != NULL);
- if (access_check_space(space, PRIV_R) != 0) {
- rc = SQL_TARANTOOL_ERROR;
+ if (access_check_space(space, PRIV_R) != 0)
goto abort_due_to_error;
- }
struct index *index = space_index(space, pOp->p2);
assert(index != NULL);
@@ -3148,8 +3106,6 @@ case OP_IteratorOpen:
cur->nullRow = 1;
open_cursor_set_hints:
cur->uc.pCursor->hints = pOp->p5 & OPFLAG_SEEKEQ;
- if (rc != 0)
- goto abort_due_to_error;
break;
}
@@ -3171,10 +3127,8 @@ case OP_OpenTEphemeral: {
struct space *space = sql_ephemeral_space_create(pOp->p2,
pOp->p4.key_info);
- if (space == NULL) {
- rc = SQL_TARANTOOL_ERROR;
+ if (space == NULL)
goto abort_due_to_error;
- }
aMem[pOp->p1].u.p = space;
aMem[pOp->p1].flags = MEM_Ptr;
break;
@@ -3200,8 +3154,8 @@ case OP_SorterOpen: {
pCx = allocateCursor(p, pOp->p1, pOp->p2, CURTYPE_SORTER);
if (pCx==0) goto no_mem;
pCx->key_def = def;
- rc = sqlVdbeSorterInit(db, pCx);
- if (rc) goto abort_due_to_error;
+ if (sqlVdbeSorterInit(db, pCx) != SQL_OK)
+ goto abort_due_to_error;
break;
}
@@ -3433,7 +3387,6 @@ case OP_SeekGT: { /* jump, in3 */
} else {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_text(pIn3), "integer");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
iKey = i;
@@ -3514,10 +3467,8 @@ case OP_SeekGT: { /* jump, in3 */
#endif
r.eqSeen = 0;
r.opcode = oc;
- rc = sqlCursorMovetoUnpacked(pC->uc.pCursor, &r, &res);
- if (rc!=SQL_OK) {
+ if (sqlCursorMovetoUnpacked(pC->uc.pCursor, &r, &res) != SQL_OK)
goto abort_due_to_error;
- }
if (eqOnly && r.eqSeen==0) {
assert(res!=0);
goto seek_not_found;
@@ -3529,8 +3480,8 @@ case OP_SeekGT: { /* jump, in3 */
if (oc>=OP_SeekGE) { assert(oc==OP_SeekGE || oc==OP_SeekGT);
if (res<0 || (res==0 && oc==OP_SeekGT)) {
res = 0;
- rc = sqlCursorNext(pC->uc.pCursor, &res);
- if (rc!=SQL_OK) goto abort_due_to_error;
+ if (sqlCursorNext(pC->uc.pCursor, &res) != SQL_OK)
+ goto abort_due_to_error;
} else {
res = 0;
}
@@ -3538,8 +3489,8 @@ case OP_SeekGT: { /* jump, in3 */
assert(oc==OP_SeekLT || oc==OP_SeekLE);
if (res>0 || (res==0 && oc==OP_SeekLT)) {
res = 0;
- rc = sqlCursorPrevious(pC->uc.pCursor, &res);
- if (rc!=SQL_OK) goto abort_due_to_error;
+ if (sqlCursorPrevious(pC->uc.pCursor, &res) != SQL_OK)
+ goto abort_due_to_error;
} else {
/* res might be negative because the table is empty. Check to
* see if this is the case.
@@ -3681,10 +3632,11 @@ case OP_Found: { /* jump, in3 */
}
}
rc = sqlCursorMovetoUnpacked(pC->uc.pCursor, pIdxKey, &res);
- if (pFree) sqlDbFree(db, pFree);
- if (rc!=SQL_OK) {
+ if (pFree != NULL)
+ sqlDbFree(db, pFree);
+ assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
+ if (rc != SQL_OK)
goto abort_due_to_error;
- }
pC->seekResult = res;
alreadyExists = (res==0);
pC->nullRow = 1-alreadyExists;
@@ -3744,10 +3696,8 @@ case OP_NextIdEphemeral: {
struct space *space = (struct space*)p->aMem[pOp->p1].u.p;
assert(space->def->id == 0);
uint64_t rowid;
- if (space->vtab->ephemeral_rowid_next(space, &rowid) != 0) {
- rc = SQL_TARANTOOL_ERROR;
+ if (space->vtab->ephemeral_rowid_next(space, &rowid) != 0)
goto abort_due_to_error;
- }
/*
* FIXME: since memory cell can comprise only 32-bit
* integer, make sure it can fit in. This check should
@@ -3756,7 +3706,6 @@ case OP_NextIdEphemeral: {
*/
if (rowid > INT32_MAX) {
diag_set(ClientError, ER_ROWID_OVERFLOW);
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
pOut = &aMem[pOp->p2];
@@ -3903,7 +3852,8 @@ case OP_SorterCompare: {
pIn3 = &aMem[pOp->p3];
nKeyCol = pOp->p4.i;
res = 0;
- rc = sqlVdbeSorterCompare(pC, pIn3, nKeyCol, &res);
+ if (sqlVdbeSorterCompare(pC, pIn3, nKeyCol, &res) != 0)
+ rc = SQL_TARANTOOL_ERROR;
VdbeBranchTaken(res!=0,2);
if (rc) goto abort_due_to_error;
if (res) goto jump_to_p2;
@@ -3928,10 +3878,10 @@ case OP_SorterData: {
pOut = &aMem[pOp->p2];
pC = p->apCsr[pOp->p1];
assert(isSorter(pC));
- rc = sqlVdbeSorterRowkey(pC, pOut);
- assert(rc!=SQL_OK || (pOut->flags & MEM_Blob));
+ if (sqlVdbeSorterRowkey(pC, pOut) != SQL_OK)
+ goto abort_due_to_error;
+ assert(pOut->flags & MEM_Blob);
assert(pOp->p1>=0 && pOp->p1<p->nCursor);
- if (rc) goto abort_due_to_error;
p->apCsr[pOp->p3]->cacheStatus = CACHE_STALE;
break;
}
@@ -3998,11 +3948,9 @@ case OP_RowData: {
testcase( n==0);
sqlVdbeMemRelease(pOut);
- rc = sql_vdbe_mem_alloc_region(pOut, n);
- if (rc)
- goto no_mem;
- rc = sqlCursorPayload(pCrsr, 0, n, pOut->z);
- if (rc) goto abort_due_to_error;
+ if (sql_vdbe_mem_alloc_region(pOut, n) != SQL_OK ||
+ sqlCursorPayload(pCrsr, 0, n, pOut->z) != SQL_OK)
+ goto abort_due_to_error;
UPDATE_MAX_BLOBSIZE(pOut);
REGISTER_TRACE(pOp->p2, pOut);
break;
@@ -4137,15 +4085,18 @@ case OP_Rewind: { /* jump */
pC->seekOp = OP_Rewind;
#endif
if (isSorter(pC)) {
- rc = sqlVdbeSorterRewind(pC, &res);
+ if (sqlVdbeSorterRewind(pC, &res) != SQL_OK)
+ goto abort_due_to_error;
} else {
assert(pC->eCurType==CURTYPE_TARANTOOL);
pCrsr = pC->uc.pCursor;
assert(pCrsr);
- rc = tarantoolsqlFirst(pCrsr, &res);
+ if (tarantoolsqlFirst(pCrsr, &res) != SQL_OK)
+ rc = SQL_TARANTOOL_ERROR;
pC->cacheStatus = CACHE_STALE;
+ if (rc != SQL_OK)
+ goto abort_due_to_error;
}
- if (rc) goto abort_due_to_error;
pC->nullRow = (u8)res;
assert(pOp->p2>0 && pOp->p2<p->nOp);
VdbeBranchTaken(res!=0,2);
@@ -4291,11 +4242,8 @@ case OP_SorterInsert: { /* in2 */
assert(isSorter(cursor));
pIn2 = &aMem[pOp->p2];
assert((pIn2->flags & MEM_Blob) != 0);
- rc = ExpandBlob(pIn2);
- if (rc != 0)
- goto abort_due_to_error;
- rc = sqlVdbeSorterWrite(cursor, pIn2);
- if (rc != 0)
+ if (ExpandBlob(pIn2) != SQL_OK ||
+ sqlVdbeSorterWrite(cursor, pIn2) != SQL_OK)
goto abort_due_to_error;
break;
}
@@ -4325,8 +4273,7 @@ case OP_IdxInsert: {
assert((pIn2->flags & MEM_Blob) != 0);
if (pOp->p5 & OPFLAG_NCHANGE)
p->nChange++;
- rc = ExpandBlob(pIn2);
- if (rc != 0)
+ if (ExpandBlob(pIn2) != SQL_OK)
goto abort_due_to_error;
struct space *space;
if (pOp->p4type == P4_SPACEPTR)
@@ -4360,6 +4307,7 @@ case OP_IdxInsert: {
} else if (pOp->p5 & OPFLAG_OE_ROLLBACK) {
p->errorAction = ON_CONFLICT_ACTION_ROLLBACK;
}
+ assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
if (rc != 0)
goto abort_due_to_error;
break;
@@ -4427,14 +4375,12 @@ case OP_Update: {
if (is_error) {
diag_set(OutOfMemory, stream.pos - stream.buf,
"mpstream_flush", "stream");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
uint32_t ops_size = region_used(region) - used;
const char *ops = region_join(region, ops_size);
if (ops == NULL) {
diag_set(OutOfMemory, ops_size, "region_join", "raw");
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
@@ -4460,6 +4406,7 @@ case OP_Update: {
} else if (pOp->p5 & OPFLAG_OE_ROLLBACK) {
p->errorAction = ON_CONFLICT_ACTION_ROLLBACK;
}
+ assert(rc == SQL_OK || rc == SQL_TARANTOOL_ERROR);
if (rc != 0)
goto abort_due_to_error;
break;
@@ -4510,8 +4457,7 @@ case OP_SDelete: {
struct space *space = space_by_id(pOp->p1);
assert(space != NULL);
assert(space_is_system(space));
- rc = sql_delete_by_key(space, 0, pIn2->z, pIn2->n);
- if (rc)
+ if (sql_delete_by_key(space, 0, pIn2->z, pIn2->n) != SQL_OK)
goto abort_due_to_error;
if (pOp->p5 & OPFLAG_NCHANGE)
p->nChange++;
@@ -4545,18 +4491,19 @@ case OP_IdxDelete: {
r.default_rc = 0;
r.aMem = &aMem[pOp->p2];
r.opcode = OP_IdxDelete;
- rc = sqlCursorMovetoUnpacked(pCrsr, &r, &res);
- if (rc) goto abort_due_to_error;
+ if (sqlCursorMovetoUnpacked(pCrsr, &r, &res) != SQL_OK)
+ goto abort_due_to_error;
if (res==0) {
assert(pCrsr->eState == CURSOR_VALID);
if (pCrsr->curFlags & BTCF_TaCursor) {
- rc = tarantoolsqlDelete(pCrsr, 0);
+ if (tarantoolsqlDelete(pCrsr, 0) != SQL_OK)
+ goto abort_due_to_error;
} else if (pCrsr->curFlags & BTCF_TEphemCursor) {
- rc = tarantoolsqlEphemeralDelete(pCrsr);
+ if (tarantoolsqlEphemeralDelete(pCrsr) != SQL_OK)
+ goto abort_due_to_error;
} else {
unreachable();
}
- if (rc) goto abort_due_to_error;
}
pC->cacheStatus = CACHE_STALE;
pC->seekResult = 0;
@@ -4668,14 +4615,14 @@ case OP_Clear: {
rc = 0;
if (pOp->p2 > 0) {
if (box_truncate(space_id) != 0)
- rc = SQL_TARANTOOL_ERROR;
+ goto abort_due_to_error;
} else {
uint32_t tuple_count;
- rc = tarantoolsqlClearTable(space, &tuple_count);
- if (rc == 0 && (pOp->p5 & OPFLAG_NCHANGE) != 0)
+ if (tarantoolsqlClearTable(space, &tuple_count) != SQL_OK)
+ goto abort_due_to_error;
+ if ((pOp->p5 & OPFLAG_NCHANGE) != 0)
p->nChange += tuple_count;
}
- if (rc) goto abort_due_to_error;
break;
}
@@ -4698,8 +4645,8 @@ case OP_ResetSorter: {
} else {
assert(pC->eCurType==CURTYPE_TARANTOOL);
assert(pC->uc.pCursor->curFlags & BTCF_TEphemCursor);
- rc = tarantoolsqlEphemeralClearTable(pC->uc.pCursor);
- if (rc) goto abort_due_to_error;
+ if (tarantoolsqlEphemeralClearTable(pC->uc.pCursor) != SQL_OK)
+ goto abort_due_to_error;
}
break;
}
@@ -4732,8 +4679,8 @@ case OP_RenameTable: {
zNewTableName = pOp->p4.z;
zOldTableName = sqlDbStrNDup(db, zOldTableName,
sqlStrlen30(zOldTableName));
- rc = sql_rename_table(space_id, zNewTableName);
- if (rc) goto abort_due_to_error;
+ if (sql_rename_table(space_id, zNewTableName) != SQL_OK)
+ goto abort_due_to_error;
/*
* Rebuild 'CREATE TRIGGER' expressions of all triggers
* created on this table. Sure, this action is not atomic
@@ -4743,20 +4690,18 @@ case OP_RenameTable: {
for (struct sql_trigger *trigger = triggers; trigger != NULL; ) {
/* Store pointer as trigger will be destructed. */
struct sql_trigger *next_trigger = trigger->next;
- rc = tarantoolsqlRenameTrigger(trigger->zName,
- zOldTableName, zNewTableName);
- if (rc != SQL_OK) {
- /*
- * FIXME: In the case of error,
- * part of triggers would have invalid
- * space name in tuple so can not been
- * persisted.
- * Server could be restarted.
- * In this case, rename table back and
- * try again.
- */
+ /*
+ * FIXME: In the case of error,
+ * part of triggers would have invalid
+ * space name in tuple so can not been
+ * persisted.
+ * Server could be restarted.
+ * In this case, rename table back and
+ * try again.
+ */
+ if (tarantoolsqlRenameTrigger(trigger->zName, zOldTableName,
+ zNewTableName) != SQL_OK)
goto abort_due_to_error;
- }
trigger = next_trigger;
}
sqlDbFree(db, (void*)zOldTableName);
@@ -4771,8 +4716,8 @@ case OP_RenameTable: {
*/
case OP_LoadAnalysis: {
assert(pOp->p1==0 );
- rc = sql_analysis_load(db);
- if (rc) goto abort_due_to_error;
+ if (sql_analysis_load(db) != SQL_OK)
+ goto abort_due_to_error;
break;
}
@@ -4828,8 +4773,8 @@ case OP_Program: { /* jump */
}
if (p->nFrame>=db->aLimit[SQL_LIMIT_TRIGGER_DEPTH]) {
- rc = SQL_ERROR;
- sqlVdbeError(p, "too many levels of trigger recursion");
+ diag_set(ClientError, ER_SQL_EXECUTE, "too many levels of "\
+ "trigger recursion");
goto abort_due_to_error;
}
@@ -5157,11 +5102,9 @@ case OP_AggStep: {
(pCtx->pFunc->xSFunc)(pCtx,pCtx->argc,pCtx->argv); /* IMP: R-24505-23230 */
if (pCtx->is_aborted) {
sqlVdbeMemRelease(&t);
- rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
- } else {
- assert(t.flags==MEM_Null);
}
+ assert(t.flags==MEM_Null);
if (pCtx->skipFlag) {
assert(pOp[-1].opcode==OP_CollSeq);
i = pOp[-1].p1;
@@ -5188,11 +5131,8 @@ case OP_AggFinal: {
assert(pOp->p1>0 && pOp->p1<=(p->nMem+1 - p->nCursor));
pMem = &aMem[pOp->p1];
assert((pMem->flags & ~(MEM_Null|MEM_Agg))==0);
- rc = sqlVdbeMemFinalize(pMem, pOp->p4.pFunc);
- if (rc) {
- sqlVdbeError(p, "%s", sql_value_text(pMem));
+ if (sqlVdbeMemFinalize(pMem, pOp->p4.pFunc) != 0)
goto abort_due_to_error;
- }
UPDATE_MAX_BLOBSIZE(pMem);
if (sqlVdbeMemTooBig(pMem)) {
goto too_big;
@@ -5301,10 +5241,8 @@ case OP_IncMaxid: {
assert(pOp->p1 > 0);
pOut = &aMem[pOp->p1];
- rc = tarantoolsqlIncrementMaxid((uint64_t*) &pOut->u.i);
- if (rc!=SQL_OK) {
+ if (tarantoolsqlIncrementMaxid((uint64_t*) &pOut->u.i) != SQL_OK)
goto abort_due_to_error;
- }
pOut->flags = MEM_Int;
break;
}
@@ -5368,28 +5306,8 @@ default: { /* This is really OP_Noop and OP_Explain */
* an error of some kind.
*/
abort_due_to_error:
- if (db->mallocFailed) rc = SQL_NOMEM;
- assert(rc);
- if (p->zErrMsg==0 && rc!=SQL_IOERR_NOMEM) {
- const char *msg;
- /* Avoiding situation when Tarantool error is set,
- * but error message isn't.
- */
- if (rc == SQL_TARANTOOL_ERROR && tarantoolErrorMessage()) {
- msg = tarantoolErrorMessage();
- } else {
- msg = sqlErrStr(rc);
- }
- sqlVdbeError(p, "%s", msg);
- }
+ rc = SQL_TARANTOOL_ERROR;
p->rc = rc;
- sqlSystemError(db, rc);
- testcase( sqlGlobalConfig.xLog!=0);
- sql_log(rc, "statement aborts at %d: [%s] %s",
- (int)(pOp - aOp), p->zSql, p->zErrMsg);
- sqlVdbeHalt(p);
- if (rc==SQL_IOERR_NOMEM) sqlOomFault(db);
- rc = SQL_ERROR;
/* This is the only way out of this procedure. */
vdbe_return:
@@ -5398,21 +5316,20 @@ vdbe_return:
assert(rc!=SQL_OK || nExtraDelete==0
|| sql_strlike_ci("DELETE%", p->zSql, 0) != 0
);
+ assert(rc == SQL_OK || rc == SQL_BUSY || rc == SQL_TARANTOOL_ERROR ||
+ rc == SQL_ROW || rc == SQL_DONE);
return rc;
/* Jump to here if a string or blob larger than SQL_MAX_LENGTH
* is encountered.
*/
too_big:
- sqlVdbeError(p, "string or blob too big");
- rc = SQL_TOOBIG;
+ diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
goto abort_due_to_error;
/* Jump to here if a malloc() fails.
*/
no_mem:
sqlOomFault(db);
- sqlVdbeError(p, "out of memory");
- rc = SQL_NOMEM;
goto abort_due_to_error;
}
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index ee14510..9e6a426 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -244,10 +244,6 @@ struct Mem {
#define MEM_Agg 0x4000 /* Mem.z points to an agg function context */
#define MEM_Zero 0x8000 /* Mem.i contains count of 0s appended to blob */
#define MEM_Subtype 0x10000 /* Mem.eSubtype is valid */
-#ifdef SQL_OMIT_INCRBLOB
-#undef MEM_Zero
-#define MEM_Zero 0x0000
-#endif
/**
* In contrast to Mem_TypeMask, this one allows to get
@@ -450,7 +446,6 @@ struct Vdbe {
/*
* Function prototypes
*/
-void sqlVdbeError(Vdbe *, const char *, ...);
void sqlVdbeFreeCursor(Vdbe *, VdbeCursor *);
void sqlVdbePopStack(Vdbe *, int);
int sqlVdbeCursorRestore(VdbeCursor *);
@@ -532,13 +527,8 @@ void sqlVdbeMemPrettyPrint(Mem * pMem, char *zBuf);
#endif
int sqlVdbeMemHandleBom(Mem * pMem);
-#ifndef SQL_OMIT_INCRBLOB
int sqlVdbeMemExpandBlob(Mem *);
#define ExpandBlob(P) (((P)->flags&MEM_Zero)?sqlVdbeMemExpandBlob(P):0)
-#else
-#define sqlVdbeMemExpandBlob(x) SQL_OK
-#define ExpandBlob(P) SQL_OK
-#endif
/**
* Perform comparison of two keys: one is packed and one is not.
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 673ccd1..3bdfa7d 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -536,15 +536,6 @@ sqlStep(Vdbe * p)
p->rc = SQL_NOMEM;
}
end_of_step:
- /* At this point local variable rc holds the value that should be
- * returned if this statement was compiled using the legacy
- * sql_prepare() interface. According to the docs, this can only
- * be one of the values in the first assert() below. Variable p->rc
- * contains the value that would be returned if sql_finalize()
- * were called on statement p.
- */
- assert(rc == SQL_ROW || rc == SQL_DONE || rc == SQL_ERROR
- || (rc & 0xff) == SQL_BUSY || rc == SQL_MISUSE);
if (p->isPrepareV2 && rc != SQL_ROW && rc != SQL_DONE) {
/* If this statement was prepared using sql_prepare_v2(), and an
* error has occurred, then return the error code in p->rc to the
@@ -564,20 +555,17 @@ int
sql_step(sql_stmt * pStmt)
{
int rc; /* Result from sqlStep() */
- int rc2 = SQL_OK; /* Result from sqlReprepare() */
Vdbe *v = (Vdbe *) pStmt; /* the prepared statement */
int cnt = 0; /* Counter to prevent infinite loop of reprepares */
- sql *db; /* The database connection */
if (vdbeSafetyNotNull(v)) {
return SQL_MISUSE;
}
- db = v->db;
v->doingRerun = 0;
while ((rc = sqlStep(v)) == SQL_SCHEMA
&& cnt++ < SQL_MAX_SCHEMA_RETRY) {
int savedPc = v->pc;
- rc2 = rc = sqlReprepare(v);
+ rc = sqlReprepare(v);
if (rc != SQL_OK)
break;
sql_reset(pStmt);
@@ -585,26 +573,6 @@ sql_step(sql_stmt * pStmt)
v->doingRerun = 1;
assert(v->expired == 0);
}
- if (rc2 != SQL_OK) {
- /* This case occurs after failing to recompile an sql statement.
- * The error message from the SQL compiler has already been loaded
- * into the database handle. This block copies the error message
- * from the database handle into the statement and sets the statement
- * program counter to 0 to ensure that when the statement is
- * finalized or reset the parser error message is available via
- * sql_errmsg() and sql_errcode().
- */
- const char *zErr = (const char *)sql_value_text(db->pErr);
- sqlDbFree(db, v->zErrMsg);
- if (!db->mallocFailed) {
- v->zErrMsg = sqlDbStrDup(db, zErr);
- v->rc = rc2;
- } else {
- v->zErrMsg = 0;
- v->rc = rc = SQL_NOMEM;
- }
- }
- rc = sqlApiExit(db, rc);
return rc;
}
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 619b211..3f573d0 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -116,19 +116,6 @@ sql_vdbe_prepare(struct Vdbe *vdbe)
}
/*
- * Change the error string stored in Vdbe.zErrMsg
- */
-void
-sqlVdbeError(Vdbe * p, const char *zFormat, ...)
-{
- va_list ap;
- sqlDbFree(p->db, p->zErrMsg);
- va_start(ap, zFormat);
- p->zErrMsg = sqlVMPrintf(p->db, zFormat, ap);
- va_end(ap);
-}
-
-/*
* Remember the SQL string for a prepared statement.
*/
void
@@ -2115,10 +2102,11 @@ sqlVdbeCheckFk(Vdbe * p, int deferred)
if ((deferred && txn != NULL && txn->psql_txn != NULL &&
txn->psql_txn->fk_deferred_count > 0) ||
(!deferred && p->nFkConstraint > 0)) {
- p->rc = SQL_CONSTRAINT_FOREIGNKEY;
+ p->rc = SQL_TARANTOOL_ERROR;
p->errorAction = ON_CONFLICT_ACTION_ABORT;
- sqlVdbeError(p, "FOREIGN KEY constraint failed");
- return SQL_ERROR;
+ diag_set(ClientError, ER_SQL_EXECUTE, "FOREIGN KEY constraint "\
+ "failed");
+ return SQL_TARANTOOL_ERROR;
}
return SQL_OK;
}
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 585dc21..d6375a6 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -210,7 +210,6 @@ sqlVdbeMemMakeWriteable(Mem * pMem)
* If the given Mem* has a zero-filled tail, turn it into an ordinary
* blob stored in dynamically allocated space.
*/
-#ifndef SQL_OMIT_INCRBLOB
int
sqlVdbeMemExpandBlob(Mem * pMem)
{
@@ -232,7 +231,6 @@ sqlVdbeMemExpandBlob(Mem * pMem)
pMem->flags &= ~(MEM_Zero | MEM_Term);
return SQL_OK;
}
-#endif
/*
* It is already known that pMem contains an unterminated string.
@@ -315,8 +313,7 @@ sqlVdbeMemStringify(Mem * pMem, u8 bForce)
* This routine calls the finalize method for that function. The
* result of the aggregate is stored back into pMem.
*
- * Return SQL_ERROR if the finalizer reports an error. SQL_OK
- * otherwise.
+ * Return -1 if the finalizer reports an error. 0 otherwise.
*/
int
sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
@@ -337,9 +334,10 @@ sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
if (pMem->szMalloc > 0)
sqlDbFree(pMem->db, pMem->zMalloc);
memcpy(pMem, &t, sizeof(t));
- return ctx.is_aborted ? SQL_TARANTOOL_ERROR : SQL_OK;
+ if (ctx.is_aborted)
+ return -1;
}
- return SQL_OK;
+ return 0;
}
/*
diff --git a/test/sql-tap/gh-2931-savepoints.test.lua b/test/sql-tap/gh-2931-savepoints.test.lua
index 6123fc4..be499b6 100755
--- a/test/sql-tap/gh-2931-savepoints.test.lua
+++ b/test/sql-tap/gh-2931-savepoints.test.lua
@@ -39,7 +39,7 @@ local testcases = {
{0,{1,1}}},
{"5",
[[rollback to savepoint s1_2;]],
- {1, "Failed to execute SQL statement: no such savepoint: S1_2"}},
+ {1, "Can not rollback to savepoint: the savepoint does not exist"}},
{"6",
[[insert into t1 values(2);
select * from t1 union all select * from t2;]],
diff --git a/test/sql/savepoints.result b/test/sql/savepoints.result
index bb4a296..d20e0ed 100644
--- a/test/sql/savepoints.result
+++ b/test/sql/savepoints.result
@@ -67,7 +67,7 @@ end;
...
release_sv_fail();
---
-- error: 'Failed to execute SQL statement: no such savepoint: T1'
+- error: 'Can not rollback to savepoint: the savepoint does not exist'
...
box.commit();
---
</pre>
</blockquote>
</body>
</html>