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 CCDDD2974F for ; Wed, 13 Mar 2019 13:03:17 -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 HDeIfZ2mlGeL for ; Wed, 13 Mar 2019 13:03:17 -0400 (EDT) Received: from smtp15.mail.ru (smtp15.mail.ru [94.100.176.133]) (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 5DEA229773 for ; Wed, 13 Mar 2019 13:03:17 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v4 5/8] sql: remove field zErrMsg from struct Parse Date: Wed, 13 Mar 2019 20:03:15 +0300 Message-Id: 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 due to some of its code were moved to previous patches. On 3/5/19 12:06 PM, n.pettik wrote: > >>>> @@ -146,11 +145,7 @@ sqlPrepare(sql * db, /* Database handle. */ >>>> *ppStmt = (sql_stmt *) sParse.pVdbe; >>>> } >>>> >>>> - if (zErrMsg) { >>>> - sqlErrorWithMsg(db, rc, "%s", zErrMsg); >>>> - } else { >>>> - sqlError(db, rc); >>>> - } >>>> + sqlError(db, rc); >>> >>> sqlError seems to be useless/dead. Please, make a note somewhere >>> to remove it as follow-up to error-refactoring patch-set. >>> >> Ok, made a note. > > And where is it? Please, paste a link to it. I will compile all of notices to this issue and send them a bit later. >> New version: >> >> commit 29c77ea3f8463994b98bcb23653f901cf46e472a >> Author: Mergen Imeev >> Date: Wed Feb 27 09:40:17 2019 +0300 >> >> sql: remove field zErrMsg of struct Parse >> >> This field become unused and should be removed. >> >> Part of #3965 >> >> diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c >> index 42737ff..4359fa7 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); >> >> @@ -169,11 +168,7 @@ sqlPrepare(sql * db, /* Database handle. */ >> *ppStmt = (sql_stmt *) sParse.pVdbe; >> } >> >> - if (zErrMsg) { >> - sqlErrorWithMsg(db, rc, "%s", zErrMsg); >> - } else { >> - sqlError(db, rc); >> - } >> + sqlError(db, rc); > > As we pointed out, you can remove this call. Removed. >> >> /* Delete any TriggerPrg structures allocated while parsing this statement. */ >> while (sParse.pTriggerPrg) { >> @@ -318,7 +313,6 @@ sql_parser_destroy(Parse *parser) >> db->lookaside.bDisable -= parser->disableLookaside; >> } >> parser->disableLookaside = 0; >> - sqlDbFree(db, parser->zErrMsg); >> switch (parser->parsed_ast_type) { >> case AST_TYPE_SELECT: >> sql_select_delete(db, parser->parsed_ast.select); >> diff --git a/src/box/sql/select.c b/src/box/sql/select.c >> index cfac553..66cbc73 100644 >> --- a/src/box/sql/select.c >> +++ b/src/box/sql/select.c >> @@ -5431,9 +5431,7 @@ vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_ma >> * The results are returned according to the SelectDest structure. >> * See comments in sqlInt.h for further information. >> * >> - * This routine returns the number of errors. If any errors are >> - * encountered, then an appropriate error message is left in >> - * pParse->zErrMsg. >> + * This routine returns the number of errors. > > That’s not true: > > rc = pParse->is_aborted; > > /* Control jumps to here if an error is encountered above, or upon > * successful coding of the SELECT. > */ > select_end: > pParse->iSelectId = iRestoreSelectId; > > /* Identify column names if results of the SELECT are to be output. > */ > if (rc == SQL_OK && pDest->eDest == SRT_Output) { > generateColumnNames(pParse, pTabList, pEList); > } > > sqlDbFree(db, sAggInfo.aCol); > sqlDbFree(db, sAggInfo.aFunc); > #ifdef SQL_DEBUG > SELECTTRACE(1, pParse, p, ("end processing\n")); > pParse->nSelectIndent--; > #endif > return rc; > > So it doesn’t return count of occurred errors. > >> diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c >> index 4ca3c2e..1a7248e 100644 >> --- a/src/box/sql/tokenize.c >> +++ b/src/box/sql/tokenize.c >> @@ -439,13 +439,10 @@ parser_space_delete(struct sql *db, struct space *space) >> >> /* >> * 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. >> + * passed in. An SQL_ status code is returned. > > On the other hand: > > return nErr; > > So what is the truth? > Fixed, rewritten part of the comment. New version: commit d0d78b7eb71413f1e5b0cc7bf0c3c83e4aafe55b Author: Mergen Imeev Date: Wed Feb 27 09:40:17 2019 +0300 sql: remove field zErrMsg from struct Parse This field become unused and should be removed. Part of #3965 diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c index 85385ee..512a3dd 100644 --- a/src/box/sql/prepare.c +++ b/src/box/sql/prepare.c @@ -311,7 +311,6 @@ sql_parser_destroy(Parse *parser) db->lookaside.bDisable -= parser->disableLookaside; } parser->disableLookaside = 0; - sqlDbFree(db, parser->zErrMsg); switch (parser->parsed_ast_type) { case AST_TYPE_SELECT: sql_select_delete(db, parser->parsed_ast.select); diff --git a/src/box/sql/select.c b/src/box/sql/select.c index e83b898..9a8ea9d 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -5431,12 +5431,11 @@ vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_ma * The results are returned according to the SelectDest structure. * See comments in sqlInt.h for further information. * - * This routine returns the number of errors. If any errors are - * encountered, then an appropriate error message is left in - * pParse->zErrMsg. - * * This routine does NOT free the Select structure passed in. The * calling function needs to do that. + * + * @retval 0 on success. + * @retval != 0 on error. */ int sqlSelect(Parse * pParse, /* The parser context */ diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index b05e67b..5ddaca2 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -2653,7 +2653,6 @@ struct fk_constraint_parse { */ struct Parse { sql *db; /* The main database structure */ - char *zErrMsg; /* An error message */ Vdbe *pVdbe; /* An engine for executing database bytecode */ u8 colNamesSet; /* TRUE after OP_ColumnName has been issued to pVdbe */ u8 nTempReg; /* Number of temporary registers in aTempReg[] */ diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c index be04c17..db06c39 100644 --- a/src/box/sql/tokenize.c +++ b/src/box/sql/tokenize.c @@ -531,8 +531,6 @@ sqlRunParser(Parse * pParse, const char *zSql) sqlParserFree(pEngine, sql_free); 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->is_aborted) { sqlVdbeDelete(pParse->pVdbe); pParse->pVdbe = 0; diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 7eacd33..2f1268a 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -721,25 +721,6 @@ onErrorText(int onError) } #endif -/* - * Parse context structure pFrom has just been used to create a sub-vdbe - * (trigger program). If an error has occurred, transfer error information - * from pFrom to pTo. - */ -static void -transferParseError(Parse * pTo, Parse * pFrom) -{ - assert(pFrom->zErrMsg == 0 || pFrom->is_aborted); - assert(pTo->zErrMsg == 0 || pTo->is_aborted); - if (!pTo->is_aborted) { - pTo->zErrMsg = pFrom->zErrMsg; - pTo->is_aborted = pFrom->is_aborted; - } else { - sqlDbFree(pFrom->db, pFrom->zErrMsg); - } - pFrom->zErrMsg = NULL; -} - /** * Create and populate a new TriggerPrg object with a sub-program * implementing trigger pTrigger with ON CONFLICT policy orconf. @@ -848,7 +829,8 @@ sql_row_trigger_program(struct Parse *parser, struct sql_trigger *trigger, VdbeComment((v, "End: %s.%s", trigger->zName, onErrorText(orconf))); - transferParseError(parser, pSubParse); + if (!parser->is_aborted) + parser->is_aborted = pSubParse->is_aborted; if (db->mallocFailed == 0) { pProgram->aOp = sqlVdbeTakeOpArray(v, &pProgram->nOp, -- 2.7.4