From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 484EB2974F for ; Wed, 13 Mar 2019 13:03:16 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id z7hsPbv6R1eg for ; Wed, 13 Mar 2019 13:03:16 -0400 (EDT) Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id A77A42971E for ; Wed, 13 Mar 2019 13:03:15 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v4 4/8] sql: remove field nErr from struct Parse Date: Wed, 13 Mar 2019 20:03:13 +0300 Message-Id: <0f737d0b5d137b924914f0ef48cb7e32624e3f76.1552494059.git.imeevma@gmail.com> MIME-Version: 1.0 In-Reply-To: References: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: korablev@tarantool.org Cc: tarantool-patches@freelists.org 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 >> 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 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