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 DF477284FA for ; Tue, 26 Mar 2019 13:52:25 -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 C9sDJO99vxEc for ; Tue, 26 Mar 2019 13:52:25 -0400 (EDT) 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 EE80027D75 for ; Tue, 26 Mar 2019 13:52:23 -0400 (EDT) Date: Tue, 26 Mar 2019 20:52:20 +0300 From: Mergen Imeev Subject: [tarantool-patches] Re: [PATCH v4 8/8] sql: remove sqlErrorMsg() Message-ID: <20190326175219.GA30438@tarantool.org> References: <99f4adc61352899a83a7097b965845bafaa968ef.1552494059.git.imeevma@gmail.com> <0D9B8996-D469-498B-8D6C-AFD90C6DD35E@tarantool.org> <20190325184737.GA18346@tarantool.org> <2E9AC693-B13A-4FFD-8F0D-A3FA7AA442A3@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2E9AC693-B13A-4FFD-8F0D-A3FA7AA442A3@tarantool.org> 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: "n.pettik" Cc: tarantool-patches@freelists.org Hi! Thank you for review. My answers, diff and new patch below. On Tue, Mar 26, 2019 at 04:34:26PM +0300, n.pettik wrote: > > >> @@ -1039,8 +1036,8 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages > >> 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); > >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, > >> + tt_sprintf(err, i + 1)); > >> > >>> @@ -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; > >> > >> Why don’t use selectOpName? > >> Like it was in sqlSelectWrongNumTermsError(). > >> > > Function selectOpName defined in different file and is static. > > I think it isn't worth to share it for one error. > > I believe it is worth doing. For instance, you can return > sqlSelectWrongNumTermsError() which was placed in select.c > and called that static func. > Fixed. > > Diff between versions: > > > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > > index e26596a..1077285 100644 > > --- a/src/box/sql/expr.c > > +++ b/src/box/sql/expr.c > > @@ -666,13 +666,7 @@ codeVectorCompare(Parse * pParse, /* Code generator context */ > > int regRight = 0; > > u8 op = pExpr->op; > > int addrDone = sqlVdbeMakeLabel(v); > > - > > - if (nLeft != sqlExprVectorSize(pRight)) { > > - diag_set(ClientError, ER_SQL_PARSER_GENERIC, > > - "row value misused"); > > - pParse->is_aborted = true; > > - return; > > - } > > Here the problem is that function is never called in our test suite: > I placed assert(0); and no one test failed. Please, add at least > simple tests like SELECT (1, 2) == (1, 2) to make sure that > codeVectorCompare() can be processed. Tests which you’ve added > fails before this func is called. > > Added comment: > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index 107728590..397f8209c 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -666,6 +666,11 @@ codeVectorCompare(Parse * pParse, /* Code generator context */ > int regRight = 0; > u8 op = pExpr->op; > int addrDone = sqlVdbeMakeLabel(v); > + /* > + * Situation when vectors have different dimensions is > + * filtred way before - during expr resolution: > + * see resolveExprStep(). > + */ > Added. > > + assert(nLeft == sqlExprVectorSize(pRight)); > > assert(pExpr->op == TK_EQ || pExpr->op == TK_NE > > || pExpr->op == TK_LT || pExpr->op == TK_GT > > || pExpr->op == TK_LE || pExpr->op == TK_GE); > > > > > > @@ -4384,10 +4378,8 @@ sqlWhereBegin(Parse * pParse, /* The parser context */ > > * equal to pTabList->nSrc but might be shortened to 1 if the > > * WHERE_OR_SUBCLAUSE flag is set. > > */ > > - for (ii = 0; ii < pTabList->nSrc; ii++) { > > + for (ii = 0; ii < pTabList->nSrc; ii++) > > createMask(pMaskSet, pTabList->a[ii].iCursor); > > - sqlWhereTabFuncArgs(pParse, &pTabList->a[ii], &pWInfo->sWC); > > Move removal of this func to a separate patch pls alongside with mentions of > table-valued funcs. It is not related to errors. > > Returned this function, replaced error in it by assert. Diff: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 1077285..80d17d1 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -666,6 +666,12 @@ codeVectorCompare(Parse * pParse, /* Code generator context */ int regRight = 0; u8 op = pExpr->op; int addrDone = sqlVdbeMakeLabel(v); + + /* + * Situation when vectors have different dimensions is + * filtred way before - during expr resolution: + * see resolveExprStep(). + */ assert(nLeft == sqlExprVectorSize(pRight)); assert(pExpr->op == TK_EQ || pExpr->op == TK_NE || pExpr->op == TK_LT || pExpr->op == TK_GT diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index c08d934..e4e3697 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -1459,28 +1459,13 @@ resolveSelectStep(Walker * pWalker, Select * p) "all VALUES must have the same "\ "number of terms"); } else { - const char *err_msg = + const char *err = "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; - } + const char *op = select_op_name(p->pNext->op); diag_set(ClientError, ER_SQL_PARSER_GENERIC, - err_msg); + tt_sprintf(err, op)); } pParse->is_aborted = true; return WRC_Abort; diff --git a/src/box/sql/select.c b/src/box/sql/select.c index e217c19..778de2c 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1466,25 +1466,25 @@ sql_expr_list_to_key_info(struct Parse *parse, struct ExprList *list, int start) /* * Name of the connection operator, used for error messages. */ -static const char * -selectOpName(int id) +const char * +select_op_name(int op_id) { - char *z; - switch (id) { + const char *op_name; + switch (op_id) { case TK_ALL: - z = "UNION ALL"; + op_name = "UNION ALL"; break; case TK_INTERSECT: - z = "INTERSECT"; + op_name = "INTERSECT"; break; case TK_EXCEPT: - z = "EXCEPT"; + op_name = "EXCEPT"; break; default: - z = "UNION"; + op_name = "UNION"; break; } - return z; + return op_name; } /* @@ -1542,7 +1542,7 @@ explainComposite(Parse * pParse, /* Parse context */ "COMPOUND SUBQUERIES %d AND %d %s(%s)", iSub1, iSub2, bUseTmp ? "USING TEMP B-TREE " : "", - selectOpName(op) + select_op_name(op) ); sqlVdbeAddOp4(v, OP_Explain, pParse->iSelectId, 0, 0, zMsg, P4_DYNAMIC); @@ -2614,7 +2614,7 @@ multiSelect(Parse * pParse, /* Parsing context */ if (pPrior->pOrderBy) { const char *err_msg = tt_sprintf("ORDER BY clause should come after %s not "\ - "before", selectOpName(p->op)); + "before", select_op_name(p->op)); diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); pParse->is_aborted = true; rc = 1; @@ -2623,7 +2623,7 @@ multiSelect(Parse * pParse, /* Parsing context */ if (pPrior->pLimit) { const char *err_msg = tt_sprintf("LIMIT clause should come after %s not "\ - "before", selectOpName(p->op)); + "before", select_op_name(p->op)); diag_set(ClientError, ER_SQL_PARSER_GENERIC, err_msg); pParse->is_aborted = true; rc = 1; diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 15e6f22..50fc812 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4426,6 +4426,15 @@ void sqlExpirePreparedStatements(sql *); int sqlCodeSubselect(Parse *, Expr *, int); void sqlSelectPrep(Parse *, Select *, NameContext *); +/** + * Returns name of the connection operator. + * + * @param op_id ID of the connection operator. + * @retval Name of the connection operator. + */ +const char * +select_op_name(int op_id); + int sqlMatchSpanName(const char *, const char *, const char *); int sqlResolveExprNames(NameContext *, Expr *); int sqlResolveExprListNames(NameContext *, ExprList *); diff --git a/src/box/sql/where.c b/src/box/sql/where.c index c71a170..19ee2d0 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -4378,8 +4378,10 @@ sqlWhereBegin(Parse * pParse, /* The parser context */ * equal to pTabList->nSrc but might be shortened to 1 if the * WHERE_OR_SUBCLAUSE flag is set. */ - for (ii = 0; ii < pTabList->nSrc; ii++) + for (ii = 0; ii < pTabList->nSrc; ii++) { createMask(pMaskSet, pTabList->a[ii].iCursor); + sqlWhereTabFuncArgs(pParse, &pTabList->a[ii], &pWInfo->sWC); + } #ifdef SQL_DEBUG for (ii = 0; ii < pTabList->nSrc; ii++) { Bitmask m = diff --git a/src/box/sql/whereInt.h b/src/box/sql/whereInt.h index 9cf1eb8..47430ae 100644 --- a/src/box/sql/whereInt.h +++ b/src/box/sql/whereInt.h @@ -479,6 +479,7 @@ void sqlWhereSplit(WhereClause *, Expr *, u8); Bitmask sqlWhereExprUsage(WhereMaskSet *, Expr *); Bitmask sqlWhereExprListUsage(WhereMaskSet *, ExprList *); void sqlWhereExprAnalyze(SrcList *, WhereClause *); +void sqlWhereTabFuncArgs(Parse *, struct SrcList_item *, WhereClause *); /* * Bitmasks for the operators on WhereTerm objects. These are all diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c index 9d01a7e..ec08889 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -1472,3 +1472,45 @@ sqlWhereExprAnalyze(SrcList * pTabList, /* the FROM clause */ exprAnalyze(pTabList, pWC, i); } } + +/* + * For table-valued-functions, transform the function arguments into + * new WHERE clause terms. + */ +void +sqlWhereTabFuncArgs(Parse * pParse, /* Parsing context */ + struct SrcList_item *pItem, /* The FROM clause term to process */ + WhereClause * pWC /* Xfer function arguments to here */ + ) +{ + int j, k; + ExprList *pArgs; + Expr *pColRef; + Expr *pTerm; + if (pItem->fg.isTabFunc == 0) + return; + struct space_def *space_def = pItem->space->def; + pArgs = pItem->u1.pFuncArg; + if (pArgs == 0) + return; + for (j = k = 0; j < pArgs->nExpr; j++) { + while (k < (int)space_def->field_count) + k++; + /* + * This assert replaces error. At the moment, this + * error cannot appear due to this function being + * unused. + */ + assert(k < (int)space_def->field_count); + pColRef = sqlExprAlloc(pParse->db, TK_COLUMN, 0, 0); + if (pColRef == 0) + return; + pColRef->iTable = pItem->iCursor; + pColRef->iColumn = k++; + pColRef->space_def = space_def; + pTerm = sqlPExpr(pParse, TK_EQ, pColRef, + sqlExprDup(pParse->db, + pArgs->a[j].pExpr, 0)); + whereClauseInsert(pWC, pTerm, TERM_DYNAMIC); + } +} diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua index a132139..5585dbb 100755 --- a/test/sql-tap/sql-errors.test.lua +++ b/test/sql-tap/sql-errors.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(43) +test:plan(45) test:execsql([[ CREATE TABLE t0 (i INT PRIMARY KEY, a INT); @@ -486,4 +486,24 @@ test:do_catchsql_test( -- }) +test:do_execsql_test( + "sql-errors-1.44", + [[ + SELECT (1, 2, 3) < (1, 2, 4); + ]], { + -- + 1 + -- + }) + +test:do_execsql_test( + "sql-errors-1.45", + [[ + SELECT (1, 2, 3) < (1, 2, 2); + ]], { + -- + 0 + -- + }) + test:finish_test() New patch: commit 43d2787aeec1eb133e0261308d4b6242fe220833 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 fe53262..39c86bc 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -499,13 +499,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; } @@ -3057,11 +3056,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 0eb28d7..f536bb8 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -102,10 +102,10 @@ 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) { diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index de993c1..80d17d1 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -667,10 +667,12 @@ codeVectorCompare(Parse * pParse, /* Code generator context */ u8 op = pExpr->op; int addrDone = sqlVdbeMakeLabel(v); - if (nLeft != sqlExprVectorSize(pRight)) { - sqlErrorMsg(pParse, "row value misused"); - return; - } + /* + * Situation when vectors have different dimensions is + * filtred way before - during expr resolution: + * see resolveExprStep(). + */ + assert(nLeft == sqlExprVectorSize(pRight)); assert(pExpr->op == TK_EQ || pExpr->op == TK_NE || pExpr->op == TK_LT || pExpr->op == TK_GT || pExpr->op == TK_LE || pExpr->op == TK_GE); @@ -1201,7 +1203,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; @@ -1785,8 +1794,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; } @@ -2669,41 +2680,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: * @@ -2984,15 +2960,23 @@ 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; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, expr_count, nVector)); + pParse->is_aborted = true; return 1; } } else if (nVector != 1) { - sqlVectorErrorMsg(pParse, pIn->pLeft); + assert((pIn->pLeft->flags & EP_xIsSelect) != 0); + int expr_count = pIn->pLeft->x.pSelect->pEList->nExpr; + err = tt_sprintf("sub-select returns %d columns - expected 1", + expr_count); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, err); + pParse->is_aborted = true; return 1; } return 0; @@ -3990,9 +3974,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; @@ -4159,7 +4144,11 @@ 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"; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, nCol)); + pParse->is_aborted = true; } else { return sqlCodeSubselect(pParse, pExpr, 0); } @@ -4178,9 +4167,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; } @@ -4279,7 +4270,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; } @@ -4379,8 +4372,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 0ca38ae..7e98f42 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"; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, nColumn, pColumn->nId)); + pParse->is_aborted = true; goto insert_cleanup; } diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index a141065..d2614d9 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -904,7 +904,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; @@ -1390,9 +1392,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 30a6b5f..e4e3697 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -390,16 +390,21 @@ 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); + diag_set(ClientError, + ER_SQL_PARSER_GENERIC, + tt_sprintf(err, zAs)); + 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 +431,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) { @@ -626,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) { @@ -639,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++; @@ -679,9 +687,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 @@ -693,9 +703,11 @@ 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()"; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, nId, zId)); + pParse->is_aborted = true; pNC->nErr++; } if (is_agg) @@ -796,7 +808,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; } @@ -898,21 +912,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. @@ -973,9 +972,15 @@ 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 { @@ -1021,9 +1026,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"; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, i + 1)); + pParse->is_aborted = true; return 1; } } @@ -1071,8 +1079,14 @@ sqlResolveOrderGroupBy(Parse * pParse, /* Parsing context. Leave error messages 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, @@ -1136,8 +1150,13 @@ resolveOrderGroupBy(NameContext * pNC, /* The name context of the SELECT stateme * 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; @@ -1417,12 +1436,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; } } @@ -1432,10 +1454,22 @@ 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 = + "SELECTs to the left and right of %s "\ + "do not have the same number of "\ + "result columns"; + const char *op = select_op_name(p->pNext->op); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, op)); + } + 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 23c9499..778de2c 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, @@ -1455,25 +1466,25 @@ sql_expr_list_to_key_info(struct Parse *parse, struct ExprList *list, int start) /* * Name of the connection operator, used for error messages. */ -static const char * -selectOpName(int id) +const char * +select_op_name(int op_id) { - char *z; - switch (id) { + const char *op_name; + switch (op_id) { case TK_ALL: - z = "UNION ALL"; + op_name = "UNION ALL"; break; case TK_INTERSECT: - z = "INTERSECT"; + op_name = "INTERSECT"; break; case TK_EXCEPT: - z = "EXCEPT"; + op_name = "EXCEPT"; break; default: - z = "UNION"; + op_name = "UNION"; break; } - return z; + return op_name; } /* @@ -1531,7 +1542,7 @@ explainComposite(Parse * pParse, /* Parse context */ "COMPOUND SUBQUERIES %d AND %d %s(%s)", iSub1, iSub2, bUseTmp ? "USING TEMP B-TREE " : "", - selectOpName(op) + select_op_name(op) ); sqlVdbeAddOp4(v, OP_Explain, pParse->iSelectId, 0, 0, zMsg, P4_DYNAMIC); @@ -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", select_op_name(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", select_op_name(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. @@ -3624,7 +3626,14 @@ 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); + assert((pCopy->flags & EP_xIsSelect) != 0); + const char *err = "sub-select returns %d "\ + "columns - expected 1"; + int expr_count = + pCopy->x.pSelect->pEList->nExpr; + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, expr_count)); + pParse->is_aborted = true; } else { pNew = sqlExprDup(db, pCopy, 0); if (pNew && (pExpr->flags & EP_FromJoin)) { @@ -4520,21 +4529,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 @@ -4629,11 +4623,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); @@ -4665,9 +4666,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 || @@ -4683,10 +4686,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; } @@ -4842,8 +4848,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); @@ -5254,9 +5267,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 8fb2b18..50fc812 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3193,7 +3193,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 *); @@ -4428,14 +4427,13 @@ 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. + * Returns name of the connection operator. * - * @param parse Parsing context. - * @param p Select struct to analyze. + * @param op_id ID of the connection operator. + * @retval Name of the connection operator. */ -void -sqlSelectWrongNumTermsError(struct Parse *parse, struct Select *p); +const char * +select_op_name(int op_id); int sqlMatchSpanName(const char *, const char *, const char *); int sqlResolveExprNames(NameContext *, Expr *); @@ -4884,7 +4882,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 b23d60a..c984950 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 71a1e00..0b645eb 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..15a2f55 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -1208,15 +1208,9 @@ valueFromFunction(sql * db, /* The database connection */ ctx.pOut = pVal; ctx.pFunc = pFunc; pFunc->xSFunc(&ctx, nVal, apVal); - if (ctx.isError) { - rc = ctx.isError; - sqlErrorMsg(pCtx->pParse, "%s", sql_value_text(pVal)); - } else { - sql_value_apply_type(pVal, type); - assert(rc == SQL_OK); - } - if (rc != SQL_OK) - pCtx->pParse->is_aborted = true; + assert(!ctx.isError); + sql_value_apply_type(pVal, type); + assert(rc == SQL_OK); value_from_function_out: if (rc != SQL_OK) { diff --git a/src/box/sql/where.c b/src/box/sql/where.c index 2f83e81..19ee2d0 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -3913,11 +3913,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst) nFrom = nTo; } - if (nFrom == 0) { - sqlErrorMsg(pParse, "no query solution"); - sqlDbFree(db, pSpace); - return SQL_ERROR; - } + assert(nFrom != 0); /* Find the lowest cost path. pFrom will be left pointing to that path */ pFrom = aFrom; diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c index 6df28ad..ec08889 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -1496,12 +1496,12 @@ sqlWhereTabFuncArgs(Parse * pParse, /* Parsing context */ for (j = k = 0; j < pArgs->nExpr; j++) { 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); - return; - } + /* + * This assert replaces error. At the moment, this + * error cannot appear due to this function being + * unused. + */ + assert(k < (int)space_def->field_count); pColRef = sqlExprAlloc(pParse->db, TK_COLUMN, 0, 0); if (pColRef == 0) return; 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 635df6a..5585dbb 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(45) 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,94 @@ 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:do_catchsql_test( + "sql-errors-1.41", + [[ + SELECT (1,2,3) == (1,2,3,4); + ]], { + -- + 1,"row value misused" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.42", + [[ + SELECT (1, 2); + ]], { + -- + 1,"row value misused" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.43", + [[ + SELECT (i,a) AS m FROM t0 WHERE m < 1; + ]], { + -- + 1,"row value misused" + -- + }) + +test:do_execsql_test( + "sql-errors-1.44", + [[ + SELECT (1, 2, 3) < (1, 2, 4); + ]], { + -- + 1 + -- + }) + +test:do_execsql_test( + "sql-errors-1.45", + [[ + SELECT (1, 2, 3) < (1, 2, 2); + ]], { + -- + 0 + -- + }) + 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 = {} ---