[tarantool-patches] Re: [PATCH v3 5/9] sql: remove field zErrMsg of struct Parse
n.pettik
korablev at tarantool.org
Tue Mar 5 12:06:48 MSK 2019
>>> @@ -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.
> New version:
>
> commit 29c77ea3f8463994b98bcb23653f901cf46e472a
> Author: Mergen Imeev <imeevma at gmail.com>
> 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.
>
> /* 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?
> */
> 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 */
More information about the Tarantool-patches
mailing list