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 A9C352971E for ; Wed, 13 Mar 2019 13:03:27 -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 TFEZDNUlT_9J for ; Wed, 13 Mar 2019 13:03:27 -0400 (EDT) Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (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 655D2296F2 for ; Wed, 13 Mar 2019 13:03:26 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v4 8/8] sql: remove sqlErrorMsg() Date: Wed, 13 Mar 2019 20:03:24 +0300 Message-Id: <99f4adc61352899a83a7097b965845bafaa968ef.1552494059.git.imeevma@gmail.com> 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 a ot of changes in previous patches. Some of functionality of previous version of this patch were moved in new patch "sql: rework semantic errors" On 3/5/19 3:16 PM, n.pettik wrote: > >> @@ -1284,10 +1302,13 @@ sql_create_view(struct Parse *parse_context, struct Token *begin, >> goto create_view_fail; >> if (aliases != NULL) { >> if ((int)select_res_space->def->field_count != aliases->nExpr) { >> - sqlErrorMsg(parse_context, "expected %d columns "\ >> - "for '%s' but got %d", aliases->nExpr, >> - space->def->name, >> - select_res_space->def->field_count); >> + const char *err_msg = >> + tt_sprintf("expected %d columns for '%s' but "\ >> + "got %d", aliases->nExpr, >> + space->def->name, >> + select_res_space->def->field_count); >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); >> + parse_context->is_aborted = true; >> goto create_view_fail; >> } >> sqlColumnsFromExprList(parse_context, aliases, space->def); >> @@ -1609,13 +1630,17 @@ sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list, >> * and DROP VIEW is not used on a table. >> */ >> if (is_view && !space->def->opts.is_view) { >> - sqlErrorMsg(parse_context, "use DROP TABLE to delete table %s", >> - space_name); >> + const char *err_msg = tt_sprintf("use DROP TABLE to delete "\ >> + "table %s", space_name); >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); > > Why not ER_DROP_SPACE? Fixed in "sql: rework semantic errors". >> + parse_context->is_aborted = true; >> goto exit_drop_table; >> } >> if (!is_view && space->def->opts.is_view) { >> - sqlErrorMsg(parse_context, "use DROP VIEW to delete view %s", >> - space_name); >> + const char *err_msg = tt_sprintf("use DROP VIEW to delete "\ >> + "view %s", space_name); >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); > > The same question. Fixed in "sql: rework semantic errors". >> + parse_context->is_aborted = true; >> goto exit_drop_table; >> } >> /* >> @@ -1760,8 +1785,10 @@ sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child, >> } >> } else { >> if (parent_space->def->opts.is_view) { >> - sqlErrorMsg(parse_context, >> - "referenced table can't be view"); >> + const char *err_msg = tt_sprintf("referenced table "\ >> + "can't be view"); >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); > > ER_CREATE_FK_CONSTRAINT Fixed in "sql: rework semantic errors". >> + parse_context->is_aborted = true; >> goto exit_create_fk; >> } >> } >> @@ -2149,7 +2176,9 @@ sql_create_index(struct Parse *parse, struct Token *token, >> struct space_def *def = space->def; >> >> if (def->opts.is_view) { >> - sqlErrorMsg(parse, "views can not be indexed"); >> + const char *err_msg = tt_sprintf("views can not be indexed"); >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); > > ER_CREATE_INDEX Fixed in "sql: rework semantic errors". >> + parse->is_aborted = true; >> goto exit_create_index; >> } >> /* >> @@ -2947,11 +2976,8 @@ sqlSavepoint(Parse * pParse, int op, Token * pName) >> return; >> } >> if (op == SAVEPOINT_BEGIN && >> - sqlCheckIdentifierName(pParse, zName) >> - != SQL_OK) { >> - sqlErrorMsg(pParse, "bad savepoint name"); >> + sqlCheckIdentifierName(pParse, zName) != SQL_OK) > > != 0 Function sqlCheckIdentifierName() still works in terms of {SQL_OK, SQL_ERROR}, so I left this part as it was. >> return; >> - } >> sqlVdbeAddOp4(v, OP_Savepoint, op, 0, 0, zName, P4_DYNAMIC); >> } >> } >> @@ -1757,8 +1759,10 @@ sqlExprListAppendVector(Parse * pParse, /* Parsing context */ >> */ >> if (pExpr->op != TK_SELECT >> && pColumns->nId != (n = sqlExprVectorSize(pExpr))) { >> - sqlErrorMsg(pParse, "%d columns assigned %d values", >> - pColumns->nId, n); >> + const char *err_msg = tt_sprintf("%d columns assigned %d "\ >> + "values", pColumns->nId, n); >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); >> + pParse->is_aborted = true; >> goto vector_append_error; >> } >> >> @@ -1878,7 +1882,10 @@ sqlExprListCheckLength(Parse * pParse, >> testcase(pEList && pEList->nExpr == mx); >> testcase(pEList && pEList->nExpr == mx + 1); >> if (pEList && pEList->nExpr > mx) { >> - sqlErrorMsg(pParse, "too many columns in %s", zObject); >> + const char *err_msg = tt_sprintf("too many columns in %s", >> + zObject); > > You introduced LIMIT and “too many columns” errors in previous patch. > Why you can’t use them here? Fixed in "sql: rework semantic errors". >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); >> + pParse->is_aborted = true; >> } >> } >> >> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y >> index b69f059..3653824 100644 >> --- a/src/box/sql/parse.y >> +++ b/src/box/sql/parse.y >> @@ -897,7 +897,9 @@ expr(A) ::= VARIABLE(X). { >> Token t = X; >> if (pParse->parse_only) { >> spanSet(&A, &t, &t); >> - sqlErrorMsg(pParse, "bindings are not allowed in DDL"); >> + const char *err_msg = tt_sprintf("bindings are not allowed in DDL”); > > Em, why can’t you inline this var? > > diag_set(ClientError, ER_SQL_PARSER_GENERIC, "bindings are not allowed in DDL"); Fixed. >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); >> + pParse->is_aborted = true; >> A.pExpr = NULL; >> } else if (!(X.z[0]=='#' && sqlIsdigit(X.z[1]))) { >> u32 n = X.n; >> >> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c >> index 02eca37..63a1b96 100644 >> --- a/src/box/sql/resolve.c >> +++ b/src/box/sql/resolve.c >> @@ -392,14 +392,25 @@ lookupName(Parse * pParse, /* The parsing context */ >> pOrig = pEList->a[j].pExpr; >> if ((pNC->ncFlags & NC_AllowAgg) == 0 >> && ExprHasProperty(pOrig, EP_Agg)) { >> - sqlErrorMsg(pParse, >> - "misuse of aliased aggregate %s", >> - zAs); >> + const char *err_msg = >> + tt_sprintf("misuse of "\ >> + "aliased "\ >> + "aggregate "\ >> + "%s", zAs); > > Such formatting looks terrible. Lets break 80 border or move > declaration of this error msg at few blocks above. Fixed, I think. >> @@ -633,9 +647,13 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) >> exprProbability(pList->a[1]. >> pExpr); >> if (pExpr->iTable < 0) { >> - sqlErrorMsg(pParse, >> - "second argument to likelihood() must be a " >> - "constant between 0.0 and 1.0"); >> + const char *err_msg = >> + tt_sprintf("second argument to likelihood() must be a "\ >> + "constant between 0.0 and 1.0”); > > You don’t need sprintf here. Fixed. >> + diag_set(ClientError, >> + ER_SQL_PARSER_GENERIC, >> + err_msg); >> + pParse->is_aborted = true; >> pNC->nErr++; >> } >> } else { >> @@ -802,7 +827,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) >> testcase(pExpr->op == TK_GT); >> testcase(pExpr->op == TK_GE); >> testcase(pExpr->op == TK_BETWEEN); >> - sqlErrorMsg(pParse, "row value misused"); >> + const char *err_msg = tt_sprintf("row value "\ >> + "misused”); > > The same. Fixed. >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, >> + err_msg); >> + pParse->is_aborted = true; >> } >> break; >> } >> @@ -950,7 +964,10 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages >> db = pParse->db; >> #if SQL_MAX_COLUMN >> if (pOrderBy->nExpr > db->aLimit[SQL_LIMIT_COLUMN]) { >> - sqlErrorMsg(pParse, "too many terms in ORDER BY clause"); >> + const char *err_msg = tt_sprintf("too many terms in ORDER BY "\ >> + "clause”); > > The same. Fixed. >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); >> + pParse->is_aborted = true; >> return 1; >> } >> #endif >> @@ -1024,9 +1045,11 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages >> } >> for (i = 0; i < pOrderBy->nExpr; i++) { >> if (pOrderBy->a[i].done == 0) { >> - sqlErrorMsg(pParse, >> - "%r ORDER BY term does not match any " >> - "column in the result set", i + 1); >> + const char *err_msg = >> + tt_sprintf("ORDER BY term does not match any "\ >> + "column in the result set”); > > The same. Fixed. >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); >> + pParse->is_aborted = true; >> return 1; >> } >> } >> @@ -1417,9 +1450,15 @@ resolveSelectStep(Walker * pWalker, Select * p) >> for (i = 0, pItem = pGroupBy->a; i < pGroupBy->nExpr; >> i++, pItem++) { >> if (ExprHasProperty(pItem->pExpr, EP_Agg)) { >> - sqlErrorMsg(pParse, >> - "aggregate functions are not allowed in " >> - "the GROUP BY clause"); >> + const char *err_msg = >> + tt_sprintf("aggregate "\ >> + "functions are not "\ >> + "allowed in the "\ >> + "GROUP BY clause”); > > Ok, verify all usages of sprintf and make sure they are really required. > I see several more in code. Fixed. >> @@ -613,8 +622,11 @@ sqlProcessJoin(Parse * pParse, Select * p) >> /* Disallow both ON and USING clauses in the same join >> */ >> if (pRight->pOn && pRight->pUsing) { >> - sqlErrorMsg(pParse, "cannot have both ON and USING " >> - "clauses in the same join"); >> + const char *err_msg = >> + tt_sprintf("cannot have both ON and USING " >> + "clauses in the same join"); >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); >> + pParse->is_aborted = true; >> return 1; >> } >> >> @@ -650,10 +662,16 @@ sqlProcessJoin(Parse * pParse, Select * p) >> || !tableAndColumnIndex(pSrc, i + 1, zName, >> &iLeft, &iLeftCol) >> ) { >> - sqlErrorMsg(pParse, >> - "cannot join using column %s - column " >> - "not present in both tables", >> - zName); >> + const char *err_msg = >> + tt_sprintf("cannot join using "\ >> + "column %s - "\ >> + "column not "\ >> + "present in both "\ >> + "tables", zName); > > Horrible formatting. Fixed, I think. >> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h >> index 85718e1..5a74c65 100644 >> --- a/src/box/sql/sqlInt.h >> +++ b/src/box/sql/sqlInt.h >> @@ -3182,7 +3182,6 @@ void sqlTreeViewWith(TreeView *, const With *); >> #endif >> >> void sqlSetString(char **, sql *, const char *); >> -void sqlErrorMsg(Parse *, const char *, ...); >> void sqlDequote(char *); >> void sqlNormalizeName(char *z); >> void sqlTokenInit(Token *, char *); >> diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c >> index 2f1268a..297ba01 100644 >> --- a/src/box/sql/trigger.c >> +++ b/src/box/sql/trigger.c >> @@ -621,8 +621,11 @@ codeTriggerProgram(Parse * pParse, /* The parser context */ >> sqlSubProgramsRemaining--; >> >> if (sqlSubProgramsRemaining == 0) { >> - sqlErrorMsg(pParse, >> - "Maximum number of chained trigger activations exceeded."); >> + const char *err_msg = tt_sprintf("Maximum number of chained "\ >> + "trigger activations "\ >> + "exceeded.”); > > Please, make sure that it is valid error. And the rest of errors > I pointed in google doc file. If this is unreachable code, then > replace error with assert. This error is valid. As for the other ones - I couldn't reach some of them, but didn't remove them for now. >> -/* >> * Convert an SQL-style quoted string into a normal string by removing >> * the quote characters. The conversion is done in-place. If the >> * input does not begin with a quote character, then this routine >> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c >> index 9957f3a..d4fbf17 100644 >> --- a/src/box/sql/vdbemem.c >> +++ b/src/box/sql/vdbemem.c >> @@ -1222,7 +1222,9 @@ valueFromFunction(sql * db, /* The database connection */ >> pFunc->xSFunc(&ctx, nVal, apVal); >> if (ctx.isError) { >> rc = ctx.isError; >> - sqlErrorMsg(pCtx->pParse, "%s", sql_value_text(pVal)); >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, >> + sql_value_text(pVal)); > > What kind of error is it? Please, add reasonable error message, > not only text representation of value. I did, but not sure if this error is reachable. I wasn't able to reproduce it. >> + pCtx->pParse->is_aborted = true; >> } else { >> sql_value_apply_type(pVal, type); >> assert(rc == SQL_OK); >> diff --git a/src/box/sql/where.c b/src/box/sql/where.c >> index 33885d0..e2c91e0 100644 >> --- a/src/box/sql/where.c >> +++ b/src/box/sql/where.c >> @@ -3914,7 +3914,9 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst) >> } >> >> if (nFrom == 0) { >> - sqlErrorMsg(pParse, "no query solution"); >> + const char *err_msg = tt_sprintf("no query solution”); > > Same: this path seems to be unreachable. I think so too, but left as it is for now. >> diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua >> index e190ad7..16a31a8 100755 >> --- a/test/sql-tap/e_select1.test.lua >> +++ b/test/sql-tap/e_select1.test.lua >> @@ -1855,13 +1855,13 @@ test:do_catchsql_test( >> "e_select-8.7.1.1", >> "SELECT x FROM d1 UNION ALL SELECT a FROM d2 ORDER BY x*z", >> { >> - 1, "1st ORDER BY term does not match any column in the result set"}) >> + 1, "ORDER BY term does not match any column in the result set”} > > Why did you change error message? > I see quite a lot affected tests. I did this due to non-standart format %r that is used here. Changed text of the error. New version: commit 99f4adc61352899a83a7097b965845bafaa968ef Author: Mergen Imeev Date: Sat Mar 9 18:54:19 2019 +0300 sql: remove sqlErrorMsg() This patch completely replaces sqlErrorMsg() with diag_set() and removes sqlErrorMsg(). Closes #3965 Closes #3036 diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 26434b1..565bc17 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -490,13 +490,12 @@ sql_column_add_nullable_action(struct Parse *parser, 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); + const char *err = "NULL declaration for column '%s' of table " + "'%s' has been already set to '%s'"; + const char *action = + on_conflict_action_strs[field->nullable_action]; + err = tt_sprintf(err, field->name, def->name, action); + diag_set(ClientError, ER_SQL, err); parser->is_aborted = true; return; } @@ -3044,11 +3043,12 @@ sqlWithAdd(Parse * pParse, /* Parsing context */ zName = sqlNameFromToken(pParse->db, pName); if (zName && pWith) { int i; + const char *err = "Ambiguous table name in WITH query: %s"; for (i = 0; i < pWith->nCte; i++) { if (strcmp(zName, pWith->a[i].zName) == 0) { - sqlErrorMsg(pParse, - "duplicate WITH table name: %s", - zName); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, zName)); + pParse->is_aborted = true; } } } diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index e8a5f0d..fcd320d 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -96,17 +96,17 @@ sql_table_truncate(struct Parse *parse, struct SrcList *tab_list) goto tarantool_error; } 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); + const char *err = "can not truncate space '%s' because other " + "objects depend on it"; + diag_set(ClientError, ER_SQL, + tt_sprintf(err, space->def->name)); goto tarantool_error; } 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); + const char *err = "can not truncate space '%s' because it is "\ + "a view"; + diag_set(ClientError, ER_SQL, + tt_sprintf(err, space->def->name)); goto tarantool_error; } sqlVdbeAddOp2(v, OP_Clear, space->def->id, true); diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index de06ee0..7205cb0 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -668,7 +668,9 @@ codeVectorCompare(Parse * pParse, /* Code generator context */ int addrDone = sqlVdbeMakeLabel(v); if (nLeft != sqlExprVectorSize(pRight)) { - sqlErrorMsg(pParse, "row value misused"); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "row value misused"); + pParse->is_aborted = true; return; } assert(pExpr->op == TK_EQ || pExpr->op == TK_NE @@ -1195,7 +1197,14 @@ sqlExprAssignVarNumber(Parse * pParse, Expr * pExpr, u32 n) testcase(i == 1); testcase(i == SQL_BIND_PARAMETER_MAX - 1); testcase(i == SQL_BIND_PARAMETER_MAX); - if (!is_ok || i < 1 || i > SQL_BIND_PARAMETER_MAX) { + if (i < 1) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "Index of binding slots must start "\ + "from 1"); + pParse->is_aborted = true; + return; + } + if (!is_ok || i > SQL_BIND_PARAMETER_MAX) { diag_set(ClientError, ER_SQL_BIND_PARAMETER_MAX, SQL_BIND_PARAMETER_MAX); pParse->is_aborted = true; @@ -1779,8 +1788,10 @@ sqlExprListAppendVector(Parse * pParse, /* Parsing context */ */ if (pExpr->op != TK_SELECT && pColumns->nId != (n = sqlExprVectorSize(pExpr))) { - sqlErrorMsg(pParse, "%d columns assigned %d values", - pColumns->nId, n); + const char *err = tt_sprintf("%d columns assigned %d values", + pColumns->nId, n); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; goto vector_append_error; } @@ -2644,41 +2655,6 @@ expr_in_type(Parse *pParse, Expr *pExpr) } /* - * Load the Parse object passed as the first argument with an error - * message of the form: - * - * "sub-select returns N columns - expected M" - */ -void -sqlSubselectError(Parse * pParse, int nActual, int nExpect) -{ - const char *zFmt = "sub-select returns %d columns - expected %d"; - sqlErrorMsg(pParse, zFmt, nActual, nExpect); -} - -/* - * Expression pExpr is a vector that has been used in a context where - * it is not permitted. If pExpr is a sub-select vector, this routine - * loads the Parse object with a message of the form: - * - * "sub-select returns N columns - expected 1" - * - * Or, if it is a regular scalar vector: - * - * "row value misused" - */ -void -sqlVectorErrorMsg(Parse * pParse, Expr * pExpr) -{ - if (pExpr->flags & EP_xIsSelect) { - sqlSubselectError(pParse, pExpr->x.pSelect->pEList->nExpr, - 1); - } else { - sqlErrorMsg(pParse, "row value misused"); - } -} - -/* * Generate code for scalar subqueries used as a subquery expression, EXISTS, * or IN operators. Examples: * @@ -2959,15 +2935,26 @@ int sqlExprCheckIN(Parse * pParse, Expr * pIn) { int nVector = sqlExprVectorSize(pIn->pLeft); + const char *err; if ((pIn->flags & EP_xIsSelect)) { if (nVector != pIn->x.pSelect->pEList->nExpr) { - sqlSubselectError(pParse, - pIn->x.pSelect->pEList->nExpr, - nVector); + err = "sub-select returns %d columns - expected %d"; + int expr_count = pIn->x.pSelect->pEList->nExpr; + err = tt_sprintf(err, expr_count, nVector); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; return 1; } } else if (nVector != 1) { - sqlVectorErrorMsg(pParse, pIn->pLeft); + if (pIn->pLeft->flags & EP_xIsSelect) { + err = "sub-select returns %d columns - expected 1"; + int expr_count = pIn->pLeft->x.pSelect->pEList->nExpr; + err = tt_sprintf(err, expr_count); + } else { + err = "row value misused"; + } + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; return 1; } return 0; @@ -3965,9 +3952,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) AggInfo *pInfo = pExpr->pAggInfo; if (pInfo == 0) { assert(!ExprHasProperty(pExpr, EP_IntValue)); - sqlErrorMsg(pParse, - "misuse of aggregate: %s()", - pExpr->u.zToken); + const char *err = "misuse of aggregate: %s()"; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, pExpr->u.zToken)); + pParse->is_aborted = true; } else { pExpr->type = pInfo->aFunc->pFunc->ret_type; return pInfo->aFunc[pExpr->iAgg].iMem; @@ -4134,7 +4122,12 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) testcase(op == TK_SELECT); if (op == TK_SELECT && (nCol = pExpr->x.pSelect->pEList->nExpr) != 1) { - sqlSubselectError(pParse, nCol, 1); + const char *err = "sub-select returns %d "\ + "columns - expected 1"; + err = tt_sprintf(err, nCol); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + err); + pParse->is_aborted = true; } else { return sqlCodeSubselect(pParse, pExpr, 0); } @@ -4153,9 +4146,11 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) sqlExprVectorSize(pExpr-> pLeft)) ) { - sqlErrorMsg(pParse, - "%d columns assigned %d values", - pExpr->iTable, n); + const char *err = + "%d columns assigned %d values"; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, pExpr->iTable, n)); + pParse->is_aborted = true; } return pExpr->pLeft->iTable + pExpr->iColumn; } @@ -4254,7 +4249,9 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) } case TK_VECTOR:{ - sqlErrorMsg(pParse, "row value misused"); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "row value misused"); + pParse->is_aborted = true; break; } @@ -4354,8 +4351,9 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) } case TK_RAISE: if (pParse->triggered_space == NULL) { - sqlErrorMsg(pParse, "RAISE() may only be used " - "within a trigger-program"); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, "RAISE() "\ + "may only be used within a trigger-program"); + pParse->is_aborted = true; return 0; } if (pExpr->on_conflict_action == ON_CONFLICT_ACTION_ABORT) diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 263df4d..7635f7b 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -396,10 +396,11 @@ sqlInsert(Parse * pParse, /* Parser context */ goto insert_cleanup; } if (bit_test(used_columns, j)) { - const char *err; - err = "table id list: duplicate column name %s"; - sqlErrorMsg(pParse, - err, pColumn->a[i].zName); + const char *err = "table id list: duplicate "\ + "column name %s"; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, pColumn->a[i].zName)); + pParse->is_aborted = true; goto insert_cleanup; } bit_set(used_columns, j); @@ -510,14 +511,19 @@ sqlInsert(Parse * pParse, /* Parser context */ if (pColumn == NULL && nColumn != 0 && nColumn != (int)space_def->field_count) { - sqlErrorMsg(pParse, - "table %S has %d columns but %d values were supplied", - pTabList, 0, space_def->field_count, nColumn); + const char *err = + "table %s has %d columns but %d values were supplied"; + err = tt_sprintf(err, pTabList->a[0].zName, + space_def->field_count, nColumn); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; goto insert_cleanup; } if (pColumn != 0 && nColumn != pColumn->nId) { - sqlErrorMsg(pParse, "%d values for %d columns", nColumn, - pColumn->nId); + const char *err = "%d values for %d columns"; + err = tt_sprintf(err, nColumn, pColumn->nId); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; goto insert_cleanup; } diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index acf947b..80a9265 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -902,7 +902,9 @@ expr(A) ::= VARIABLE(X). { Token t = X; if (pParse->parse_only) { spanSet(&A, &t, &t); - sqlErrorMsg(pParse, "bindings are not allowed in DDL"); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "bindings are not allowed in DDL"); + pParse->is_aborted = true; A.pExpr = NULL; } else if (!(X.z[0]=='#' && sqlIsdigit(X.z[1]))) { u32 n = X.n; @@ -1386,9 +1388,9 @@ trigger_cmd_list(A) ::= trigger_cmd(A) SEMI. { trnm(A) ::= nm(A). trnm(A) ::= nm DOT nm(X). { A = X; - sqlErrorMsg(pParse, - "qualified table names are not allowed on INSERT, UPDATE, and DELETE " - "statements within triggers"); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, "qualified table names are not "\ + "allowed on INSERT, UPDATE, and DELETE statements within triggers"); + pParse->is_aborted = true; } // Disallow the INDEX BY and NOT INDEXED clauses on UPDATE and DELETE diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 9171d05..6720914 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -390,16 +390,22 @@ lookupName(Parse * pParse, /* The parsing context */ assert(pExpr->x.pList == 0); assert(pExpr->x.pSelect == 0); pOrig = pEList->a[j].pExpr; + const char *err = "misuse of aliased "\ + "aggregate %s"; if ((pNC->ncFlags & NC_AllowAgg) == 0 && ExprHasProperty(pOrig, EP_Agg)) { - sqlErrorMsg(pParse, - "misuse of aliased aggregate %s", - zAs); + err = tt_sprintf(err, zAs); + diag_set(ClientError, + ER_SQL_PARSER_GENERIC, + err); + pParse->is_aborted = true; return WRC_Abort; } if (sqlExprVectorSize(pOrig) != 1) { - sqlErrorMsg(pParse, - "row value misused"); + diag_set(ClientError, + ER_SQL_PARSER_GENERIC, + "row value misused"); + pParse->is_aborted = true; return WRC_Abort; } resolveAlias(pParse, pEList, j, pExpr, @@ -426,12 +432,15 @@ lookupName(Parse * pParse, /* The parsing context */ * more matches. Either way, we have an error. */ if (cnt > 1) { + const char *err; if (zTab) { - sqlErrorMsg(pParse, "ambiguous column name: %s.%s", - zTab, zCol); + err = tt_sprintf("ambiguous column name: %s.%s", zTab, + zCol); } else { - sqlErrorMsg(pParse, "ambiguous column name: %s", zCol); + err = tt_sprintf("ambiguous column name: %s", zCol); } + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; pTopNC->nErr++; } if (cnt == 0) { @@ -625,7 +634,7 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) } else { is_agg = pDef->xFinalize != 0; pExpr->type = pDef->ret_type; - const char *err_msg = + const char *err = "second argument to likelihood() must "\ "be a constant between 0.0 and 1.0"; if (pDef->funcFlags & SQL_FUNC_UNLIKELY) { @@ -638,7 +647,7 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) if (pExpr->iTable < 0) { diag_set(ClientError, ER_ILLEGAL_PARAMS, - err_msg); + err); pParse->is_aborted = true; pNC->nErr++; @@ -683,9 +692,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) } } if (is_agg && (pNC->ncFlags & NC_AllowAgg) == 0) { - sqlErrorMsg(pParse, - "misuse of aggregate function %.*s()", - nId, zId); + const char *err = + tt_sprintf("misuse of aggregate "\ + "function %.*s()", nId, zId); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; pNC->nErr++; is_agg = 0; } else if (no_such_func && pParse->db->init.busy == 0 @@ -697,9 +708,12 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) pParse->is_aborted = true; pNC->nErr++; } else if (wrong_num_args) { - sqlErrorMsg(pParse, - "wrong number of arguments to function %.*s()", - nId, zId); + const char *err = "wrong number of arguments "\ + "to function %.*s()"; + err = tt_sprintf(err, nId, zId); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + err); + pParse->is_aborted = true; pNC->nErr++; } if (is_agg) @@ -803,7 +817,9 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) testcase(pExpr->op == TK_GT); testcase(pExpr->op == TK_GE); testcase(pExpr->op == TK_BETWEEN); - sqlErrorMsg(pParse, "row value misused"); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "row value misused"); + pParse->is_aborted = true; } break; } @@ -905,21 +921,6 @@ resolveOrderByTermToExprList(Parse * pParse, /* Parsing context for error messag } /* - * Generate an ORDER BY or GROUP BY term out-of-range error. - */ -static void -resolveOutOfRangeError(Parse * pParse, /* The error context into which to write the error */ - const char *zType, /* "ORDER" or "GROUP" */ - int i, /* The index (1-based) of the term out of range */ - int mx /* Largest permissible value of i */ - ) -{ - sqlErrorMsg(pParse, - "%r %s BY term out of range - should be " - "between 1 and %d", i, zType, mx); -} - -/* * Analyze the ORDER BY clause in a compound SELECT statement. Modify * each term of the ORDER BY clause is a constant integer between 1 * and N where N is the number of columns in the compound SELECT. @@ -980,9 +981,16 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages pE = sqlExprSkipCollate(pItem->pExpr); if (sqlExprIsInteger(pE, &iCol)) { if (iCol <= 0 || iCol > pEList->nExpr) { - resolveOutOfRangeError(pParse, "ORDER", - i + 1, - pEList->nExpr); + const char *err = + "Error at ORDER BY in place "\ + "%d: term out of range - "\ + "should be between 1 and %d"; + err = tt_sprintf(err, i + 1, + pEList->nExpr); + diag_set(ClientError, + ER_SQL_PARSER_GENERIC, + err); + pParse->is_aborted = true; return 1; } } else { @@ -1028,9 +1036,12 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages } for (i = 0; i < pOrderBy->nExpr; i++) { if (pOrderBy->a[i].done == 0) { - sqlErrorMsg(pParse, - "%r ORDER BY term does not match any " - "column in the result set", i + 1); + const char *err = "Error at ORDER BY in place %d: "\ + "term does not match any column in "\ + "the result set"; + err = tt_sprintf(err, i + 1); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; return 1; } } @@ -1080,8 +1091,14 @@ sqlResolveOrderGroupBy(Parse *pParse, Select *pSelect, ExprList *pOrderBy, for (i = 0, pItem = pOrderBy->a; i < pOrderBy->nExpr; i++, pItem++) { if (pItem->u.x.iOrderByCol) { if (pItem->u.x.iOrderByCol > pEList->nExpr) { - resolveOutOfRangeError(pParse, zType, i + 1, - pEList->nExpr); + const char *err = "Error at %s BY in place "\ + "%d: term out of range - "\ + "should be between 1 and %d"; + err = tt_sprintf(err, zType, i + 1, + pEList->nExpr); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + err); + pParse->is_aborted = true; return 1; } resolveAlias(pParse, pEList, pItem->u.x.iOrderByCol - 1, @@ -1148,8 +1165,13 @@ resolveOrderGroupBy(NameContext *pNC, Select *pSelect, ExprList *pOrderBy, * order-by term to a copy of the result-set expression */ if (iCol < 1 || iCol > 0xffff) { - resolveOutOfRangeError(pParse, zType, i + 1, - nResult); + const char *err = "Error at %s BY in place "\ + "%d: term out of range - "\ + "should be between 1 and %d"; + err = tt_sprintf(err, zType, i + 1, nResult); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + err); + pParse->is_aborted = true; return 1; } pItem->u.x.iOrderByCol = (u16) iCol; @@ -1429,12 +1451,15 @@ resolveSelectStep(Walker * pWalker, Select * p) || db->mallocFailed) { return WRC_Abort; } + const char *err_msg = "aggregate functions are not "\ + "allowed in the GROUP BY clause"; for (i = 0, pItem = pGroupBy->a; i < pGroupBy->nExpr; i++, pItem++) { if (ExprHasProperty(pItem->pExpr, EP_Agg)) { - sqlErrorMsg(pParse, - "aggregate functions are not allowed in " - "the GROUP BY clause"); + diag_set(ClientError, + ER_SQL_PARSER_GENERIC, + err_msg); + pParse->is_aborted = true; return WRC_Abort; } } @@ -1444,10 +1469,37 @@ resolveSelectStep(Walker * pWalker, Select * p) * number of expressions in the select list. */ if (p->pNext && p->pEList->nExpr != p->pNext->pEList->nExpr) { - sqlSelectWrongNumTermsError(pParse, p->pNext); + if (p->pNext->selFlags & SF_Values) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "all VALUES must have the same "\ + "number of terms"); + } else { + const char *err_msg = + "SELECTs to the left and right of %s "\ + "do not have the same number of "\ + "result columns"; + switch (p->pNext->op) { + case TK_ALL: + err_msg = tt_sprintf(err_msg, + "UNION ALL"); + break; + case TK_INTERSECT: + err_msg = tt_sprintf(err_msg, + "INTERSECT"); + break; + case TK_EXCEPT: + err_msg = tt_sprintf(err_msg, "EXCEPT"); + break; + default: + err_msg = tt_sprintf(err_msg, "UNION"); + break; + } + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + err_msg); + } + pParse->is_aborted = true; return WRC_Abort; } - /* Advance to the next term of the compound */ p = p->pPrior; diff --git a/src/box/sql/select.c b/src/box/sql/select.c index f2cad56..ca8e5a8 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -397,13 +397,19 @@ sqlJoinType(Parse * pParse, Token * pA, Token * pB, Token * pC) } if ((jointype & (JT_INNER | JT_OUTER)) == (JT_INNER | JT_OUTER) || (jointype & JT_ERROR) != 0) { - const char *zSp = " "; assert(pB != 0); - if (pC == 0) { - zSp++; + const char *err; + if (pC == NULL) { + err = tt_sprintf("unknown or unsupported join type: "\ + "%.*s %.*s", pA->n, pA->z, pB->n, + pB->z); + } else { + err = tt_sprintf("unknown or unsupported join type: "\ + "%.*s %.*s %.*s", pA->n, pA->z, pB->n, + pB->z, pC->n, pC->z); } - sqlErrorMsg(pParse, "unknown or unsupported join type: " - "%T %T%s%T", pA, pB, zSp, pC); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; jointype = JT_INNER; } else if ((jointype & JT_OUTER) != 0 && (jointype & (JT_LEFT | JT_RIGHT)) != JT_LEFT) { @@ -590,9 +596,10 @@ sqlProcessJoin(Parse * pParse, Select * p) */ if (pRight->fg.jointype & JT_NATURAL) { if (pRight->pOn || pRight->pUsing) { - sqlErrorMsg(pParse, - "a NATURAL join may not have " - "an ON or USING clause", 0); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "a NATURAL join may not have " + "an ON or USING clause"); + pParse->is_aborted = true; return 1; } for (j = 0; j < (int)right_space->def->field_count; j++) { @@ -613,8 +620,10 @@ sqlProcessJoin(Parse * pParse, Select * p) /* Disallow both ON and USING clauses in the same join */ if (pRight->pOn && pRight->pUsing) { - sqlErrorMsg(pParse, "cannot have both ON and USING " - "clauses in the same join"); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "cannot have both ON and USING clauses in "\ + "the same join"); + pParse->is_aborted = true; return 1; } @@ -637,6 +646,8 @@ sqlProcessJoin(Parse * pParse, Select * p) * not contained in both tables to be joined. */ if (pRight->pUsing) { + const char *err = "cannot join using column %s - "\ + "column not present in both tables"; IdList *pList = pRight->pUsing; for (j = 0; j < pList->nId; j++) { char *zName; /* Name of the term in the USING clause */ @@ -650,10 +661,10 @@ sqlProcessJoin(Parse * pParse, Select * p) || !tableAndColumnIndex(pSrc, i + 1, zName, &iLeft, &iLeftCol) ) { - sqlErrorMsg(pParse, - "cannot join using column %s - column " - "not present in both tables", - zName); + err = tt_sprintf(err, zName); + diag_set(ClientError, + ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; return 1; } addWhereTerm(pParse, pSrc, iLeft, iLeftCol, @@ -2601,16 +2612,20 @@ multiSelect(Parse * pParse, /* Parsing context */ pPrior = p->pPrior; dest = *pDest; if (pPrior->pOrderBy) { - sqlErrorMsg(pParse, - "ORDER BY clause should come after %s not before", - selectOpName(p->op)); + const char *err_msg = + tt_sprintf("ORDER BY clause should come after %s not "\ + "before", selectOpName(p->op)); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); + pParse->is_aborted = true; rc = 1; goto multi_select_end; } if (pPrior->pLimit) { - sqlErrorMsg(pParse, - "LIMIT clause should come after %s not before", - selectOpName(p->op)); + const char *err_msg = + tt_sprintf("LIMIT clause should come after %s not "\ + "before", selectOpName(p->op)); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); + pParse->is_aborted = true; rc = 1; goto multi_select_end; } @@ -2988,19 +3003,6 @@ multiSelect(Parse * pParse, /* Parsing context */ } #endif /* SQL_OMIT_COMPOUND_SELECT */ -void -sqlSelectWrongNumTermsError(struct Parse *parse, struct Select * p) -{ - if (p->selFlags & SF_Values) { - sqlErrorMsg(parse, "all VALUES must have the same number "\ - "of terms"); - } else { - sqlErrorMsg(parse, "SELECTs to the left and right of %s " - "do not have the same number of result columns", - selectOpName(p->op)); - } -} - /** * Code an output subroutine for a coroutine implementation of a * SELECT statment. @@ -3622,7 +3624,19 @@ substExpr(Parse * pParse, /* Report errors here */ assert(pEList != 0 && pExpr->iColumn < pEList->nExpr); assert(pExpr->pLeft == 0 && pExpr->pRight == 0); if (sqlExprIsVector(pCopy)) { - sqlVectorErrorMsg(pParse, pCopy); + const char *err; + if (pCopy->flags & EP_xIsSelect) { + err = "sub-select returns %d columns "\ + "- expected 1"; + int expr_count = + pCopy->x.pSelect->pEList->nExpr; + err = tt_sprintf(err, expr_count); + } else { + err = "row value misused"; + } + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + err); + pParse->is_aborted = true; } else { pNew = sqlExprDup(db, pCopy, 0); if (pNew && (pExpr->flags & EP_FromJoin)) { @@ -4518,21 +4532,6 @@ convertCompoundSelectToSubquery(Walker * pWalker, Select * p) return WRC_Continue; } -/* - * Check to see if the FROM clause term pFrom has table-valued function - * arguments. If it does, leave an error message in pParse and return - * non-zero, since pFrom is not allowed to be a table-valued function. - */ -static int -cannotBeFunction(Parse * pParse, struct SrcList_item *pFrom) -{ - if (pFrom->fg.isTabFunc) { - sqlErrorMsg(pParse, "'%s' is not a function", pFrom->zName); - return 1; - } - return 0; -} - #ifndef SQL_OMIT_CTE /* * Argument pWith (which may be NULL) points to a linked list of nested @@ -4627,11 +4626,18 @@ withExpand(Walker * pWalker, struct SrcList_item *pFrom) * In this case, proceed. */ if (pCte->zCteErr) { - sqlErrorMsg(pParse, pCte->zCteErr, pCte->zName); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(pCte->zCteErr, pCte->zName)); + pParse->is_aborted = true; return SQL_ERROR; } - if (cannotBeFunction(pParse, pFrom)) + if (pFrom->fg.isTabFunc) { + const char *err = "'%s' is not a function"; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, pFrom->zName)); + pParse->is_aborted = true; return SQL_ERROR; + } assert(pFrom->space == NULL); pFrom->space = sql_ephemeral_space_new(pParse, pCte->zName); @@ -4663,9 +4669,11 @@ withExpand(Walker * pWalker, struct SrcList_item *pFrom) } } if (ref_counter > 1) { - sqlErrorMsg(pParse, - "multiple references to recursive table: %s", - pCte->zName); + const char *err_msg = + tt_sprintf("multiple references to recursive "\ + "table: %s", pCte->zName); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); + pParse->is_aborted = true; return SQL_ERROR; } assert(ref_counter == 0 || @@ -4681,10 +4689,13 @@ withExpand(Walker * pWalker, struct SrcList_item *pFrom) pEList = pLeft->pEList; if (pCte->pCols) { if (pEList && pEList->nExpr != pCte->pCols->nExpr) { - sqlErrorMsg(pParse, - "table %s has %d values for %d columns", - pCte->zName, pEList->nExpr, - pCte->pCols->nExpr); + const char *err_msg = + tt_sprintf("table %s has %d values "\ + "for %d columns", + pCte->zName, pEList->nExpr, + pCte->pCols->nExpr); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); + pParse->is_aborted = true; pParse->pWith = pSavedWith; return SQL_ERROR; } @@ -4840,8 +4851,15 @@ selectExpander(Walker * pWalker, Select * p) struct space *space = sql_lookup_space(pParse, pFrom); if (space == NULL) return WRC_Abort; - if (cannotBeFunction(pParse, pFrom)) + if (pFrom->fg.isTabFunc) { + const char *err = + tt_sprintf("'%s' is not a function", + pFrom->zName); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + err); + pParse->is_aborted = true; return WRC_Abort; + } if (space->def->opts.is_view) { struct Select *select = sql_view_compile(db, space->def->opts.sql); @@ -5252,9 +5270,10 @@ resetAccumulator(Parse * pParse, AggInfo * pAggInfo) Expr *pE = pFunc->pExpr; assert(!ExprHasProperty(pE, EP_xIsSelect)); if (pE->x.pList == 0 || pE->x.pList->nExpr != 1) { - sqlErrorMsg(pParse, - "DISTINCT aggregates must have exactly one " - "argument"); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "DISTINCT aggregates must have "\ + "exactly one argument"); + pParse->is_aborted = true; pFunc->iDistinct = -1; } else { struct sql_key_info *key_info = diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 1e49d5b..67a0fb0 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3194,7 +3194,6 @@ void sqlTreeViewWith(TreeView *, const With *); #endif void sqlSetString(char **, sql *, const char *); -void sqlErrorMsg(Parse *, const char *, ...); void sqlDequote(char *); void sqlNormalizeName(char *z); void sqlTokenInit(Token *, char *); @@ -4410,16 +4409,6 @@ void sqlExpirePreparedStatements(sql *); int sqlCodeSubselect(Parse *, Expr *, int); void sqlSelectPrep(Parse *, Select *, NameContext *); -/** - * Error message for when two or more terms of a compound select - * have different size result sets. - * - * @param parse Parsing context. - * @param p Select struct to analyze. - */ -void -sqlSelectWrongNumTermsError(struct Parse *parse, struct Select *p); - int sqlMatchSpanName(const char *, const char *, const char *); int sqlResolveExprNames(NameContext *, Expr *); int sqlResolveExprListNames(NameContext *, ExprList *); @@ -4868,7 +4857,6 @@ int sqlExprVectorSize(Expr * pExpr); int sqlExprIsVector(Expr * pExpr); Expr *sqlVectorFieldSubexpr(Expr *, int); Expr *sqlExprForVectorField(Parse *, Expr *, int); -void sqlVectorErrorMsg(Parse *, Expr *); /* Tarantool: right now query compilation is invoked on top of * fiber's stack. Need to limit number of nested programs under diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 2f1268a..9f06e53 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -621,8 +621,9 @@ codeTriggerProgram(Parse * pParse, /* The parser context */ sqlSubProgramsRemaining--; if (sqlSubProgramsRemaining == 0) { - sqlErrorMsg(pParse, - "Maximum number of chained trigger activations exceeded."); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Maximum number "\ + "of chained trigger activations exceeded."); + pParse->is_aborted = true; } for (pStep = pStepList; pStep; pStep = pStep->pNext) { diff --git a/src/box/sql/update.c b/src/box/sql/update.c index bc0ab66..ee3b3b5 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -179,10 +179,15 @@ sqlUpdate(Parse * pParse, /* The parser context */ sql_space_column_is_in_pk(space, j)) is_pk_modified = true; if (aXRef[j] != -1) { - sqlErrorMsg(pParse, - "set id list: duplicate" - " column name %s", - pChanges->a[i].zName); + const char *err = + "set id list: duplicate "\ + "column name %s"; + err = tt_sprintf(err, + pChanges->a[i].zName); + diag_set(ClientError, + ER_SQL_PARSER_GENERIC, + err); + pParse->is_aborted = true; goto update_cleanup; } aXRef[j] = i; diff --git a/src/box/sql/util.c b/src/box/sql/util.c index d9bb2af..cac404f 100644 --- a/src/box/sql/util.c +++ b/src/box/sql/util.c @@ -211,38 +211,6 @@ sqlErrorWithMsg(sql * db, int err_code, const char *zFormat, ...) } /* - * Add an error to the diagnostics area and set - * pParse->is_aborted. - * The following formatting characters are allowed: - * - * %s Insert a string - * %z A string that should be freed after use - * %d Insert an integer - * %T Insert a token - * %S Insert the first element of a SrcList - * - * This function should be used to report any error that occurs while - * compiling an SQL statement (i.e. within sql_prepare()). The - * last thing the sql_prepare() function does is copy the error - * stored by this function into the database handle using sqlError(). - * Functions sqlError() or sqlErrorWithMsg() should be used - * during statement execution (sql_step() etc.). - */ -void -sqlErrorMsg(Parse * pParse, const char *zFormat, ...) -{ - char *zMsg; - va_list ap; - sql *db = pParse->db; - va_start(ap, zFormat); - zMsg = sqlVMPrintf(db, zFormat, ap); - va_end(ap); - diag_set(ClientError, ER_SQL_PARSER_GENERIC, zMsg); - sqlDbFree(db, zMsg); - pParse->is_aborted = true; -} - -/* * Convert an SQL-style quoted string into a normal string by removing * the quote characters. The conversion is done in-place. If the * input does not begin with a quote character, then this routine diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index f417c49..9f0dee4 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -1210,13 +1210,14 @@ valueFromFunction(sql * db, /* The database connection */ pFunc->xSFunc(&ctx, nVal, apVal); if (ctx.isError) { rc = ctx.isError; - sqlErrorMsg(pCtx->pParse, "%s", sql_value_text(pVal)); + const char *err = "Error in function '%s': %s"; + err = tt_sprintf(err, pFunc->zName, sql_value_text(pVal)); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pCtx->pParse->is_aborted = true; } else { sql_value_apply_type(pVal, type); assert(rc == SQL_OK); } - if (rc != SQL_OK) - pCtx->pParse->is_aborted = true; value_from_function_out: if (rc != SQL_OK) { diff --git a/src/box/sql/where.c b/src/box/sql/where.c index 33885d0..1e8c2e0 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -3914,7 +3914,9 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst) } if (nFrom == 0) { - sqlErrorMsg(pParse, "no query solution"); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "no query solution"); + pParse->is_aborted = true; sqlDbFree(db, pSpace); return SQL_ERROR; } diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c index 6df28ad..bc0964c 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -1497,9 +1497,11 @@ sqlWhereTabFuncArgs(Parse * pParse, /* Parsing context */ while (k < (int)space_def->field_count) k++; if (k >= (int)space_def->field_count) { - sqlErrorMsg(pParse, - "too many arguments on %s() - max %d", - space_def->name, j); + const char *err = + tt_sprintf("too many arguments on %s() - max "\ + "%d", space_def->name, j); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; return; } pColRef = sqlExprAlloc(pParse->db, TK_COLUMN, 0, 0); diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua index e190ad7..8e9a2bb 100755 --- a/test/sql-tap/e_select1.test.lua +++ b/test/sql-tap/e_select1.test.lua @@ -1855,13 +1855,13 @@ test:do_catchsql_test( "e_select-8.7.1.1", "SELECT x FROM d1 UNION ALL SELECT a FROM d2 ORDER BY x*z", { - 1, "1st ORDER BY term does not match any column in the result set"}) + 1, "Error at ORDER BY in place 1: term does not match any column in the result set"}) test:do_catchsql_test( "e_select-8.7.1.2", "SELECT x,z FROM d1 UNION ALL SELECT a,b FROM d2 ORDER BY x, x/z", { - 1, "2nd ORDER BY term does not match any column in the result set"}) + 1, "Error at ORDER BY in place 2: term does not match any column in the result set"}) test:do_select_tests( "e_select-8.7.2", @@ -2077,12 +2077,12 @@ test:do_select_tests( -- EVIDENCE-OF: R-39265-04070 If no matching expression can be found in -- the result columns of any constituent SELECT, it is an error. -- -for _, val in ipairs({{1, "SELECT a FROM d5 UNION SELECT c FROM d6 ORDER BY a+1", "1st"}, - {2, "SELECT a FROM d5 UNION SELECT c FROM d6 ORDER BY a, a+1", "2nd"}, - {3, "SELECT * FROM d5 INTERSECT SELECT * FROM d6 ORDER BY \"hello\"", "1st"}, - {4, "SELECT * FROM d5 INTERSECT SELECT * FROM d6 ORDER BY blah", "1st"}, - {5, "SELECT * FROM d5 INTERSECT SELECT * FROM d6 ORDER BY c,d,c+d", "3rd"}, - {6, "SELECT * FROM d5 EXCEPT SELECT * FROM d7 ORDER BY 1,2,b,a/b", "4th"}}) do +for _, val in ipairs({{1, "SELECT a FROM d5 UNION SELECT c FROM d6 ORDER BY a+1", "1"}, + {2, "SELECT a FROM d5 UNION SELECT c FROM d6 ORDER BY a, a+1", "2"}, + {3, "SELECT * FROM d5 INTERSECT SELECT * FROM d6 ORDER BY \"hello\"", "1"}, + {4, "SELECT * FROM d5 INTERSECT SELECT * FROM d6 ORDER BY blah", "1"}, + {5, "SELECT * FROM d5 INTERSECT SELECT * FROM d6 ORDER BY c,d,c+d", "3"}, + {6, "SELECT * FROM d5 EXCEPT SELECT * FROM d7 ORDER BY 1,2,b,a/b", "4"}}) do local tn = val[1] local select = val[2] local err_param = val[3] @@ -2090,7 +2090,7 @@ for _, val in ipairs({{1, "SELECT a FROM d5 UNION SELECT c FROM d6 ORDER BY a+1" "e_select-8.14."..tn, select, { - 1, string.format("%s ORDER BY term does not match any column in the result set", err_param)}) + 1, string.format("Error at ORDER BY in place %s: term does not match any column in the result set", err_param)}) end -- EVIDENCE-OF: R-03407-11483 Each term of the ORDER BY clause is -- processed separately and may be matched against result columns from diff --git a/test/sql-tap/null.test.lua b/test/sql-tap/null.test.lua index 50a2cfb..de4d503 100755 --- a/test/sql-tap/null.test.lua +++ b/test/sql-tap/null.test.lua @@ -295,7 +295,7 @@ test:do_catchsql_test( select b from t1 union select c from t1 order by t1.a; ]], { -- - 1, "1st ORDER BY term does not match any column in the result set" + 1, "Error at ORDER BY in place 1: term does not match any column in the result set" -- }) @@ -305,7 +305,7 @@ test:do_catchsql_test( select b from t1 union select c from t1 order by t1.a; ]], { -- - 1, "1st ORDER BY term does not match any column in the result set" + 1, "Error at ORDER BY in place 1: term does not match any column in the result set" -- }) diff --git a/test/sql-tap/select1.test.lua b/test/sql-tap/select1.test.lua index d73429a..545ae9d 100755 --- a/test/sql-tap/select1.test.lua +++ b/test/sql-tap/select1.test.lua @@ -840,7 +840,7 @@ test:do_catchsql_test( SELECT * FROM t5 ORDER BY 3; ]], { -- - 1, "1st ORDER BY term out of range - should be between 1 and 2" + 1, "Error at ORDER BY in place 1: term out of range - should be between 1 and 2" -- }) @@ -850,7 +850,7 @@ test:do_catchsql_test( SELECT * FROM t5 ORDER BY -1; ]], { -- - 1, "1st ORDER BY term out of range - should be between 1 and 2" + 1, "Error at ORDER BY in place 1: term out of range - should be between 1 and 2" -- }) @@ -1334,7 +1334,7 @@ test:do_catchsql2_test( ORDER BY f2+101; ]], { -- - 1, "1st ORDER BY term does not match any column in the result set" + 1, "Error at ORDER BY in place 1: term does not match any column in the result set" -- }) diff --git a/test/sql-tap/select3.test.lua b/test/sql-tap/select3.test.lua index 9fb825f..3c88f77 100755 --- a/test/sql-tap/select3.test.lua +++ b/test/sql-tap/select3.test.lua @@ -157,7 +157,7 @@ test:do_catchsql_test("select3-2.10", [[ SELECT log, count(*) FROM t1 GROUP BY 0 ORDER BY log; ]], { -- - 1, "1st GROUP BY term out of range - should be between 1 and 2" + 1, "Error at GROUP BY in place 1: term out of range - should be between 1 and 2" -- }) @@ -165,7 +165,7 @@ test:do_catchsql_test("select3-2.11", [[ SELECT log, count(*) FROM t1 GROUP BY 3 ORDER BY log; ]], { -- - 1, "1st GROUP BY term out of range - should be between 1 and 2" + 1, "Error at GROUP BY in place 1: term out of range - should be between 1 and 2" -- }) diff --git a/test/sql-tap/select4.test.lua b/test/sql-tap/select4.test.lua index bd2ada9..3aafedb 100755 --- a/test/sql-tap/select4.test.lua +++ b/test/sql-tap/select4.test.lua @@ -450,7 +450,7 @@ test:do_catchsql_test( ORDER BY "xyzzy"; ]], { -- - 1, "1st ORDER BY term does not match any column in the result set" + 1, "Error at ORDER BY in place 1: term does not match any column in the result set" -- }) @@ -463,7 +463,7 @@ test:do_catchsql_test( ORDER BY "xyzzy"; ]], { -- - 1, "1st ORDER BY term does not match any column in the result set" + 1, "Error at ORDER BY in place 1: term does not match any column in the result set" -- }) @@ -515,7 +515,7 @@ test:do_catchsql_test( ORDER BY 2; ]], { -- - 1, "1st ORDER BY term out of range - should be between 1 and 1" + 1, "Error at ORDER BY in place 1: term out of range - should be between 1 and 1" -- }) diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua index 4a92af6..4caa44f 100755 --- a/test/sql-tap/sql-errors.test.lua +++ b/test/sql-tap/sql-errors.test.lua @@ -1,9 +1,9 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(36) +test:plan(40) test:execsql([[ - CREATE TABLE t0 (i INT PRIMARY KEY); + CREATE TABLE t0 (i INT PRIMARY KEY, a INT); CREATE VIEW v0 AS SELECT * FROM t0; ]]) format = {} @@ -79,7 +79,7 @@ test:do_catchsql_test( test:do_catchsql_test( "sql-errors-1.7", [[ - CREATE VIEW v7(a,b) AS SELECT * FROM t0; + CREATE VIEW v7(a,b,c) AS SELECT * FROM t0; ]], { -- 1,"Failed to create space 'V7': number of aliases doesn't match provided columns" @@ -416,4 +416,44 @@ test:do_catchsql_test( -- }) +test:do_catchsql_test( + "sql-errors-1.37", + [[ + CREATE TRIGGER r0 AFTER INSERT ON t0 BEGIN INSERT INTO t0.i VALUES (2); END; + ]], { + -- + 1,"qualified table names are not allowed on INSERT, UPDATE, and DELETE statements within triggers" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.38", + [[ + UPDATE t0 SET (i, a) = (100,1,1); + ]], { + -- + 1,"2 columns assigned 3 values" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.39", + [[ + SELECT * FROM t0(); + ]], { + -- + 1,"'T0' is not a function" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.40", + [[ + SELECT $0; + ]], { + -- + 1,"Index of binding slots must start from 1" + -- + }) + test:finish_test() diff --git a/test/sql-tap/tkt2822.test.lua b/test/sql-tap/tkt2822.test.lua index 4212cbd..86674ae 100755 --- a/test/sql-tap/tkt2822.test.lua +++ b/test/sql-tap/tkt2822.test.lua @@ -200,7 +200,7 @@ test:do_catchsql_test( SELECT a, b, c FROM t1 UNION ALL SELECT a, b, c FROM t2 ORDER BY x ]], { -- - 1, "1st ORDER BY term does not match any column in the result set" + 1, "Error at ORDER BY in place 1: term does not match any column in the result set" -- }) @@ -298,7 +298,7 @@ test:do_test( ]] end, { -- - 1, "1st ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 1: term out of range - should be between 1 and 25" -- }) @@ -308,7 +308,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 0; ]], { -- - 1, "2nd ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 2: term out of range - should be between 1 and 25" -- }) @@ -318,7 +318,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 26; ]], { -- - 1, "2nd ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 2: term out of range - should be between 1 and 25" -- }) @@ -328,7 +328,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 65536; ]], { -- - 1, "2nd ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 2: term out of range - should be between 1 and 25" -- }) @@ -338,7 +338,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 2, 0; ]], { -- - 1, "3rd ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 3: term out of range - should be between 1 and 25" -- }) @@ -348,7 +348,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 2, 3, 0; ]], { -- - 1, "4th ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 4: term out of range - should be between 1 and 25" -- }) @@ -358,7 +358,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 2, 3, 4, 5, 6, 7, 8, 0; ]], { -- - 1, "9th ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 9: term out of range - should be between 1 and 25" -- }) @@ -368,7 +368,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 0; ]], { -- - 1, "10th ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 10: term out of range - should be between 1 and 25" -- }) @@ -378,7 +378,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 0; ]], { -- - 1, "11th ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 11: term out of range - should be between 1 and 25" -- }) @@ -388,7 +388,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 0; ]], { -- - 1, "12th ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 12: term out of range - should be between 1 and 25" -- }) @@ -398,7 +398,7 @@ test:do_catchsql_test( SELECT * FROM t7 ORDER BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 13, 0; ]], { -- - 1, "13th ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 13: term out of range - should be between 1 and 25" -- }) @@ -409,7 +409,7 @@ test:do_catchsql_test( 11,12,13,14,15,16,17,18,19, 0 ]], { -- - 1, "20th ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 20: term out of range - should be between 1 and 25" -- }) @@ -420,7 +420,7 @@ test:do_catchsql_test( 11,12,13,14,15,16,17,18,19, 20, 0 ]], { -- - 1, "21st ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 21: term out of range - should be between 1 and 25" -- }) @@ -431,7 +431,7 @@ test:do_catchsql_test( 11,12,13,14,15,16,17,18,19, 20, 21, 0 ]], { -- - 1, "22nd ORDER BY term out of range - should be between 1 and 25" + 1, "Error at ORDER BY in place 22: term out of range - should be between 1 and 25" -- }) diff --git a/test/sql-tap/with1.test.lua b/test/sql-tap/with1.test.lua index 16c9b12..f1a1699 100755 --- a/test/sql-tap/with1.test.lua +++ b/test/sql-tap/with1.test.lua @@ -134,7 +134,7 @@ test:do_catchsql_test(3.2, [[ SELECT * FROM tmp; ]], { -- <3.2> - 1, "duplicate WITH table name: TMP" + 1, "Ambiguous table name in WITH query: TMP" -- }) @@ -782,7 +782,7 @@ test:do_catchsql_test("10.7.1", [[ SELECT * FROM t ]], { -- <10.7.1> - 1, "1st ORDER BY term does not match any column in the result set" + 1, "Error at ORDER BY in place 1: term does not match any column in the result set" -- }) diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 56099fa..48376c8 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -571,7 +571,8 @@ cn:execute('select ?1, ?2, ?3', {1, 2, 3}) ... cn:execute('select $name, $name2', {1, 2}) --- -- error: 'Failed to execute SQL statement: SQL bind parameter limit reached: 65000' +- error: 'Failed to execute SQL statement: Index of binding slots must start from + 1' ... parameters = {} --- -- 2.7.4