[tarantool-patches] Re: [PATCH v4 3/8] sql: replace rc with is_aborted status in struct Parse
Mergen Imeev
imeevma at tarantool.org
Mon Mar 18 18:28:04 MSK 2019
Hi! Thank you for review. My answers and two new patches below.
On Thu, Mar 14, 2019 at 10:53:00PM +0300, n.pettik wrote:
>
> >>> diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
> >>> index bab6493..0d8bf15 100755
> >>> --- a/test/sql-tap/check.test.lua
> >>> +++ b/test/sql-tap/check.test.lua
> >>> @@ -516,7 +516,7 @@ test:do_catchsql_test(
> >>> );
> >>> ]], {
> >>> -- <check-5.1>
> >>> - 1, "Wrong space options (field 5): invalid expression specified (SQL error: bindings are not allowed in DDL)"
> >>> + 1, "Wrong space options (field 5): invalid expression specified (bindings are not allowed in DDL)"
> >>> -- </check-5.1>
> >>> })
> >>
> >> Why test results have changed if you provided
> >> non-functional refactoring?
> > It become this way because now the error in diag instead of being
> > only in zErrMsg of struct Parse.
>
> So, then it should be related to the previous patch, I guess.
> Otherwise, still don’t understand. Or fix commit message,
> since now it implies that only refactoring was provided.
> Or, what is better - move functional changes to separate patch.
>
I divided this ptch into two:
"sql: remove argument pzErrMsg from sqlRunParser()"
"sql: replace rc with is_aborted status in struct Parse"
> > index a2937a0..c2e5d6b 100644
> > --- a/src/box/sql.c
> > +++ b/src/box/sql.c
> > @@ -1363,12 +1363,8 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
> >
> > sql_resolve_self_reference(&parser, def, NC_IsCheck, NULL, expr_list);
> > int rc = 0;
> > - if (parser.rc != SQL_OK) {
> > - /* Tarantool error may be already set with diag. */
> > - if (parser.rc != SQL_TARANTOOL_ERROR)
> > - diag_set(ClientError, ER_SQL, parser.zErrMsg);
> > + if (parser.is_aborted)
> > rc = -1;
> > - }
> > sql_parser_destroy(&parser);
> > return rc;
>
> diff --git a/src/box/sql.c b/src/box/sql.c
> index c2e5d6bd1..ea71dd101 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -1362,9 +1362,6 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
> parser.parse_only = true;
>
> sql_resolve_self_reference(&parser, def, NC_IsCheck, NULL, expr_list);
> - int rc = 0;
> - if (parser.is_aborted)
> - rc = -1;
> sql_parser_destroy(&parser);
> - return rc;
> + return parser.is_aborted ? -1 : 0;
> }
>
I am not sure that this should be applied. I think it isn't right
to look at field is_aborted of parser after parser destruction.
> > }
> >
> > diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
> > index 828a1ae..85385ee 100644
> > --- a/src/box/sql/prepare.c
> > +++ b/src/box/sql/prepare.c
> > @@ -52,7 +52,6 @@ sqlPrepare(sql * db, /* Database handle. */
> > const char **pzTail /* OUT: End of parsed string */
> > )
> > {
> > - char *zErrMsg = 0; /* Error message */
> > int rc = SQL_OK; /* Result code */
> > Parse sParse; /* Parsing context */
> > sql_parser_create(&sParse, db);
> > @@ -89,25 +88,24 @@ sqlPrepare(sql * db, /* Database handle. */
> > }
> > zSqlCopy = sqlDbStrNDup(db, zSql, nBytes);
> > if (zSqlCopy) {
> > - sqlRunParser(&sParse, zSqlCopy, &zErrMsg);
> > + sqlRunParser(&sParse, zSqlCopy);
> > sParse.zTail = &zSql[sParse.zTail - zSqlCopy];
> > sqlDbFree(db, zSqlCopy);
> > } else {
> > sParse.zTail = &zSql[nBytes];
> > }
> > } else {
> > - sqlRunParser(&sParse, zSql, &zErrMsg);
> > + sqlRunParser(&sParse, zSql);
> > }
> > assert(0 == sParse.nQueryLoop);
> >
> > - if (sParse.rc == SQL_DONE)
> > - sParse.rc = SQL_OK;
> > if (db->mallocFailed)
> > - sParse.rc = SQL_TARANTOOL_ERROR;
> > + sParse.is_aborted = true;
> > if (pzTail) {
> > *pzTail = sParse.zTail;
> > }
> > - rc = sParse.rc;
> > + if (sParse.is_aborted)
> > + rc = SQL_TARANTOOL_ERROR;
> >
> > if (rc == SQL_OK && sParse.pVdbe && sParse.explain) {
> > static const char *const azColName[] = {
> > @@ -168,11 +166,7 @@ sqlPrepare(sql * db, /* Database handle. */
> > *ppStmt = (sql_stmt *) sParse.pVdbe;
> > }
> >
> > - if (zErrMsg) {
> > - sqlErrorWithMsg(db, rc, "%s", zErrMsg);
> > - } else {
> > - sqlError(db, rc);
> > - }
> > + db->errCode = rc;
>
> Is this thing used anywhere?
Yes, it is used in sql_errmsg(). It contains VDBE errors too, so I
think it would be better to leave it for now.
>
>
First new patch:
commit 50af26719f6d195417f546ea2cfd4e70e3b09421
Author: Mergen Imeev <imeevma at gmail.com>
Date: Fri Mar 15 21:37:58 2019 +0300
sql: remove argument pzErrMsg from sqlRunParser()
This argument has practically no functionality, but deleting it
allows us to replace the rc field of the Parse structure with a
new bool field.
Part of #3965
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 828a1ae..4eaa9a4 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -52,7 +52,6 @@ sqlPrepare(sql * db, /* Database handle. */
const char **pzTail /* OUT: End of parsed string */
)
{
- char *zErrMsg = 0; /* Error message */
int rc = SQL_OK; /* Result code */
Parse sParse; /* Parsing context */
sql_parser_create(&sParse, db);
@@ -89,14 +88,14 @@ sqlPrepare(sql * db, /* Database handle. */
}
zSqlCopy = sqlDbStrNDup(db, zSql, nBytes);
if (zSqlCopy) {
- sqlRunParser(&sParse, zSqlCopy, &zErrMsg);
+ sqlRunParser(&sParse, zSqlCopy);
sParse.zTail = &zSql[sParse.zTail - zSqlCopy];
sqlDbFree(db, zSqlCopy);
} else {
sParse.zTail = &zSql[nBytes];
}
} else {
- sqlRunParser(&sParse, zSql, &zErrMsg);
+ sqlRunParser(&sParse, zSql);
}
assert(0 == sParse.nQueryLoop);
@@ -168,11 +167,7 @@ sqlPrepare(sql * db, /* Database handle. */
*ppStmt = (sql_stmt *) sParse.pVdbe;
}
- if (zErrMsg) {
- sqlErrorWithMsg(db, rc, "%s", zErrMsg);
- } else {
- sqlError(db, rc);
- }
+ db->errCode = rc;
/* Delete any TriggerPrg structures allocated while parsing this statement. */
while (sParse.pTriggerPrg) {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index c63ff1c..b9b45b5 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3198,7 +3198,7 @@ void sqlDequote(char *);
void sqlNormalizeName(char *z);
void sqlTokenInit(Token *, char *);
int sqlKeywordCode(const unsigned char *, int);
-int sqlRunParser(Parse *, const char *, char **);
+int sqlRunParser(Parse *, const char *);
/**
* Increment error counter.
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index f0a2f16..8ee996e 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -437,17 +437,17 @@ parser_space_delete(struct sql *db, struct space *space)
sql_expr_list_delete(db, space->def->opts.checks);
}
-/*
- * Run the parser on the given SQL string. The parser structure is
- * passed in. An SQL_ status code is returned. If an error occurs
- * then an and attempt is made to write an error message into
- * memory obtained from sql_malloc() and to make *pzErrMsg point to that
- * error message.
+/**
+ * Run the parser on the given SQL string.
+ *
+ * @param pParse Parser context.
+ * @param zSql SQL string.
+ * @retval 0 on success.
+ * @retval -1 on error.
*/
int
-sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
+sqlRunParser(Parse * pParse, const char *zSql)
{
- int nErr = 0; /* Number of errors encountered */
int i; /* Loop counter */
void *pEngine; /* The LEMON-generated LALR(1) parser */
int tokenType; /* type of the next token */
@@ -463,12 +463,11 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
pParse->rc = SQL_OK;
pParse->zTail = zSql;
i = 0;
- assert(pzErrMsg != 0);
/* sqlParserTrace(stdout, "parser: "); */
pEngine = sqlParserAlloc(sqlMalloc);
if (pEngine == 0) {
sqlOomFault(db);
- return SQL_NOMEM;
+ return -1;
}
assert(pParse->new_space == NULL);
assert(pParse->parsed_ast.trigger == NULL);
@@ -518,7 +517,6 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
break;
}
}
- assert(nErr == 0);
pParse->zTail = &zSql[i];
#ifdef YYTRACKMAXSTACKDEPTH
sqlStatusHighwater(SQL_STATUS_PARSER_STACK,
@@ -530,21 +528,8 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
pParse->rc = SQL_TARANTOOL_ERROR;
}
if (pParse->rc != SQL_OK && pParse->rc != SQL_DONE
- && pParse->zErrMsg == 0) {
- const char *error;
- if (is_tarantool_error(pParse->rc) &&
- tarantoolErrorMessage() != NULL)
- error = tarantoolErrorMessage();
- else
- error = sqlErrStr(pParse->rc);
- pParse->zErrMsg = sqlMPrintf(db, "%s", error);
- }
- assert(pzErrMsg != 0);
- if (pParse->zErrMsg) {
- *pzErrMsg = pParse->zErrMsg;
- sql_log(pParse->rc, "%s", *pzErrMsg);
- nErr++;
- }
+ && pParse->zErrMsg == 0)
+ pParse->zErrMsg = sqlMPrintf(db, "%s", tarantoolErrorMessage());
if (pParse->pVdbe != NULL && pParse->nErr > 0) {
sqlVdbeDelete(pParse->pVdbe);
pParse->pVdbe = 0;
@@ -553,8 +538,7 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
if (pParse->pWithToFree)
sqlWithDelete(db, pParse->pWithToFree);
sqlDbFree(db, pParse->pVList);
- assert(nErr == 0 || pParse->rc != SQL_OK);
- return nErr;
+ return pParse->nErr > 0 || pParse->rc == SQL_TARANTOOL_ERROR ? -1 : 0;
}
struct Expr *
@@ -575,11 +559,8 @@ sql_expr_compile(sql *db, const char *expr, int expr_len)
}
sprintf(stmt, "%s%.*s", outer, expr_len, expr);
- char *sql_error = NULL;
- if (sqlRunParser(&parser, stmt, &sql_error) != SQL_OK ||
- parser.parsed_ast_type != AST_TYPE_EXPR) {
- diag_set(ClientError, ER_SQL, sql_error);
- } else {
+ if (sqlRunParser(&parser, stmt) == 0 &&
+ parser.parsed_ast_type == AST_TYPE_EXPR) {
expression = parser.parsed_ast.expr;
parser.parsed_ast.expr = NULL;
}
@@ -597,8 +578,7 @@ sql_view_compile(struct sql *db, const char *view_stmt)
struct Select *select = NULL;
- char *unused;
- if (sqlRunParser(&parser, view_stmt, &unused) != SQL_OK ||
+ if (sqlRunParser(&parser, view_stmt) != 0 ||
parser.parsed_ast_type != AST_TYPE_SELECT) {
diag_set(ClientError, ER_SQL_EXECUTE, view_stmt);
} else {
@@ -616,13 +596,9 @@ sql_trigger_compile(struct sql *db, const char *sql)
struct Parse parser;
sql_parser_create(&parser, db);
parser.parse_only = true;
- char *sql_error = NULL;
struct sql_trigger *trigger = NULL;
- if (sqlRunParser(&parser, sql, &sql_error) != SQL_OK ||
- parser.parsed_ast_type != AST_TYPE_TRIGGER) {
- if (parser.rc != SQL_TARANTOOL_ERROR)
- diag_set(ClientError, ER_SQL, sql_error);
- } else {
+ if (sqlRunParser(&parser, sql) == 0 &&
+ parser.parsed_ast_type == AST_TYPE_TRIGGER) {
trigger = parser.parsed_ast.trigger;
parser.parsed_ast.trigger = NULL;
}
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index bab6493..0d8bf15 100755
--- a/test/sql-tap/check.test.lua
+++ b/test/sql-tap/check.test.lua
@@ -516,7 +516,7 @@ test:do_catchsql_test(
);
]], {
-- <check-5.1>
- 1, "Wrong space options (field 5): invalid expression specified (SQL error: bindings are not allowed in DDL)"
+ 1, "Wrong space options (field 5): invalid expression specified (bindings are not allowed in DDL)"
-- </check-5.1>
})
@@ -528,7 +528,7 @@ test:do_catchsql_test(
);
]], {
-- <check-5.2>
- 1, "Wrong space options (field 5): invalid expression specified (SQL error: bindings are not allowed in DDL)"
+ 1, "Wrong space options (field 5): invalid expression specified (bindings are not allowed in DDL)"
-- </check-5.2>
})
diff --git a/test/sql/checks.result b/test/sql/checks.result
index e31964c..42df657 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -29,8 +29,8 @@ t = {513, 1, 'test', 'memtx', 0, opts, format}
...
s = box.space._space:insert(t)
---
-- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
- Syntax error near ''<'')'
+- error: 'Wrong space options (field 5): invalid expression specified (Syntax error
+ near ''<'')'
...
opts = {checks = {{expr = 'X>5'}}}
---
@@ -122,8 +122,8 @@ box.sql.execute("DROP TABLE w2;")
--
box.sql.execute("CREATE TABLE t5(x INT PRIMARY KEY, y INT, CHECK( x*y < ? ));")
---
-- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
- bindings are not allowed in DDL)'
+- error: 'Wrong space options (field 5): invalid expression specified (bindings are
+ not allowed in DDL)'
...
opts = {checks = {{expr = '?>5', name = 'ONE'}}}
---
@@ -136,8 +136,8 @@ t = {513, 1, 'test', 'memtx', 0, opts, format}
...
s = box.space._space:insert(t)
---
-- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
- bindings are not allowed in DDL)'
+- error: 'Wrong space options (field 5): invalid expression specified (bindings are
+ not allowed in DDL)'
...
test_run:cmd("clear filter")
---
Second new patch:
commit c8316b7f20ffad80857568fab2ad422c85218d05
Author: Mergen Imeev <imeevma at gmail.com>
Date: Wed Mar 6 22:09:26 2019 +0300
sql: replace rc with is_aborted status in struct Parse
Currently, field representing return code in struct Parse can take
only two values: SQL_OK (successfully finished parsing) and
SQL_TARANTOOL_ERROR (in case of any errors occurred). Therefore,
it can be replaced with a boolean field. Patch provides
straightforward refactoring.
Part of #3965
diff --git a/src/box/sql.c b/src/box/sql.c
index a2937a0..c2e5d6b 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1275,7 +1275,7 @@ sql_ephemeral_space_def_new(struct Parse *parser, const char *name)
if (def == NULL) {
diag_set(OutOfMemory, size, "region_alloc",
"sql_ephemeral_space_def_new");
- parser->rc = SQL_TARANTOOL_ERROR;
+ parser->is_aborted = true;
parser->nErr++;
return NULL;
}
@@ -1294,7 +1294,7 @@ sql_ephemeral_space_new(Parse *parser, const char *name)
struct space *space = (struct space *) region_alloc(&parser->region, sz);
if (space == NULL) {
diag_set(OutOfMemory, sz, "region", "space");
- parser->rc = SQL_TARANTOOL_ERROR;
+ parser->is_aborted = true;
parser->nErr++;
return NULL;
}
@@ -1363,12 +1363,8 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
sql_resolve_self_reference(&parser, def, NC_IsCheck, NULL, expr_list);
int rc = 0;
- if (parser.rc != SQL_OK) {
- /* Tarantool error may be already set with diag. */
- if (parser.rc != SQL_TARANTOOL_ERROR)
- diag_set(ClientError, ER_SQL, parser.zErrMsg);
+ if (parser.is_aborted)
rc = -1;
- }
sql_parser_destroy(&parser);
return rc;
}
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index d49ebb8..fe4754f 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -72,7 +72,7 @@ exit_rename_table:
return;
tnt_error:
sqlDbFree(db, new_name);
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
parse->nErr++;
goto exit_rename_table;
}
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index ea5cbc3..96b7099 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -907,7 +907,7 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space)
if (jump_addrs == NULL) {
diag_set(OutOfMemory, sizeof(int) * part_count,
"region", "jump_addrs");
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
parse->nErr++;
return;
}
@@ -1130,7 +1130,7 @@ sqlAnalyze(Parse * pParse, Token * pName)
}
} else {
diag_set(ClientError, ER_NO_SUCH_SPACE, z);
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
pParse->nErr++;
}
sqlDbFree(db, z);
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index deb5b89..0179a45 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -91,7 +91,7 @@ save_record(struct Parse *parser, uint32_t space_id, int reg_key,
if (record == NULL) {
diag_set(OutOfMemory, sizeof(*record), "region_alloc",
"record");
- parser->rc = SQL_TARANTOOL_ERROR;
+ parser->is_aborted = true;
parser->nErr++;
return;
}
@@ -146,8 +146,7 @@ sql_finish_coding(struct Parse *parse_context)
}
if (db->mallocFailed || parse_context->nErr != 0) {
- if (parse_context->rc == SQL_OK)
- parse_context->rc = SQL_ERROR;
+ parse_context->is_aborted = true;
return;
}
/*
@@ -192,10 +191,8 @@ sql_finish_coding(struct Parse *parse_context)
if (parse_context->nErr == 0 && !db->mallocFailed) {
assert(parse_context->iCacheLevel == 0);
sqlVdbeMakeReady(v, parse_context);
- parse_context->rc = SQL_DONE;
} else {
- if (parse_context->rc != SQL_TARANTOOL_ERROR)
- parse_context->rc = SQL_ERROR;
+ parse_context->is_aborted = true;
}
}
/**
@@ -397,7 +394,7 @@ sql_field_retrieve(Parse *parser, struct space_def *space_def, uint32_t id)
diag_set(OutOfMemory, columns_new *
sizeof(space_def->fields[0]),
"region_alloc", "sql_field_retrieve");
- parser->rc = SQL_TARANTOOL_ERROR;
+ parser->is_aborted = true;
parser->nErr++;
return NULL;
}
@@ -453,7 +450,7 @@ sqlAddColumn(Parse * pParse, Token * pName, struct type_def *type_def)
if (z == NULL) {
diag_set(OutOfMemory, pName->n + 1,
"region_alloc", "z");
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
pParse->nErr++;
return;
}
@@ -501,7 +498,7 @@ sql_column_add_nullable_action(struct Parse *parser,
on_conflict_action_strs[field->
nullable_action]);
diag_set(ClientError, ER_SQL, err_msg);
- parser->rc = SQL_TARANTOOL_ERROR;
+ parser->is_aborted = true;
parser->nErr++;
return;
}
@@ -543,7 +540,7 @@ sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan)
diag_set(OutOfMemory, default_length + 1,
"region_alloc",
"field->default_value");
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
pParse->nErr++;
return;
}
@@ -562,7 +559,7 @@ field_def_create_for_pk(struct Parse *parser, struct field_def *field,
if (field->nullable_action != ON_CONFLICT_ACTION_ABORT &&
field->nullable_action != ON_CONFLICT_ACTION_DEFAULT) {
diag_set(ClientError, ER_NULLABLE_PRIMARY, space_name);
- parser->rc = SQL_TARANTOOL_ERROR;
+ parser->is_aborted = true;
parser->nErr++;
return -1;
} else if (field->nullable_action == ON_CONFLICT_ACTION_DEFAULT) {
@@ -851,7 +848,7 @@ vdbe_emit_create_index(struct Parse *parse, struct space_def *def,
save_record(parse, BOX_INDEX_ID, entry_reg, 2, v->nOp - 1);
return;
error:
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
parse->nErr++;
}
@@ -912,7 +909,7 @@ createSpace(Parse * pParse, int iSpaceId, char *zStmt)
return;
error:
pParse->nErr++;
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
}
int
@@ -1093,7 +1090,7 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context,
return;
error:
parse_context->nErr++;
- parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->is_aborted = true;
}
/**
@@ -1121,7 +1118,7 @@ resolve_link(struct Parse *parse_context, const struct space_def *def,
diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, fk_name,
tt_sprintf("unknown column %s in foreign key definition",
field_name));
- parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->is_aborted = true;
parse_context->nErr++;
return -1;
}
@@ -1253,7 +1250,7 @@ sqlEndTable(Parse * pParse, /* Parse context */
"foreign key does not match the "\
"number of columns in the primary "\
"index of referenced table");
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
pParse->nErr++;
return;
}
@@ -1329,7 +1326,7 @@ sql_create_view(struct Parse *parse_context, struct Token *begin,
space->def->opts.sql = strndup(begin->z, n);
if (space->def->opts.sql == NULL) {
diag_set(OutOfMemory, n, "strndup", "opts.sql");
- parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->is_aborted = true;
parse_context->nErr++;
goto create_view_fail;
}
@@ -1649,7 +1646,7 @@ sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list,
if (! fk_constraint_is_self_referenced(fk->def)) {
diag_set(ClientError, ER_DROP_SPACE, space_name,
"other objects depend on it");
- parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->is_aborted = true;
parse_context->nErr++;
goto exit_drop_table;
}
@@ -1685,7 +1682,7 @@ columnno_by_name(struct Parse *parse_context, const struct space *space,
diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, fk_name,
tt_sprintf("foreign key refers to nonexistent field %s",
column_name));
- parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->is_aborted = true;
parse_context->nErr++;
return -1;
}
@@ -1895,7 +1892,7 @@ exit_create_fk:
sqlDbFree(db, constraint_name);
return;
tnt_error:
- parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->is_aborted = true;
parse_context->nErr++;
goto exit_create_fk;
}
@@ -1920,7 +1917,7 @@ sql_drop_foreign_key(struct Parse *parse_context, struct SrcList *table,
struct space *child = space_by_name(table_name);
if (child == NULL) {
diag_set(ClientError, ER_NO_SUCH_SPACE, table_name);
- parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->is_aborted = true;
parse_context->nErr++;
return;
}
@@ -2099,7 +2096,7 @@ cleanup:
key_def_delete(key_def);
return rc;
tnt_error:
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
++parse->nErr;
goto cleanup;
}
@@ -2150,7 +2147,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
if (space == NULL) {
if (! if_not_exist) {
diag_set(ClientError, ER_NO_SUCH_SPACE, name);
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
parse->nErr++;
}
goto exit_create_index;
@@ -2245,7 +2242,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
diag_set(ClientError, ER_MODIFY_INDEX, name, def->name,
"can't create index on system space");
parse->nErr++;
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
goto exit_create_index;
}
@@ -2273,7 +2270,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
index = (struct index *) region_alloc(&parse->region, sizeof(*index));
if (index == NULL) {
diag_set(OutOfMemory, sizeof(*index), "region", "index");
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
parse->nErr++;
goto exit_create_index;
}
diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
index 4594cac..5c9cf98 100644
--- a/src/box/sql/callback.c
+++ b/src/box/sql/callback.c
@@ -49,7 +49,7 @@ sql_get_coll_seq(Parse *parser, const char *name, uint32_t *coll_id)
struct coll_id *p = coll_by_name(name, strlen(name));
if (p == NULL) {
diag_set(ClientError, ER_NO_SUCH_COLLATION, name);
- parser->rc = SQL_TARANTOOL_ERROR;
+ parser->is_aborted = true;
parser->nErr++;
return NULL;
} else {
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 5170c7f..3123681 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -50,7 +50,7 @@ sql_lookup_space(struct Parse *parse, struct SrcList_item *space_name)
if (space->def->field_count == 0) {
diag_set(ClientError, ER_UNSUPPORTED, "SQL",
"space without format");
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
parse->nErr++;
return NULL;
}
@@ -116,7 +116,7 @@ cleanup:
return;
tarantool_error:
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
parse->nErr++;
goto cleanup;
}
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 82688df..39b747d 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -258,7 +258,7 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
diag_set(ClientError,
ER_ILLEGAL_COLLATION_MIX);
parse->nErr++;
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
}
return -1;
}
@@ -433,7 +433,7 @@ sql_binary_compare_coll_seq(Parse *parser, Expr *left, Expr *right,
return -1;
if (collations_check_compatibility(lhs_coll_id, is_lhs_forced,
rhs_coll_id, is_rhs_forced, id) != 0) {
- parser->rc = SQL_TARANTOOL_ERROR;
+ parser->is_aborted = true;
parser->nErr++;
return -1;
}
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 8b909f2..d8a0bda 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -506,7 +506,7 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
box_iterator_t* iter;
iter = box_index_iterator(space->def->id, 0,ITER_ALL, key_buf, key_end);
if (iter == NULL) {
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
pParse->nErr++;
goto pragma_out;
}
@@ -565,7 +565,7 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
if (!token_is_string(pValue)) {
diag_set(ClientError, ER_ILLEGAL_PARAMS,
"string value is expected");
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
pParse->nErr++;
goto pragma_out;
}
@@ -577,7 +577,7 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
sqlVdbeAddOp2(v, OP_ResultRow, 1, 1);
} else {
if (sql_default_engine_set(zRight) != 0) {
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
pParse->nErr++;
goto pragma_out;
}
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 4eaa9a4..85385ee 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -99,14 +99,13 @@ sqlPrepare(sql * db, /* Database handle. */
}
assert(0 == sParse.nQueryLoop);
- if (sParse.rc == SQL_DONE)
- sParse.rc = SQL_OK;
if (db->mallocFailed)
- sParse.rc = SQL_TARANTOOL_ERROR;
+ sParse.is_aborted = true;
if (pzTail) {
*pzTail = sParse.zTail;
}
- rc = sParse.rc;
+ if (sParse.is_aborted)
+ rc = SQL_TARANTOOL_ERROR;
if (rc == SQL_OK && sParse.pVdbe && sParse.explain) {
static const char *const azColName[] = {
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 49b0052..5b9d216 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -1341,7 +1341,7 @@ resolveSelectStep(Walker * pWalker, Select * p)
"clause or be used in an aggregate "
"function");
pParse->nErr++;
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
return WRC_Abort;
}
/*
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index ef24760..7185a10 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -2235,7 +2235,7 @@ multi_select_coll_seq_r(struct Parse *parser, struct Select *p, int n,
if (collations_check_compatibility(prior_coll_id, is_prior_forced,
current_coll_id, is_current_forced,
&res_coll_id) != 0) {
- parser->rc = SQL_TARANTOOL_ERROR;
+ parser->is_aborted = true;
parser->nErr++;
return 0;
}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index b9b45b5..22fab27 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2653,7 +2653,6 @@ struct Parse {
sql *db; /* The main database structure */
char *zErrMsg; /* An error message */
Vdbe *pVdbe; /* An engine for executing database bytecode */
- int rc; /* Return code from execution */
u8 colNamesSet; /* TRUE after OP_ColumnName has been issued to pVdbe */
u8 nTempReg; /* Number of temporary registers in aTempReg[] */
u8 isMultiWrite; /* True if statement may modify/insert multiple rows */
@@ -2688,6 +2687,8 @@ struct Parse {
u8 eOrconf; /* Default ON CONFLICT policy for trigger steps */
/** Region to make SQL temp allocations. */
struct region region;
+ /** True, if error should be raised after parsing. */
+ bool is_aborted;
/**************************************************************************
* Fields above must be initialized to zero. The fields that follow,
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 8ee996e..00a84ab 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -460,7 +460,6 @@ sqlRunParser(Parse * pParse, const char *zSql)
if (db->nVdbeActive == 0) {
db->u1.isInterrupted = 0;
}
- pParse->rc = SQL_OK;
pParse->zTail = zSql;
i = 0;
/* sqlParserTrace(stdout, "parser: "); */
@@ -484,7 +483,7 @@ sqlRunParser(Parse * pParse, const char *zSql)
if (i > mxSqlLen) {
diag_set(ClientError, ER_SQL_PARSER_GENERIC,
"string or blob too big");
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
break;
}
} else {
@@ -513,7 +512,7 @@ sqlRunParser(Parse * pParse, const char *zSql)
sqlParser(pEngine, tokenType, pParse->sLastToken,
pParse);
lastTokenParsed = tokenType;
- if (pParse->rc != SQL_OK || db->mallocFailed)
+ if (pParse->is_aborted || db->mallocFailed)
break;
}
}
@@ -524,11 +523,9 @@ sqlRunParser(Parse * pParse, const char *zSql)
);
#endif /* YYDEBUG */
sqlParserFree(pEngine, sql_free);
- if (db->mallocFailed) {
- pParse->rc = SQL_TARANTOOL_ERROR;
- }
- if (pParse->rc != SQL_OK && pParse->rc != SQL_DONE
- && pParse->zErrMsg == 0)
+ if (db->mallocFailed)
+ pParse->is_aborted = true;
+ if (pParse->is_aborted && pParse->zErrMsg == 0)
pParse->zErrMsg = sqlMPrintf(db, "%s", tarantoolErrorMessage());
if (pParse->pVdbe != NULL && pParse->nErr > 0) {
sqlVdbeDelete(pParse->pVdbe);
@@ -538,7 +535,7 @@ sqlRunParser(Parse * pParse, const char *zSql)
if (pParse->pWithToFree)
sqlWithDelete(db, pParse->pWithToFree);
sqlDbFree(db, pParse->pVList);
- return pParse->nErr > 0 || pParse->rc == SQL_TARANTOOL_ERROR ? -1 : 0;
+ return pParse->is_aborted ? -1 : 0;
}
struct Expr *
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index f7e6189..5ee0d96 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -154,7 +154,7 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm,
return;
set_tarantool_error_and_cleanup:
- parse->rc = SQL_TARANTOOL_ERROR;
+ parse->is_aborted = true;
parse->nErr++;
goto trigger_cleanup;
}
@@ -735,7 +735,7 @@ transferParseError(Parse * pTo, Parse * pFrom)
if (pTo->nErr == 0) {
pTo->zErrMsg = pFrom->zErrMsg;
pTo->nErr = pFrom->nErr;
- pTo->rc = pFrom->rc;
+ pTo->is_aborted = pFrom->is_aborted;
} else {
sqlDbFree(pFrom->db, pFrom->zErrMsg);
}
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index a6d1f5c..5aa4fda 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -212,7 +212,7 @@ sqlErrorWithMsg(sql * db, int err_code, const char *zFormat, ...)
/*
* Add an error to the diagnostics area, increment pParse->nErr
- * and set pParse->rc.
+ * and set pParse->is_aborted.
* The following formatting characters are allowed:
*
* %s Insert a string
@@ -240,14 +240,14 @@ sqlErrorMsg(Parse * pParse, const char *zFormat, ...)
diag_set(ClientError, ER_SQL_PARSER_GENERIC, zMsg);
sqlDbFree(db, zMsg);
pParse->nErr++;
- pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->is_aborted = true;
}
void
sql_parser_error(struct Parse *parse_context)
{
parse_context->nErr++;
- parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->is_aborted = true;
}
/*
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 074ff8c..f417c49 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -1203,7 +1203,7 @@ valueFromFunction(sql * db, /* The database connection */
goto value_from_function_out;
}
- assert(pCtx->pParse->rc == SQL_OK);
+ assert(!pCtx->pParse->is_aborted);
memset(&ctx, 0, sizeof(ctx));
ctx.pOut = pVal;
ctx.pFunc = pFunc;
@@ -1215,7 +1215,8 @@ valueFromFunction(sql * db, /* The database connection */
sql_value_apply_type(pVal, type);
assert(rc == SQL_OK);
}
- pCtx->pParse->rc = rc;
+ if (rc != SQL_OK)
+ pCtx->pParse->is_aborted = true;
value_from_function_out:
if (rc != SQL_OK) {
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index b46b7c3..3e1c783 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -2800,7 +2800,7 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder, /* WHERE clause information */
if (key_def == NULL) {
tnt_error:
pWInfo->pParse->nErr++;
- pWInfo->pParse->rc = SQL_TARANTOOL_ERROR;
+ pWInfo->pParse->is_aborted = true;
return SQL_TARANTOOL_ERROR;
}
More information about the Tarantool-patches
mailing list