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 ED2D3276C8 for ; Mon, 25 Feb 2019 12:14:29 -0500 (EST) 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 BjcESJyty13O for ; Mon, 25 Feb 2019 12:14:29 -0500 (EST) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 8208B27614 for ; Mon, 25 Feb 2019 12:14:29 -0500 (EST) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v2 4/5] sql: remove file zErrMsg of struct Parse Date: Mon, 25 Feb 2019 20:14:27 +0300 Message-Id: <33421784356c1d83b8f38d1ff4c90982f3ad884b.1551114402.git.imeevma@gmail.com> In-Reply-To: References: 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 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( ); ]], { -- - 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)" -- }) @@ -528,7 +528,7 @@ test:do_catchsql_test( ); ]], { -- - 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)" -- }) 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 ]], { -- - 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" -- }) 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; ]], { -- - 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" -- }) @@ -434,7 +434,7 @@ test:do_catchsql_test( SELECT SUM(s1) FROM te40 HAVING s1 = 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" -- }) @@ -444,7 +444,7 @@ test:do_catchsql_test( SELECT s1 FROM te40 HAVING SUM(s1) = 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" -- }) @@ -484,7 +484,7 @@ test:do_catchsql_test( SELECT SUM(s1),s2 FROM te40 HAVING SUM(s1) > 0; ]], { -- - 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" -- }) @@ -494,7 +494,7 @@ test:do_catchsql_test( SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0 and s2 > 0; ]], { -- - 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" -- }) 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