[tarantool-patches] [PATCH v2 4/5] sql: remove file zErrMsg of struct Parse

[imeevma@tarantool.org]

This field become unused and should be removed.

Part of #3965
---
 src/box/sql.c                 | 11 +++--------
 src/box/sql/build.c           | 14 ++++----------
 src/box/sql/delete.c          | 23 +++++++----------------
 src/box/sql/prepare.c         | 12 +++---------
 src/box/sql/resolve.c         |  9 +++------
 src/box/sql/sqlInt.h          |  3 +--
 src/box/sql/tokenize.c        | 37 +++++++------------------------------
 src/box/sql/trigger.c         |  6 ------
 test/sql-tap/check.test.lua   |  4 ++--
 test/sql-tap/select3.test.lua |  2 +-
 test/sql-tap/select5.test.lua | 10 +++++-----
 test/sql/checks.result        | 12 ++++++------
 test/sql/delete.result        |  5 ++---
 test/sql/on-conflict.result   |  3 +--
 14 files changed, 45 insertions(+), 106 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index 580f3fa..116e3e8 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1362,13 +1362,8 @@ 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.rc != SQL_OK) {
-		/* Tarantool error may be already set with diag. */
-		if (parser.rc != SQL_TARANTOOL_ERROR)
-			diag_set(ClientError, ER_SQL, parser.zErrMsg);
-		rc = -1;
-	}
+	if (parser.rc != SQL_OK)
+		return -1;
 	sql_parser_destroy(&parser);
-	return rc;
+	return 0;
 }
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index deb5b89..6afca4a 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -493,16 +493,10 @@ sql_column_add_nullable_action(struct Parse *parser,
 	struct field_def *field = &def->fields[def->field_count - 1];
 	if (field->nullable_action != ON_CONFLICT_ACTION_DEFAULT &&
 	    nullable_action != field->nullable_action) {
-		/* Prevent defining nullable_action many times. */
-		const char *err_msg =
-			tt_sprintf("NULL declaration for column '%s' of table "
-				   "'%s' has been already set to '%s'",
-				   field->name, def->name,
-				   on_conflict_action_strs[field->
-							   nullable_action]);
-		diag_set(ClientError, ER_SQL, err_msg);
-		parser->rc = SQL_TARANTOOL_ERROR;
-		parser->nErr++;
+		sqlErrorMsg(parser, "NULL declaration for column '%s' of "\
+			    "table '%s' has been already set to '%s'",
+			    field->name, def->name,
+			    on_conflict_action_strs[field-> nullable_action]);
 		return;
 	}
 	field->nullable_action = nullable_action;
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 5170c7f..a7bf3b3 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -94,31 +94,22 @@ sql_table_truncate(struct Parse *parse, struct SrcList *tab_list)
 	struct space *space = space_by_name(tab_name);
 	if (space == NULL) {
 		diag_set(ClientError, ER_NO_SUCH_SPACE, tab_name);
-		goto tarantool_error;
+		sql_parser_error(parse);
 	}
 	if (! rlist_empty(&space->parent_fk_constraint)) {
-		const char *err_msg =
-			tt_sprintf("can not truncate space '%s' because other "
-				   "objects depend on it", space->def->name);
-		diag_set(ClientError, ER_SQL, err_msg);
-		goto tarantool_error;
+		sqlErrorMsg(parse, "can not truncate space '%s' because other "
+			    "objects depend on it", space->def->name);
+		goto cleanup;
 	}
 	if (space->def->opts.is_view) {
-		const char *err_msg =
-			tt_sprintf("can not truncate space '%s' because it is "
-				   "a view", space->def->name);
-		diag_set(ClientError, ER_SQL, err_msg);
-		goto tarantool_error;
+		sqlErrorMsg(parse, "can not truncate space '%s' because it is "
+			    "a view", space->def->name);
+		goto cleanup;
 	}
 	sqlVdbeAddOp2(v, OP_Clear, space->def->id, true);
 cleanup:
 	sqlSrcListDelete(parse->db, tab_list);
 	return;
-
-tarantool_error:
-	parse->rc = SQL_TARANTOOL_ERROR;
-	parse->nErr++;
-	goto cleanup;
 }
 
 void
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index d4ba55b..feeefb1 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 */
 	int i;			/* Loop counter */
 	Parse sParse;		/* Parsing context */
