[tarantool-patches] [PATCH v4 4/8] sql: remove field nErr from struct Parse
imeevma at tarantool.org
imeevma at tarantool.org
Wed Mar 13 20:03:13 MSK 2019
Hi! Thank you for review! My answers and new version of patch
below. Here won't be diff between patches as I rewritten this
patch to change its positions with one that removes rc.
On 3/5/19 12:06 PM, n.pettik wrote:
>
>> New version:
>>
>> commit 0448cc416f3401eeecbad1883fea7193399a72aa
>> Author: Mergen Imeev <imeevma at gmail.com>
>> Date: Tue Feb 26 22:22:26 2019 +0300
>>
>> sql: remove field nErr of struct Parse
>
> Nit: remove from (not of).
Fixed.
>>
>> At the moment, the only purpose of the field nErr of struct Parse
>> is to show whether the field rc of the same struct is SQL_OK or
>> SQL_TARANTOOL_ERROR. Let's remove it.
>>
>> Part of #3965
>
> The same problem with ordering here: you replaced nErr > 0 check
> with rc == SQL_TARANTOOL_ERROR and then replaced it with
> is_aborted. If you firstly replaced rc with is_aborted and then
> nErr > 0 -> is_aborted - diff would be slightly smaller.
Fixed, now removing of rc placed before removing of nErr.
>> @@ -145,9 +144,11 @@ sql_finish_coding(struct Parse *parse_context)
>> "Exit with an error if CREATE statement fails"));
>> }
>>
>> - if (db->mallocFailed || parse_context->nErr != 0) {
>> - if (parse_context->rc == SQL_OK)
>> - parse_context->rc = SQL_ERROR;
>> + if (parse_context->rc == SQL_TARANTOOL_ERROR)
>> + return;
>> + if (db->mallocFailed) {
>> + diag_set(OutOfMemory, 0, "SQL", "db”);
>
> It is not a place to set OOM. Firstly, there are a lot of other
> places where mallocFailed is set, but diag is not.
> You should set this error in sqlOomFault(). Check if any
> other function also can set this flag.
Fixed in patch "sql: set SQL parser errors via diag_set()".
>> + parse_context->rc = SQL_TARANTOOL_ERROR;
>> return;
>> }
>> /*
>> @@ -189,13 +190,12 @@ sql_finish_coding(struct Parse *parse_context)
>> sqlVdbeGoto(v, 1);
>> }
>> /* Get the VDBE program ready for execution. */
>> - if (parse_context->nErr == 0 && !db->mallocFailed) {
>> + if (parse_context->rc == SQL_OK && !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;
>> + } else if (parse_context->rc != SQL_TARANTOOL_ERROR){
>> + diag_set(OutOfMemory, 0, "SQL", "db");
>> + parse_context->rc = SQL_TARANTOOL_ERROR;
>
> The same here.
Fixed in patch "sql: set SQL parser errors via diag_set()".
>> @@ -1284,7 +1273,7 @@ sql_create_view(struct Parse *parse_context, struct Token *begin,
>> }
>> sqlStartTable(parse_context, name, if_exists);
>> struct space *space = parse_context->new_space;
>> - if (space == NULL || parse_context->nErr != 0)
>> + if (space == NULL || parse_context->rc == SQL_TARANTOOL_ERROR)
>
> Instead of checking that rc == SQL_TARANTOOL_ERROR, I’d rather
> test that rc != 0. It doesn’t matter now, since rc is anyway removed.
Fixed due to this patch goes after one that removes rc.
>> diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
>> index 58685c4..fac2781 100644
>> --- a/src/box/sql/tokenize.c
>> +++ b/src/box/sql/tokenize.c
>> @@ -483,7 +483,9 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
>> &pParse->sLastToken.isReserved);
>> i += pParse->sLastToken.n;
>> if (i > mxSqlLen) {
>> - pParse->rc = SQL_TOOBIG;
>> + diag_set(ClientError, ER_SQL_PARSER_GENERIC,
>> + "string or blob too big");
>> + pParse->rc = SQL_TARANTOOL_ERROR;
>> break;
>
> Move this change to previous patch.
Fixed.
>> @@ -502,7 +504,9 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
>> assert(tokenType == TK_SPACE
>> || tokenType == TK_ILLEGAL);
>> if (db->u1.isInterrupted) {
>> - pParse->rc = SQL_INTERRUPT;
>> + diag_set(ClientError, ER_SQL_PARSER_GENERIC,
>> + "interrupted");
>> + pParse->rc = SQL_TARANTOOL_ERROR;
>
> The same.
Fixed.
>> @@ -529,7 +533,8 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
>> #endif /* YYDEBUG */
>> sqlParserFree(pEngine, sql_free);
>> if (db->mallocFailed) {
>> - pParse->rc = SQL_NOMEM;
>> + diag_set(OutOfMemory, 0, "SQL", "db");
>> + pParse->rc = SQL_TARANTOOL_ERROR;
>> }
>
> The same.
Fixed.
>> if (pParse->rc != SQL_OK && pParse->rc != SQL_DONE
>> && pParse->zErrMsg == 0) {
>> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
>> index f67c32e..a42e872 100644
>> --- a/src/box/sql/vdbemem.c
>> +++ b/src/box/sql/vdbemem.c
>> @@ -1227,7 +1227,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->rc = SQL_TARANTOOL_ERROR;
>>
>
> Why did you change this code?
Fixed due to this patch goes after one that removes rc.
New version:
commit 0f737d0b5d137b924914f0ef48cb7e32624e3f76
Author: Mergen Imeev <imeevma at gmail.com>
Date: Wed Mar 6 22:33:10 2019 +0300
sql: remove field nErr from struct Parse
At the moment, the only purpose of the field nErr of struct Parse
is to show whether the field is_aborted of the same struct is true
or false. Let's remove it.
Part of #3965
diff --git a/src/box/sql.c b/src/box/sql.c
index c2e5d6b..1004bb7 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1276,7 +1276,6 @@ sql_ephemeral_space_def_new(struct Parse *parser, const char *name)
diag_set(OutOfMemory, size, "region_alloc",
"sql_ephemeral_space_def_new");
parser->is_aborted = true;
- parser->nErr++;
return NULL;
}
@@ -1295,7 +1294,6 @@ sql_ephemeral_space_new(Parse *parser, const char *name)
if (space == NULL) {
diag_set(OutOfMemory, sz, "region", "space");
parser->is_aborted = true;
- parser->nErr++;
return NULL;
}
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index fe4754f..bd9b034 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -73,7 +73,6 @@ exit_rename_table:
tnt_error:
sqlDbFree(db, new_name);
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 96b7099..f95b34b 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -908,7 +908,6 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space)
diag_set(OutOfMemory, sizeof(int) * part_count,
"region", "jump_addrs");
parse->is_aborted = true;
- parse->nErr++;
return;
}
/*
@@ -1131,7 +1130,6 @@ sqlAnalyze(Parse * pParse, Token * pName)
} else {
diag_set(ClientError, ER_NO_SUCH_SPACE, z);
pParse->is_aborted = true;
- pParse->nErr++;
}
sqlDbFree(db, z);
}
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 0179a45..0c06555 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -92,7 +92,6 @@ save_record(struct Parse *parser, uint32_t space_id, int reg_key,
diag_set(OutOfMemory, sizeof(*record), "region_alloc",
"record");
parser->is_aborted = true;
- parser->nErr++;
return;
}
record->space_id = space_id;
@@ -145,10 +144,10 @@ sql_finish_coding(struct Parse *parse_context)
"Exit with an error if CREATE statement fails"));
}
- if (db->mallocFailed || parse_context->nErr != 0) {
+ if (db->mallocFailed)
parse_context->is_aborted = true;
+ if (parse_context->is_aborted)
return;
- }
/*
* Begin by generating some termination code at the end
* of the vdbe program
@@ -188,7 +187,7 @@ sql_finish_coding(struct Parse *parse_context)
sqlVdbeGoto(v, 1);
}
/* Get the VDBE program ready for execution. */
- if (parse_context->nErr == 0 && !db->mallocFailed) {
+ if (!parse_context->is_aborted && !db->mallocFailed) {
assert(parse_context->iCacheLevel == 0);
sqlVdbeMakeReady(v, parse_context);
} else {
@@ -342,7 +341,7 @@ sqlStartTable(Parse *pParse, Token *pName, int noErr)
if (space != NULL) {
if (!noErr) {
diag_set(ClientError, ER_SPACE_EXISTS, zName);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
} else {
assert(!db->init.busy || CORRUPT_DB);
}
@@ -395,7 +394,6 @@ sql_field_retrieve(Parse *parser, struct space_def *space_def, uint32_t id)
sizeof(space_def->fields[0]),
"region_alloc", "sql_field_retrieve");
parser->is_aborted = true;
- parser->nErr++;
return NULL;
}
@@ -451,7 +449,6 @@ sqlAddColumn(Parse * pParse, Token * pName, struct type_def *type_def)
diag_set(OutOfMemory, pName->n + 1,
"region_alloc", "z");
pParse->is_aborted = true;
- pParse->nErr++;
return;
}
memcpy(z, pName->z, pName->n);
@@ -460,7 +457,7 @@ sqlAddColumn(Parse * pParse, Token * pName, struct type_def *type_def)
for (uint32_t i = 0; i < def->field_count; i++) {
if (strcmp(z, def->fields[i].name) == 0) {
diag_set(ClientError, ER_SPACE_FIELD_IS_DUPLICATE, z);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
return;
}
}
@@ -499,7 +496,6 @@ sql_column_add_nullable_action(struct Parse *parser,
nullable_action]);
diag_set(ClientError, ER_SQL, err_msg);
parser->is_aborted = true;
- parser->nErr++;
return;
}
field->nullable_action = nullable_action;
@@ -541,7 +537,6 @@ sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan)
"region_alloc",
"field->default_value");
pParse->is_aborted = true;
- pParse->nErr++;
return;
}
strncpy(field->default_value, pSpan->zStart,
@@ -560,7 +555,6 @@ field_def_create_for_pk(struct Parse *parser, struct field_def *field,
field->nullable_action != ON_CONFLICT_ACTION_DEFAULT) {
diag_set(ClientError, ER_NULLABLE_PRIMARY, space_name);
parser->is_aborted = true;
- parser->nErr++;
return -1;
} else if (field->nullable_action == ON_CONFLICT_ACTION_DEFAULT) {
field->nullable_action = ON_CONFLICT_ACTION_ABORT;
@@ -649,7 +643,7 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */
sql_create_index(pParse, 0, 0, pList, 0, sortOrder, false,
SQL_INDEX_TYPE_CONSTRAINT_PK);
pList = 0;
- if (pParse->nErr > 0)
+ if (pParse->is_aborted)
goto primary_key_exit;
}
@@ -849,7 +843,6 @@ vdbe_emit_create_index(struct Parse *parse, struct space_def *def,
return;
error:
parse->is_aborted = true;
- parse->nErr++;
}
@@ -908,7 +901,6 @@ createSpace(Parse * pParse, int iSpaceId, char *zStmt)
save_record(pParse, BOX_SPACE_ID, iFirstCol, 1, v->nOp - 1);
return;
error:
- pParse->nErr++;
pParse->is_aborted = true;
}
@@ -1089,7 +1081,6 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context,
sqlReleaseTempRange(parse_context, constr_tuple_reg, 10);
return;
error:
- parse_context->nErr++;
parse_context->is_aborted = true;
}
@@ -1119,7 +1110,6 @@ resolve_link(struct Parse *parse_context, const struct space_def *def,
tt_sprintf("unknown column %s in foreign key definition",
field_name));
parse_context->is_aborted = true;
- parse_context->nErr++;
return -1;
}
@@ -1251,7 +1241,6 @@ sqlEndTable(Parse * pParse, /* Parse context */
"number of columns in the primary "\
"index of referenced table");
pParse->is_aborted = true;
- pParse->nErr++;
return;
}
for (uint32_t i = 0; i < fk_def->field_count; ++i) {
@@ -1281,7 +1270,7 @@ sql_create_view(struct Parse *parse_context, struct Token *begin,
}
sqlStartTable(parse_context, name, if_exists);
struct space *space = parse_context->new_space;
- if (space == NULL || parse_context->nErr != 0)
+ if (space == NULL || parse_context->is_aborted)
goto create_view_fail;
struct space *select_res_space =
@@ -1327,7 +1316,6 @@ sql_create_view(struct Parse *parse_context, struct Token *begin,
if (space->def->opts.sql == NULL) {
diag_set(OutOfMemory, n, "strndup", "opts.sql");
parse_context->is_aborted = true;
- parse_context->nErr++;
goto create_view_fail;
}
@@ -1600,14 +1588,14 @@ sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list,
goto exit_drop_table;
}
sqlVdbeCountChanges(v);
- assert(parse_context->nErr == 0);
+ assert(!parse_context->is_aborted);
assert(table_name_list->nSrc == 1);
const char *space_name = table_name_list->a[0].zName;
struct space *space = space_by_name(space_name);
if (space == NULL) {
if (!if_exists) {
diag_set(ClientError, ER_NO_SUCH_SPACE, space_name);
- sql_parser_error(parse_context);
+ parse_context->is_aborted = true;
}
goto exit_drop_table;
}
@@ -1647,7 +1635,6 @@ sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list,
diag_set(ClientError, ER_DROP_SPACE, space_name,
"other objects depend on it");
parse_context->is_aborted = true;
- parse_context->nErr++;
goto exit_drop_table;
}
}
@@ -1683,7 +1670,6 @@ columnno_by_name(struct Parse *parse_context, const struct space *space,
tt_sprintf("foreign key refers to nonexistent field %s",
column_name));
parse_context->is_aborted = true;
- parse_context->nErr++;
return -1;
}
return 0;
@@ -1893,7 +1879,6 @@ exit_create_fk:
return;
tnt_error:
parse_context->is_aborted = true;
- parse_context->nErr++;
goto exit_create_fk;
}
@@ -1918,7 +1903,6 @@ sql_drop_foreign_key(struct Parse *parse_context, struct SrcList *table,
if (child == NULL) {
diag_set(ClientError, ER_NO_SUCH_SPACE, table_name);
parse_context->is_aborted = true;
- parse_context->nErr++;
return;
}
char *constraint_name = sqlNameFromToken(parse_context->db,
@@ -2045,7 +2029,7 @@ index_fill_def(struct Parse *parse, struct index *index,
struct Expr *expr = expr_list->a[i].pExpr;
sql_resolve_self_reference(parse, space_def, NC_IdxExpr,
expr, 0);
- if (parse->nErr > 0)
+ if (parse->is_aborted)
goto cleanup;
struct Expr *column_expr = sqlExprSkipCollate(expr);
@@ -2097,7 +2081,6 @@ cleanup:
return rc;
tnt_error:
parse->is_aborted = true;
- ++parse->nErr;
goto cleanup;
}
@@ -2125,7 +2108,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
struct sql *db = parse->db;
assert(!db->init.busy);
- if (db->mallocFailed || parse->nErr > 0)
+ if (db->mallocFailed || parse->is_aborted)
goto exit_create_index;
if (idx_type == SQL_INDEX_TYPE_UNIQUE ||
idx_type == SQL_INDEX_TYPE_NON_UNIQUE) {
@@ -2148,7 +2131,6 @@ sql_create_index(struct Parse *parse, struct Token *token,
if (! if_not_exist) {
diag_set(ClientError, ER_NO_SUCH_SPACE, name);
parse->is_aborted = true;
- parse->nErr++;
}
goto exit_create_index;
}
@@ -2194,7 +2176,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
if (!if_not_exist) {
diag_set(ClientError, ER_INDEX_EXISTS_IN_SPACE,
name, def->name);
- sql_parser_error(parse);
+ parse->is_aborted = true;
}
goto exit_create_index;
}
@@ -2241,7 +2223,6 @@ sql_create_index(struct Parse *parse, struct Token *token,
if (tbl_name != NULL && space_is_system(space)) {
diag_set(ClientError, ER_MODIFY_INDEX, name, def->name,
"can't create index on system space");
- parse->nErr++;
parse->is_aborted = true;
goto exit_create_index;
}
@@ -2271,7 +2252,6 @@ sql_create_index(struct Parse *parse, struct Token *token,
if (index == NULL) {
diag_set(OutOfMemory, sizeof(*index), "region", "index");
parse->is_aborted = true;
- parse->nErr++;
goto exit_create_index;
}
memset(index, 0, sizeof(*index));
@@ -2441,7 +2421,7 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list,
assert(v != NULL);
struct sql *db = parse_context->db;
/* Never called with prior errors. */
- assert(parse_context->nErr == 0);
+ assert(!parse_context->is_aborted);
assert(table_token != NULL);
const char *table_name = sqlNameFromToken(db, table_token);
if (db->mallocFailed) {
@@ -2454,7 +2434,7 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list,
if (space == NULL) {
if (!if_exists) {
diag_set(ClientError, ER_NO_SUCH_SPACE, table_name);
- sql_parser_error(parse_context);
+ parse_context->is_aborted = true;
}
goto exit_drop_index;
}
@@ -2465,7 +2445,7 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list,
if (!if_exists) {
diag_set(ClientError, ER_NO_SUCH_INDEX_NAME,
index_name, table_name);
- sql_parser_error(parse_context);
+ parse_context->is_aborted = true;
}
goto exit_drop_index;
}
@@ -2823,7 +2803,7 @@ sqlSrcListAppendFromTerm(Parse * pParse, /* Parsing context */
if (!p && (pOn || pUsing)) {
diag_set(ClientError, ER_SQL_SYNTAX, "FROM clause",
"a JOIN clause is required before ON and USING");
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
goto append_from_error;
}
p = sqlSrcListAppend(db, p, pTable);
diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
index 5c9cf98..4919753 100644
--- a/src/box/sql/callback.c
+++ b/src/box/sql/callback.c
@@ -50,7 +50,6 @@ sql_get_coll_seq(Parse *parser, const char *name, uint32_t *coll_id)
if (p == NULL) {
diag_set(ClientError, ER_NO_SUCH_COLLATION, name);
parser->is_aborted = true;
- parser->nErr++;
return NULL;
} else {
*coll_id = p->id;
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 3123681..87d4ed4 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -43,7 +43,7 @@ sql_lookup_space(struct Parse *parse, struct SrcList_item *space_name)
struct space *space = space_by_name(space_name->zName);
if (space == NULL) {
diag_set(ClientError, ER_NO_SUCH_SPACE, space_name->zName);
- sql_parser_error(parse);
+ parse->is_aborted = true;
return NULL;
}
assert(space != NULL);
@@ -51,7 +51,6 @@ sql_lookup_space(struct Parse *parse, struct SrcList_item *space_name)
diag_set(ClientError, ER_UNSUPPORTED, "SQL",
"space without format");
parse->is_aborted = true;
- parse->nErr++;
return NULL;
}
space_name->space = space;
@@ -117,7 +116,6 @@ cleanup:
tarantool_error:
parse->is_aborted = true;
- parse->nErr++;
goto cleanup;
}
@@ -126,7 +124,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
struct Expr *where)
{
struct sql *db = parse->db;
- if (parse->nErr || db->mallocFailed)
+ if (parse->is_aborted || db->mallocFailed)
goto delete_from_cleanup;
assert(tab_list->nSrc == 1);
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 39b747d..4193596 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -254,10 +254,9 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
* several times: this
* function is recursive.
*/
- if (parse->nErr == 0) {
+ if (!parse->is_aborted) {
diag_set(ClientError,
ER_ILLEGAL_COLLATION_MIX);
- parse->nErr++;
parse->is_aborted = true;
}
return -1;
@@ -434,7 +433,6 @@ sql_binary_compare_coll_seq(Parse *parser, Expr *left, Expr *right,
if (collations_check_compatibility(lhs_coll_id, is_lhs_forced,
rhs_coll_id, is_rhs_forced, id) != 0) {
parser->is_aborted = true;
- parser->nErr++;
return -1;
}
return 0;
@@ -844,7 +842,7 @@ exprSetHeight(Expr * p)
void
sqlExprSetHeightAndFlags(Parse * pParse, Expr * p)
{
- if (pParse->nErr)
+ if (pParse->is_aborted)
return;
exprSetHeight(p);
sqlExprCheckHeight(pParse, p->nHeight);
@@ -1028,7 +1026,7 @@ sqlPExpr(Parse * pParse, /* Parsing context */
)
{
Expr *p;
- if (op == TK_AND && pParse->nErr == 0) {
+ if (op == TK_AND && !pParse->is_aborted) {
/* Take advantage of short-circuit false optimization for AND */
p = sqlExprAnd(pParse->db, pLeft, pRight);
} else {
@@ -2430,7 +2428,7 @@ sqlFindInIndex(Parse * pParse, /* Parsing context */
* satisfy the query. This is preferable to generating a new
* ephemeral table.
*/
- if (pParse->nErr == 0 && (p = isCandidateForInOpt(pX)) != 0) {
+ if (!pParse->is_aborted && (p = isCandidateForInOpt(pX)) != 0) {
sql *db = pParse->db; /* Database connection */
ExprList *pEList = p->pEList;
int nExpr = pEList->nExpr;
@@ -3064,7 +3062,7 @@ sqlExprCodeIN(Parse * pParse, /* Parsing and code generating context */
destIfFalse == destIfNull ? 0 : &rRhsHasNull,
aiMap, 0);
- assert(pParse->nErr || nVector == 1 || eType == IN_INDEX_EPH
+ assert(pParse->is_aborted || nVector == 1 || eType == IN_INDEX_EPH
|| eType == IN_INDEX_INDEX_ASC || eType == IN_INDEX_INDEX_DESC);
#ifdef SQL_DEBUG
/* Confirm that aiMap[] contains nVector integer values between 0 and
@@ -3387,7 +3385,7 @@ sqlExprCacheStore(Parse * pParse, int iTab, int iCol, int iReg)
struct yColCache *p;
/* Unless an error has occurred, register numbers are always positive. */
- assert(iReg > 0 || pParse->nErr || pParse->db->mallocFailed);
+ assert(iReg > 0 || pParse->is_aborted || pParse->db->mallocFailed);
assert(iCol >= -1 && iCol < 32768); /* Finite column numbers */
/* The SQL_ColumnCache flag disables the column cache. This is used
@@ -4020,7 +4018,7 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
if (pDef == 0 || pDef->xFinalize != 0) {
diag_set(ClientError, ER_NO_SUCH_FUNCTION,
zId);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
break;
}
@@ -4364,7 +4362,7 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
} else {
sqlVdbeAddOp2(v, OP_Null, 0, target);
}
- assert(pParse->db->mallocFailed || pParse->nErr > 0
+ assert(pParse->db->mallocFailed || pParse->is_aborted
|| pParse->iCacheLevel == iCacheLevel);
sqlVdbeResolveLabel(v, endLabel);
break;
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 17fbdec..6f7f020 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -277,7 +277,7 @@ sqlInsert(Parse * pParse, /* Parser context */
db = pParse->db;
memset(&dest, 0, sizeof(dest));
- if (pParse->nErr || db->mallocFailed) {
+ if (pParse->is_aborted || db->mallocFailed) {
goto insert_cleanup;
}
@@ -426,7 +426,7 @@ sqlInsert(Parse * pParse, /* Parser context */
dest.nSdst = space_def->field_count;
rc = sqlSelect(pParse, pSelect, &dest);
regFromSelect = dest.iSdst;
- if (rc || db->mallocFailed || pParse->nErr)
+ if (rc || db->mallocFailed || pParse->is_aborted)
goto insert_cleanup;
sqlVdbeEndCoroutine(v, regYield);
sqlVdbeJumpHere(v, addrTop - 1); /* label B: */
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 66fb44b..3e86a7f 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -38,11 +38,11 @@
} else {
diag_set(ClientError, ER_SQL_UNRECOGNIZED_SYNTAX, TOKEN.n, TOKEN.z);
}
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
}
%stack_overflow {
diag_set(ClientError, ER_SQL_STACK_OVERFLOW);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
}
// The name of the generated procedure that implements the parser
@@ -118,7 +118,7 @@ ecmd ::= explain cmdx SEMI. {
}
ecmd ::= SEMI. {
diag_set(ClientError, ER_SQL_STATEMENT_EMPTY);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
}
explain ::= .
explain ::= EXPLAIN. { pParse->explain = 1; }
@@ -232,7 +232,7 @@ columnname(A) ::= nm(A) typedef(Y). {sqlAddColumn(pParse,&A,&Y);}
nm(A) ::= id(A). {
if(A.isReserved) {
diag_set(ClientError, ER_SQL_KEYWORD_IS_RESERVED, A.n, A.z, A.n, A.z);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
}
}
@@ -904,7 +904,7 @@ expr(A) ::= VARIABLE(X). {
spanExpr(&A, pParse, TK_VARIABLE, X);
if (A.pExpr->u.zToken[0] == '?' && n > 1) {
diag_set(ClientError, ER_SQL_UNRECOGNIZED_SYNTAX, t.n, t.z);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
} else {
sqlExprAssignVarNumber(pParse, A.pExpr, n);
}
@@ -912,7 +912,7 @@ expr(A) ::= VARIABLE(X). {
assert( t.n>=2 );
spanSet(&A, &t, &t);
diag_set(ClientError, ER_SQL_UNRECOGNIZED_SYNTAX, t.n, t.z);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
A.pExpr = NULL;
}
}
@@ -1388,13 +1388,13 @@ tridxby ::= .
tridxby ::= INDEXED BY nm. {
diag_set(ClientError, ER_SQL_SYNTAX, "trigger body", "the INDEXED BY clause "\
"is not allowed on UPDATE or DELETE statements within triggers");
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
}
tridxby ::= NOT INDEXED. {
diag_set(ClientError, ER_SQL_SYNTAX, "trigger body", "the NOT INDEXED "\
"clause is not allowed on UPDATE or DELETE statements within "\
"triggers");
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
}
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index d8a0bda..2b9c9b4 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -440,7 +440,7 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
pPragma = pragmaLocate(zLeft);
if (pPragma == 0) {
diag_set(ClientError, ER_SQL_NO_SUCH_PRAGMA, zLeft);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
goto pragma_out;
}
/* Register the result column names for pragmas that return results */
@@ -507,7 +507,6 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
iter = box_index_iterator(space->def->id, 0,ITER_ALL, key_buf, key_end);
if (iter == NULL) {
pParse->is_aborted = true;
- pParse->nErr++;
goto pragma_out;
}
int rc = box_iterator_next(iter, &tuple);
@@ -566,7 +565,6 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
diag_set(ClientError, ER_ILLEGAL_PARAMS,
"string value is expected");
pParse->is_aborted = true;
- pParse->nErr++;
goto pragma_out;
}
if (zRight == NULL) {
@@ -578,7 +576,6 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
} else {
if (sql_default_engine_set(zRight) != 0) {
pParse->is_aborted = true;
- pParse->nErr++;
goto pragma_out;
}
sqlVdbeAddOp0(v, OP_Expire);
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 5b9d216..94bb0af 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -441,7 +441,7 @@ lookupName(Parse * pParse, /* The parsing context */
diag_set(ClientError, ER_NO_SUCH_FIELD_NAME, zCol,
zTab);
}
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
pTopNC->nErr++;
}
@@ -707,7 +707,7 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
#endif
) {
diag_set(ClientError, ER_NO_SUCH_FUNCTION, zId);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
pNC->nErr++;
} else if (wrong_num_args) {
sqlErrorMsg(pParse,
@@ -809,7 +809,7 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
break;
}
}
- return (pParse->nErr
+ return (pParse->is_aborted
|| pParse->db->mallocFailed) ? WRC_Abort : WRC_Continue;
}
@@ -1195,7 +1195,7 @@ resolveSelectStep(Walker * pWalker, Select * p)
*/
if ((p->selFlags & SF_Expanded) == 0) {
sqlSelectPrep(pParse, p, pOuterNC);
- return (pParse->nErr
+ return (pParse->is_aborted
|| db->mallocFailed) ? WRC_Abort : WRC_Prune;
}
@@ -1252,7 +1252,7 @@ resolveSelectStep(Walker * pWalker, Select * p)
sqlResolveSelectNames(pParse,
pItem->pSelect,
pOuterNC);
- if (pParse->nErr || db->mallocFailed)
+ if (pParse->is_aborted || db->mallocFailed)
return WRC_Abort;
for (pNC = pOuterNC; pNC; pNC = pNC->pNext)
@@ -1340,7 +1340,6 @@ resolveSelectStep(Walker * pWalker, Select * p)
"argument must appear in the GROUP BY "
"clause or be used in an aggregate "
"function");
- pParse->nErr++;
pParse->is_aborted = true;
return WRC_Abort;
}
@@ -1536,7 +1535,7 @@ sqlResolveExprNames(NameContext * pNC, /* Namespace to resolve expressions in. *
#if SQL_MAX_EXPR_DEPTH>0
pNC->pParse->nHeight -= pExpr->nHeight;
#endif
- if (pNC->nErr > 0 || w.pParse->nErr > 0) {
+ if (pNC->nErr > 0 || w.pParse->is_aborted) {
ExprSetProperty(pExpr, EP_Error);
}
if (pNC->ncFlags & NC_HasAgg) {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 7185a10..e83b898 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -196,13 +196,13 @@ sqlSelectNew(Parse * pParse, /* Parsing context */
pNew->pLimit = pLimit;
pNew->pOffset = pOffset;
pNew->pWith = 0;
- assert(pOffset == 0 || pLimit != 0 || pParse->nErr > 0
+ assert(pOffset == 0 || pLimit != 0 || pParse->is_aborted
|| db->mallocFailed != 0);
if (db->mallocFailed) {
clearSelect(db, pNew, pNew != &standin);
pNew = 0;
} else {
- assert(pNew->pSrc != 0 || pParse->nErr > 0);
+ assert(pNew->pSrc != 0 || pParse->is_aborted);
}
assert(pNew != &standin);
return pNew;
@@ -2003,7 +2003,7 @@ sqlResultSetOfSelect(Parse * pParse, Select * pSelect)
user_session->sql_flags |= ~SQL_FullColNames;
user_session->sql_flags &= SQL_ShortColNames;
sqlSelectPrep(pParse, pSelect, 0);
- if (pParse->nErr)
+ if (pParse->is_aborted)
return NULL;
while (pSelect->pPrior)
pSelect = pSelect->pPrior;
@@ -2094,7 +2094,7 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
(p->pOffset->flags & EP_Collate) != 0)) {
diag_set(ClientError, ER_SQL_UNRECOGNIZED_SYNTAX,
sizeof("COLLATE"), "COLLATE");
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
return;
}
p->iLimit = iLimit = ++pParse->nMem;
@@ -2236,7 +2236,6 @@ multi_select_coll_seq_r(struct Parse *parser, struct Select *p, int n,
current_coll_id, is_current_forced,
&res_coll_id) != 0) {
parser->is_aborted = true;
- parser->nErr++;
return 0;
}
*is_forced_coll = (is_prior_forced || is_current_forced);
@@ -3115,7 +3114,7 @@ generateOutputSubroutine(struct Parse *parse, struct Select *p,
* of the scan loop.
*/
case SRT_Mem:{
- assert(in->nSdst == 1 || parse->nErr > 0);
+ assert(in->nSdst == 1 || parse->is_aborted);
testcase(in->nSdst != 1);
sqlExprCodeMove(parse, in->iSdst, dest->iSDParm,
1);
@@ -3585,7 +3584,7 @@ multiSelectOrderBy(Parse * pParse, /* Parsing context */
*** subqueries ***
*/
explainComposite(pParse, p->op, iSub1, iSub2, 0);
- return pParse->nErr != 0;
+ return pParse->is_aborted;
}
#endif
@@ -4428,7 +4427,7 @@ sqlIndexedByLookup(Parse * pParse, struct SrcList_item *pFrom)
if (idx == NULL) {
diag_set(ClientError, ER_NO_SUCH_INDEX_NAME,
zIndexedBy, space->def->name);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
return SQL_ERROR;
}
pFrom->pIBIndex = idx->def;
@@ -5054,7 +5053,7 @@ selectExpander(Walker * pWalker, Select * p)
diag_set(ClientError,
ER_SQL_SELECT_WILDCARD);
}
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
}
}
}
@@ -5096,7 +5095,7 @@ sqlExprWalkNoop(Walker * NotUsed, Expr * NotUsed2)
* name resolution is performed.
*
* If anything goes wrong, an error message is written into pParse.
- * The calling function can detect the problem by looking at pParse->nErr
+ * The calling function can detect the problem by looking at pParse->is_aborted
* and/or pParse->db->mallocFailed.
*/
static void
@@ -5205,10 +5204,10 @@ sqlSelectPrep(Parse * pParse, /* The parser context */
if (p->selFlags & SF_HasTypeInfo)
return;
sqlSelectExpand(pParse, p);
- if (pParse->nErr || db->mallocFailed)
+ if (pParse->is_aborted || db->mallocFailed)
return;
sqlResolveSelectNames(pParse, p, pOuterNC);
- if (pParse->nErr || db->mallocFailed)
+ if (pParse->is_aborted || db->mallocFailed)
return;
sqlSelectAddTypeInfo(pParse, p);
}
@@ -5463,7 +5462,7 @@ sqlSelect(Parse * pParse, /* The parser context */
pParse->iSelectId = pParse->iNextSelectId++;
db = pParse->db;
- if (p == 0 || db->mallocFailed || pParse->nErr) {
+ if (p == 0 || db->mallocFailed || pParse->is_aborted) {
return 1;
}
memset(&sAggInfo, 0, sizeof(sAggInfo));
@@ -5498,7 +5497,7 @@ sqlSelect(Parse * pParse, /* The parser context */
memset(&sSort, 0, sizeof(sSort));
sSort.pOrderBy = p->pOrderBy;
pTabList = p->pSrc;
- if (pParse->nErr || db->mallocFailed) {
+ if (pParse->is_aborted || db->mallocFailed) {
goto select_end;
}
assert(p->pEList != 0);
@@ -6392,7 +6391,7 @@ sqlSelect(Parse * pParse, /* The parser context */
/* The SELECT has been coded. If there is an error in the Parse structure,
* set the return code to 1. Otherwise 0.
*/
- rc = (pParse->nErr > 0);
+ rc = (pParse->is_aborted);
/* Control jumps to here if an error is encountered above, or upon
* successful coding of the SELECT.
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index dd21091..b05e67b 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2665,7 +2665,6 @@ struct Parse {
u8 nColCache; /* Number of entries in aColCache[] */
int nRangeReg; /* Size of the temporary register block */
int iRangeReg; /* First register in temporary register block */
- int nErr; /* Number of errors seen */
int nTab; /* Number of previously allocated VDBE cursors */
int nMem; /* Number of memory cells used so far */
int nOpAlloc; /* Number of slots allocated for Vdbe.aOp[] */
@@ -3204,14 +3203,6 @@ int sqlKeywordCode(const unsigned char *, int);
int sqlRunParser(Parse *, const char *);
/**
- * Increment error counter.
- *
- * @param parse_context Current parsing context.
- */
-void
-sql_parser_error(struct Parse *parse_context);
-
-/**
* This routine is called after a single SQL statement has been
* parsed and a VDBE program to execute that statement has been
* prepared. This routine puts the finishing touches on the
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 5c23ec0..be04c17 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -511,7 +511,7 @@ sqlRunParser(Parse * pParse, const char *zSql)
diag_set(ClientError, ER_SQL_UNKNOWN_TOKEN,
pParse->sLastToken.n,
pParse->sLastToken.z);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
break;
}
} else {
@@ -533,7 +533,7 @@ sqlRunParser(Parse * pParse, const char *zSql)
pParse->is_aborted = true;
if (pParse->is_aborted && pParse->zErrMsg == 0)
pParse->zErrMsg = sqlMPrintf(db, "%s", tarantoolErrorMessage());
- if (pParse->pVdbe != NULL && pParse->nErr > 0) {
+ if (pParse->pVdbe != NULL && pParse->is_aborted) {
sqlVdbeDelete(pParse->pVdbe);
pParse->pVdbe = 0;
}
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 5ee0d96..7eacd33 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -155,7 +155,6 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm,
set_tarantool_error_and_cleanup:
parse->is_aborted = true;
- parse->nErr++;
goto trigger_cleanup;
}
@@ -169,7 +168,7 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list,
struct sql *db = parse->db;
parse->parsed_ast.trigger = NULL;
- if (NEVER(parse->nErr) || trigger == NULL)
+ if (NEVER(parse->is_aborted) || trigger == NULL)
goto cleanup;
char *trigger_name = trigger->zName;
trigger->step_list = step_list;
@@ -730,11 +729,10 @@ onErrorText(int onError)
static void
transferParseError(Parse * pTo, Parse * pFrom)
{
- assert(pFrom->zErrMsg == 0 || pFrom->nErr);
- assert(pTo->zErrMsg == 0 || pTo->nErr);
- if (pTo->nErr == 0) {
+ assert(pFrom->zErrMsg == 0 || pFrom->is_aborted);
+ assert(pTo->zErrMsg == 0 || pTo->is_aborted);
+ if (!pTo->is_aborted) {
pTo->zErrMsg = pFrom->zErrMsg;
- pTo->nErr = pFrom->nErr;
pTo->is_aborted = pFrom->is_aborted;
} else {
sqlDbFree(pFrom->db, pFrom->zErrMsg);
@@ -924,7 +922,7 @@ vdbe_code_row_trigger_direct(struct Parse *parser, struct sql_trigger *trigger,
struct Vdbe *v = sqlGetVdbe(parser);
TriggerPrg *pPrg = sql_row_trigger(parser, trigger, space, orconf);
- assert(pPrg != NULL || parser->nErr != 0 ||
+ assert(pPrg != NULL || parser->is_aborted ||
parser->db->mallocFailed != 0);
/*
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 670e547..05ceeb4 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -116,7 +116,7 @@ sqlUpdate(Parse * pParse, /* The parser context */
int upd_cols_cnt = 0;
db = pParse->db;
- if (pParse->nErr || db->mallocFailed) {
+ if (pParse->is_aborted || db->mallocFailed) {
goto update_cleanup;
}
assert(pTabList->nSrc == 1);
@@ -192,7 +192,7 @@ sqlUpdate(Parse * pParse, /* The parser context */
if (j >= (int)def->field_count) {
diag_set(ClientError, ER_NO_SUCH_FIELD_NAME,
pChanges->a[i].zName, def->name);
- sql_parser_error(pParse);
+ pParse->is_aborted = true;
goto update_cleanup;
}
}
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index 5aa4fda..d9bb2af 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -211,8 +211,8 @@ sqlErrorWithMsg(sql * db, int err_code, const char *zFormat, ...)
}
/*
- * Add an error to the diagnostics area, increment pParse->nErr
- * and set pParse->is_aborted.
+ * Add an error to the diagnostics area and set
+ * pParse->is_aborted.
* The following formatting characters are allowed:
*
* %s Insert a string
@@ -239,17 +239,9 @@ sqlErrorMsg(Parse * pParse, const char *zFormat, ...)
va_end(ap);
diag_set(ClientError, ER_SQL_PARSER_GENERIC, zMsg);
sqlDbFree(db, zMsg);
- pParse->nErr++;
pParse->is_aborted = true;
}
-void
-sql_parser_error(struct Parse *parse_context)
-{
- parse_context->nErr++;
- parse_context->is_aborted = true;
-}
-
/*
* Convert an SQL-style quoted string into a normal string by removing
* the quote characters. The conversion is done in-place. If the
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 6c5c61e..cf70e06 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -2799,7 +2799,6 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder, /* WHERE clause information */
struct key_def *key_def = key_def_new(&part, 1);
if (key_def == NULL) {
tnt_error:
- pWInfo->pParse->nErr++;
pWInfo->pParse->is_aborted = true;
return SQL_TARANTOOL_ERROR;
}
@@ -4458,7 +4457,7 @@ sqlWhereBegin(Parse * pParse, /* The parser context */
(user_session->sql_flags & SQL_ReverseOrder) != 0) {
pWInfo->revMask = ALLBITS;
}
- if (pParse->nErr || NEVER(db->mallocFailed)) {
+ if (pParse->is_aborted || NEVER(db->mallocFailed)) {
goto whereBeginError;
}
#ifdef SQL_DEBUG
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 018fd8a..e1c3366 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1472,7 +1472,7 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W
sqlWhereBegin(pParse, pOrTab, pOrExpr,
0, 0, wctrlFlags,
iCovCur);
- assert(pSubWInfo || pParse->nErr
+ assert(pSubWInfo || pParse->is_aborted
|| db->mallocFailed);
if (pSubWInfo) {
WhereLoop *pSubLoop;
--
2.7.4
More information about the Tarantool-patches
mailing list