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 A870229DA1 for ; Mon, 25 Mar 2019 14:47:42 -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 iXzVqY2uuCMA for ; Mon, 25 Mar 2019 14:47:42 -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 9F1CC29D8A for ; Mon, 25 Mar 2019 14:47:41 -0400 (EDT) Date: Mon, 25 Mar 2019 21:47:37 +0300 From: Mergen Imeev Subject: [tarantool-patches] Re: [PATCH v4 8/8] sql: remove sqlErrorMsg() Message-ID: <20190325184737.GA18346@tarantool.org> References: <99f4adc61352899a83a7097b965845bafaa968ef.1552494059.git.imeevma@gmail.com> <0D9B8996-D469-498B-8D6C-AFD90C6DD35E@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <0D9B8996-D469-498B-8D6C-AFD90C6DD35E@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 between versions and new version below. On Fri, Mar 15, 2019 at 04:36:46PM +0300, n.pettik wrote: > > >>> + 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. > > Under the hood SQL_OK is 0. We are going to remove > this status codes, so let's fix it now. > Fixed in previous patch. > >>> 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. Fixed. Replaced errors that I wasn't able to reproduce by assert. Some of them were replaced in previous patch. > > This error is valid. As for the other ones - I couldn't reach some > > of them, but didn't remove them for now. > > Why? Comment code and place assertion instead. > Otherwise, provide solid argument to avoid doing that. > Fixed. > >>> -/* > >>> * 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. Replaced by assert. > > I did, but not sure if this error is reachable. I wasn't able to > > reproduce it. > > See comment above. > Fixed. > >>> + 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. > > The same. > Fixed. > > 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 > > I wouldn’t close both issues since there’s still place to work on: > split generic error into specific ones, use diag for vdbe. > I created new issue for VDBE errors: https://github.com/tarantool/tarantool/issues/4074 I will write about replacing GENERIC errcode by specific one when #3036 and #3965 will be closed. I am going to close them after this patch pass through review. Futher work will be done under new issues. > > 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”); > > I see a lot of “row value misused” errors in source code, > but only one test case on this error. > I replaced three of them by assert. For remaining three I created tests. > > @@ -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”; > > A bit shorter version. Please apply this and other diffs: > Fixed. > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index 7205cb0a8..344e1ab9a 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -2940,16 +2940,16 @@ sqlExprCheckIN(Parse * pParse, Expr * pIn) > if (nVector != pIn->x.pSelect->pEList->nExpr) { > 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); > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, > + tt_sprintf(err, expr_count, nVector);); > pParse->is_aborted = true; > return 1; > } > } else if (nVector != 1) { > 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); > + 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; > > @@ -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); > > @@ -4124,9 +4124,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > && (nCol = pExpr->x.pSelect->pEList->nExpr) != 1) { > const char *err = "sub-select returns %d "\ > "columns - expected 1"; > - err = tt_sprintf(err, nCol); > diag_set(ClientError, ER_SQL_PARSER_GENERIC, > - err); > + tt_sprintf(err, nCol);); > > > @@ -510,14 +511,19 @@ sqlInsert(Parse * pParse, /* Parser context */ > > > > 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); > > diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c > index 7635f7b69..6f30ab37e 100644 > --- a/src/box/sql/insert.c > +++ b/src/box/sql/insert.c > @@ -521,8 +521,8 @@ sqlInsert(Parse * pParse, /* Parser context */ > } > if (pColumn != 0 && 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); > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, > + tt_sprintf(err, nColumn, pColumn->nId)); > pParse->is_aborted = true; > goto insert_cleanup; > > > + pParse->is_aborted = true; > > goto insert_cleanup; > > } > > > > > > // 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); > > diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c > index 672091465..30f2b1f5a 100644 > --- a/src/box/sql/resolve.c > +++ b/src/box/sql/resolve.c > @@ -394,10 +394,9 @@ lookupName(Parse * pParse, /* The parsing context */ > "aggregate %s"; > if ((pNC->ncFlags & NC_AllowAgg) == 0 > && ExprHasProperty(pOrig, EP_Agg)) { > - err = tt_sprintf(err, zAs); > diag_set(ClientError, > ER_SQL_PARSER_GENERIC, > - err); > + tt_sprintf(err, zAs)); > pParse->is_aborted = true; > > > @@ -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); > > @@ -710,9 +709,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) > } else if (wrong_num_args) { > const char *err = "wrong number of arguments "\ > "to function %.*s()"; > - err = tt_sprintf(err, nId, zId); > diag_set(ClientError, ER_SQL_PARSER_GENERIC, > - err); > + tt_sprintf(err, nId, zId)); > pParse->is_aborted = true; > pNC->nErr++; > > > @@ -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); > > @@ -988,8 +986,7 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages > err = tt_sprintf(err, i + 1, > pEList->nExpr); > diag_set(ClientError, > - ER_SQL_PARSER_GENERIC, > - err); > + ER_SQL_PARSER_GENERIC, err); > pParse->is_aborted = true; > return 1; > > > @@ -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); > > @@ -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. > > + 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; > > > > 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”; > > It doesn’t look better than previous variant. If you don’t > know what kind of error this is and how it can appear - > just replace it with assertion. > Replaced bu assert. > > + 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”); > > As I already said in previous review - replace with assertion > and comment. > Fixed. > > + pParse->is_aborted = true; > > sqlDbFree(db, pSpace); > > return SQL_ERROR; > > } > 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; - } + 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); @@ -2965,19 +2959,16 @@ sqlExprCheckIN(Parse * pParse, Expr * pIn) if (nVector != pIn->x.pSelect->pEList->nExpr) { 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); + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + tt_sprintf(err, expr_count, nVector)); pParse->is_aborted = true; return 1; } } else if (nVector != 1) { - 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"; - } + 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; @@ -4149,9 +4140,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) && (nCol = pExpr->x.pSelect->pEList->nExpr) != 1) { const char *err = "sub-select returns %d "\ "columns - expected 1"; - err = tt_sprintf(err, nCol); diag_set(ClientError, ER_SQL_PARSER_GENERIC, - err); + tt_sprintf(err, nCol)); pParse->is_aborted = true; } else { return sqlCodeSubselect(pParse, pExpr, 0); diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index b89acdc..7e98f42 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -521,8 +521,8 @@ sqlInsert(Parse * pParse, /* Parser context */ } if (pColumn != 0 && 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); + 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/resolve.c b/src/box/sql/resolve.c index 208ebd0..ef40927 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -394,10 +394,9 @@ lookupName(Parse * pParse, /* The parsing context */ "aggregate %s"; if ((pNC->ncFlags & NC_AllowAgg) == 0 && ExprHasProperty(pOrig, EP_Agg)) { - err = tt_sprintf(err, zAs); diag_set(ClientError, ER_SQL_PARSER_GENERIC, - err); + tt_sprintf(err, zAs)); pParse->is_aborted = true; return WRC_Abort; } @@ -706,9 +705,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) } else if (wrong_num_args) { const char *err = "wrong number of arguments "\ "to function %.*s()"; - err = tt_sprintf(err, nId, zId); diag_set(ClientError, ER_SQL_PARSER_GENERIC, - err); + tt_sprintf(err, nId, zId)); pParse->is_aborted = true; pNC->nErr++; } @@ -981,8 +979,7 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages err = tt_sprintf(err, i + 1, pEList->nExpr); diag_set(ClientError, - ER_SQL_PARSER_GENERIC, - err); + ER_SQL_PARSER_GENERIC, err); pParse->is_aborted = true; return 1; } @@ -1032,8 +1029,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)); pParse->is_aborted = true; return 1; } diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 1e9b7d7..1b85074 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -3624,18 +3624,13 @@ substExpr(Parse * pParse, /* Report errors here */ assert(pEList != 0 && pExpr->iColumn < pEList->nExpr); assert(pExpr->pLeft == 0 && pExpr->pRight == 0); if (sqlExprIsVector(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"; - } + 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, - err); + tt_sprintf(err, expr_count)); pParse->is_aborted = true; } else { pNew = sqlExprDup(db, pCopy, 0); diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index 9f0dee4..15a2f55 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -1208,16 +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; - 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); - } + 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 8ba9039..c71a170 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -3913,13 +3913,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst) nFrom = nTo; } - if (nFrom == 0) { - diag_set(ClientError, ER_SQL_PARSER_GENERIC, - "no query solution"); - pParse->is_aborted = true; - sqlDbFree(db, pSpace); - return SQL_ERROR; - } + assert(nFrom != 0); /* Find the lowest cost path. pFrom will be left pointing to that path */ pFrom = aFrom; @@ -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); - } #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 47430ae..9cf1eb8 100644 --- a/src/box/sql/whereInt.h +++ b/src/box/sql/whereInt.h @@ -479,7 +479,6 @@ 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 bc0964c..9d01a7e 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -1472,47 +1472,3 @@ 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++; - if (k >= (int)space_def->field_count) { - 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); - 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 3f1dd14..b43400e 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(40) +test:plan(43) test:execsql([[ CREATE TABLE t0 (i INT PRIMARY KEY, a INT); @@ -456,4 +456,34 @@ test:do_catchsql_test( -- }) +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:finish_test() New version: commit d81875339f3312bfb4de1e516ecb9cf6de77f7fc 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 b6b6c24..704d636 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..1077285 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -666,11 +666,7 @@ codeVectorCompare(Parse * pParse, /* Code generator context */ int regRight = 0; u8 op = pExpr->op; int addrDone = sqlVdbeMakeLabel(v); - - if (nLeft != sqlExprVectorSize(pRight)) { - sqlErrorMsg(pParse, "row value misused"); - return; - } + 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 +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; @@ -1785,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; } @@ -2669,41 +2674,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 +2954,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 +3968,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 +4138,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 +4161,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 +4264,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 +4366,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 ff668e0..a82b85a 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 8cdcfd0..ef40927 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; } } @@ -1073,8 +1081,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, @@ -1141,8 +1155,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; @@ -1422,12 +1441,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; } } @@ -1437,10 +1459,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 3f58f30..1b85074 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,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)) { @@ -4518,21 +4527,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 +4621,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 +4664,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 +4684,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 +4846,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 +5265,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 363ee8d..166e3fd 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 *); @@ -4427,16 +4426,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 *); @@ -4885,7 +4874,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..c71a170 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; @@ -4382,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); - } #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 47430ae..9cf1eb8 100644 --- a/src/box/sql/whereInt.h +++ b/src/box/sql/whereInt.h @@ -479,7 +479,6 @@ 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 6df28ad..9d01a7e 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -1472,45 +1472,3 @@ 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++; - if (k >= (int)space_def->field_count) { - sqlErrorMsg(pParse, - "too many arguments on %s() - max %d", - space_def->name, j); - return; - } - 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/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 ac242c4..b43400e 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(43) 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,74 @@ 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: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 = {} ---