@@ -90,14 +89,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);
 
@@ -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);
 
 	/* Delete any TriggerPrg structures allocated while parsing this statement. */
 	while (sParse.pTriggerPrg) {
@@ -295,7 +290,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/resolve.c b/src/box/sql/resolve.c
index aed9e26..1339157 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -1332,12 +1332,9 @@ resolveSelectStep(Walker * pWalker, Select * p)
 				return WRC_Abort;
 			if ((sNC.ncFlags & NC_HasAgg) == 0 ||
 			    (sNC.ncFlags & NC_HasUnaggregatedId) != 0) {
-				diag_set(ClientError, ER_SQL, "HAVING "
-					 "argument must appear in the GROUP BY "
-					 "clause or be used in an aggregate "
-					 "function");
-				pParse->nErr++;
-				pParse->rc = SQL_TARANTOOL_ERROR;
+				sqlErrorMsg(pParse, "HAVING argument must "\
+					    "appear in the GROUP BY clause or "\
+					    "be used in an aggregate function");
 				return WRC_Abort;
 			}
 			/*
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index dbfdbc6..1a28f1a 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 */
 	int rc;			/* Return code from execution */
 	u8 colNamesSet;		/* TRUE after OP_ColumnName has been issued to pVdbe */
@@ -3221,7 +3220,7 @@ void sqlDequote(char *);
 void sqlNormalizeName(char *z);
 void sqlTokenInit(Token *, char *);
 int sqlKeywordCode(const unsigned char *, int);
-int sqlRunParser(Parse *, const char *, char **);
+int sqlRunParser(Parse *, const char *);
 
 /**
  * Increment error counter.
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 44a7bb9..5ea0ea5 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -445,7 +445,7 @@ parser_space_delete(struct sql *db, struct space *space)
  * error message.
  */
 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 */
@@ -463,7 +463,6 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
 	pParse->rc = SQL_OK;
 	pParse->zTail = zSql;
 	i = 0;
-	assert(pzErrMsg != 0);
 	/* sqlParserTrace(stdout, "parser: "); */
 	pEngine = sqlParserAlloc(sqlMalloc);
 	if (pEngine == 0) {
@@ -531,22 +530,8 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
 	if (db->mallocFailed) {
 		pParse->rc = SQL_NOMEM_BKPT;
 	}
-	if (pParse->rc != SQL_OK && pParse->rc != SQL_DONE
-	    && pParse->zErrMsg == 0) {
-		const char *error;
-		if (is_tarantool_error(pParse->rc) &&
-		    tarantoolErrorMessage() != NULL)
-			error = tarantoolErrorMessage();
-		else
-			error = sqlErrStr(pParse->rc);
-		pParse->zErrMsg = sqlMPrintf(db, "%s", error);
-	}
-	assert(pzErrMsg != 0);
-	if (pParse->zErrMsg) {
-		*pzErrMsg = pParse->zErrMsg;
-		sql_log(pParse->rc, "%s", *pzErrMsg);
+	if (pParse->rc != SQL_OK && pParse->rc != SQL_DONE)
 		nErr++;
-	}
 	if (pParse->pVdbe != NULL && pParse->nErr > 0) {
 		sqlVdbeDelete(pParse->pVdbe);
 		pParse->pVdbe = 0;
@@ -577,11 +562,8 @@ sql_expr_compile(sql *db, const char *expr, int expr_len)
 	}
 	sprintf(stmt, "%s%.*s", outer, expr_len, expr);
 
-	char *sql_error = NULL;
-	if (sqlRunParser(&parser, stmt, &sql_error) != SQL_OK ||
-	    parser.parsed_ast_type != AST_TYPE_EXPR) {
-		diag_set(ClientError, ER_SQL, sql_error);
-	} else {
+	if (sqlRunParser(&parser, stmt) == SQL_OK &&
+	    parser.parsed_ast_type == AST_TYPE_EXPR) {
 		expression = parser.parsed_ast.expr;
 		parser.parsed_ast.expr = NULL;
 	}
@@ -599,8 +581,7 @@ sql_view_compile(struct sql *db, const char *view_stmt)
 
 	struct Select *select = NULL;
 
-	char *unused;
-	if (sqlRunParser(&parser, view_stmt, &unused) != SQL_OK ||
+	if (sqlRunParser(&parser, view_stmt) != SQL_OK ||
 	    parser.parsed_ast_type != AST_TYPE_SELECT) {
 		diag_set(ClientError, ER_SQL_EXECUTE, view_stmt);
 	} else {
@@ -618,13 +599,9 @@ sql_trigger_compile(struct sql *db, const char *sql)
 	struct Parse parser;
 	sql_parser_create(&parser, db);
 	parser.parse_only = true;
-	char *sql_error = NULL;
 	struct sql_trigger *trigger = NULL;
-	if (sqlRunParser(&parser, sql, &sql_error) != SQL_OK ||
-	    parser.parsed_ast_type != AST_TYPE_TRIGGER) {
-	    if (parser.rc != SQL_TARANTOOL_ERROR)
-		diag_set(ClientError, ER_SQL, sql_error);
-	} else {
+	if (sqlRunParser(&parser, sql) == SQL_OK &&
+	    parser.parsed_ast_type == AST_TYPE_TRIGGER) {
 		trigger = parser.parsed_ast.trigger;
 		parser.parsed_ast.trigger = NULL;
 	}
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index f7e6189..4c7ab8a 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -730,16 +730,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) {
-		pTo->zErrMsg = pFrom->zErrMsg;
 		pTo->nErr = pFrom->nErr;
 		pTo->rc = pFrom->rc;
-	} else {
-		sqlDbFree(pFrom->db, pFrom->zErrMsg);
 	}
-	pFrom->zErrMsg = NULL;
 }
 
 /**
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index 8ce3184..c2e9a91 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>
     })
 
@@ -528,7 +528,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <check-5.2>
-        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.2>
     })
 
diff --git a/test/sql-tap/select3.test.lua b/test/sql-tap/select3.test.lua
index 6ae3135..c68ca4d 100755
--- a/test/sql-tap/select3.test.lua
+++ b/test/sql-tap/select3.test.lua
@@ -200,7 +200,7 @@ test:do_catchsql_test("select3-3.1", [[
   SELECT log, count(*) FROM t1 HAVING log>=4
 ]], {
   -- <select3-3.1>
-  1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
+  1, "HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
   -- </select3-3.1>
 })
 
diff --git a/test/sql-tap/select5.test.lua b/test/sql-tap/select5.test.lua
index d47e340..180576b 100755
--- a/test/sql-tap/select5.test.lua
+++ b/test/sql-tap/select5.test.lua
@@ -424,7 +424,7 @@ test:do_catchsql_test(
         SELECT s1 FROM te40 HAVING s1 = 1;
     ]], {
     -- <select5-9.1>
-    1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
+    1, "HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
     -- </select5-9.1>
 })
 
@@ -434,7 +434,7 @@ test:do_catchsql_test(
         SELECT SUM(s1) FROM te40 HAVING s1 = 2;
     ]], {
     -- <select5-9.2>
-    1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
+    1, "HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
     -- </select5-9.2>
 })
 
@@ -444,7 +444,7 @@ test:do_catchsql_test(
         SELECT s1 FROM te40 HAVING SUM(s1) = 2;
     ]], {
     -- <select5-9.3>
-    1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
+    1, "HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
     -- </select5-9.3>
 })
 
@@ -484,7 +484,7 @@ test:do_catchsql_test(
         SELECT SUM(s1),s2 FROM te40 HAVING SUM(s1) > 0;
     ]], {
     -- <select5-9.7>
-    1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
+    1, "HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
     -- </select5-9.7>
 })
 
@@ -494,7 +494,7 @@ test:do_catchsql_test(
         SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0 and s2 > 0;
     ]], {
     -- <select5-9.8>
-    1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
+    1, "HAVING argument must appear in the GROUP BY clause or be used in an aggregate function"
     -- </select5-9.8>
 })
 
diff --git a/test/sql/checks.result b/test/sql/checks.result
index cfce2e4..e6a6b72 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -29,8 +29,8 @@ t = {513, 1, 'test', 'memtx', 0, opts, format}
 ...
 s = box.space._space:insert(t)
 ---
-- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
-    Unrecognized syntax near ''<'')'
+- error: 'Wrong space options (field 5): invalid expression specified (Unrecognized
+    syntax near ''<'')'
 ...
 opts = {checks = {{expr = 'X>5'}}}
 ---
@@ -122,8 +122,8 @@ box.sql.execute("DROP TABLE w2;")
 --
 box.sql.execute("CREATE TABLE t5(x INT PRIMARY KEY, y INT, CHECK( x*y < ? ));")
 ---
-- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
-    bindings are not allowed in DDL)'
+- error: 'Wrong space options (field 5): invalid expression specified (bindings are
+    not allowed in DDL)'
 ...
 opts = {checks = {{expr = '?>5', name = 'ONE'}}}
 ---
@@ -136,8 +136,8 @@ t = {513, 1, 'test', 'memtx', 0, opts, format}
 ...
 s = box.space._space:insert(t)
 ---
-- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
-    bindings are not allowed in DDL)'
+- error: 'Wrong space options (field 5): invalid expression specified (bindings are
+    not allowed in DDL)'
 ...
 test_run:cmd("clear filter")
 ---
diff --git a/test/sql/delete.result b/test/sql/delete.result
index e024dd6..a0c8352 100644
--- a/test/sql/delete.result
+++ b/test/sql/delete.result
@@ -93,7 +93,7 @@ box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;")
 ...
 box.sql.execute("TRUNCATE TABLE v1;")
 ---
-- error: 'SQL error: can not truncate space ''V1'' because it is a view'
+- error: can not truncate space 'V1' because it is a view
 ...
 -- Can't truncate table with FK.
 box.sql.execute("CREATE TABLE t2(x INT PRIMARY KEY REFERENCES t1(id));")
@@ -101,8 +101,7 @@ box.sql.execute("CREATE TABLE t2(x INT PRIMARY KEY REFERENCES t1(id));")
 ...
 box.sql.execute("TRUNCATE TABLE t1;")
 ---
-- error: 'SQL error: can not truncate space ''T1'' because other objects depend on
-    it'
+- error: can not truncate space 'T1' because other objects depend on it
 ...
 -- Table triggers should be ignored.
 box.sql.execute("DROP TABLE t2;")
diff --git a/test/sql/on-conflict.result b/test/sql/on-conflict.result
index 6d37e69..6ddce86 100644
--- a/test/sql/on-conflict.result
+++ b/test/sql/on-conflict.result
@@ -58,8 +58,7 @@ box.sql.execute("CREATE TABLE te17 (s1 INT NULL PRIMARY KEY);")
 ...
 box.sql.execute("CREATE TABLE test (a int PRIMARY KEY, b int NULL ON CONFLICT IGNORE);")
 ---
-- error: 'SQL error: NULL declaration for column ''B'' of table ''TEST'' has been
-    already set to ''none'''
+- error: NULL declaration for column 'B' of table 'TEST' has been already set to 'none'
 ...
 box.sql.execute("CREATE TABLE test (a int, b int NULL, c int, PRIMARY KEY(a, b, c))")
 ---
-- 
2.7.4