[tarantool-patches] Re: [PATCH v4 3/8] sql: replace rc with is_aborted status in struct Parse

[n.pettik]

>>> 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.

> 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;
 }

> }
> 
> 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?