[tarantool-patches] Re: [PATCH v4 8/8] sql: remove sqlErrorMsg()
Mergen Imeev
imeevma at tarantool.org
Mon Mar 25 21:47:37 MSK 2019
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 <imeevma at gmail.com>
> > 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(
-- </sql-errors-1.40>
})
+test:do_catchsql_test(
+ "sql-errors-1.41",
+ [[
+ SELECT (1,2,3) == (1,2,3,4);
+ ]], {
+ -- <sql-errors-1.41>
+ 1,"row value misused"
+ -- </sql-errors-1.41>
+ })
+
+test:do_catchsql_test(
+ "sql-errors-1.42",
+ [[
+ SELECT (1, 2);
+ ]], {
+ -- <sql-errors-1.42>
+ 1,"row value misused"
+ -- </sql-errors-1.42>
+ })
+
+test:do_catchsql_test(
+ "sql-errors-1.43",
+ [[
+ SELECT (i,a) AS m FROM t0 WHERE m < 1;
+ ]], {
+ -- <sql-errors-1.43>
+ 1,"row value misused"
+ -- </sql-errors-1.43>
+ })
+
test:finish_test()
New version:
commit d81875339f3312bfb4de1e516ecb9cf6de77f7fc
Author: Mergen Imeev <imeevma at gmail.com>
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;
]], {
-- <null-6.5>
- 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"
-- </null-6.5>
})
@@ -305,7 +305,7 @@ test:do_catchsql_test(
select b from t1 union select c from t1 order by t1.a;
]], {
-- <null-6.6>
- 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"
-- </null-6.6>
})
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;
]], {
-- <select1-4.10.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"
-- </select1-4.10.1>
})
@@ -850,7 +850,7 @@ test:do_catchsql_test(
SELECT * FROM t5 ORDER BY -1;
]], {
-- <select1-4.10.2>
- 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"
-- </select1-4.10.2>
})
@@ -1334,7 +1334,7 @@ test:do_catchsql2_test(
ORDER BY f2+101;
]], {
-- <select1-6.11>
- 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"
-- </select1-6.11>
})
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;
]], {
-- <select3-2.10>
- 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"
-- </select3-2.10>
})
@@ -165,7 +165,7 @@ test:do_catchsql_test("select3-2.11", [[
SELECT log, count(*) FROM t1 GROUP BY 3 ORDER BY log;
]], {
-- <select3-2.11>
- 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"
-- </select3-2.11>
})
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";
]], {
-- <select4-5.2c>
- 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"
-- </select4-5.2c>
})
@@ -463,7 +463,7 @@ test:do_catchsql_test(
ORDER BY "xyzzy";
]], {
-- <select4-5.2d>
- 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"
-- </select4-5.2d>
})
@@ -515,7 +515,7 @@ test:do_catchsql_test(
ORDER BY 2;
]], {
-- <select4-5.2h>
- 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"
-- </select4-5.2h>
})
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;
]], {
-- <sql-errors-1.7>
1,"Failed to create space 'V7': number of aliases doesn't match provided columns"
@@ -416,4 +416,74 @@ test:do_catchsql_test(
-- </sql-errors-1.36>
})
+test:do_catchsql_test(
+ "sql-errors-1.37",
+ [[
+ CREATE TRIGGER r0 AFTER INSERT ON t0 BEGIN INSERT INTO t0.i VALUES (2); END;
+ ]], {
+ -- <sql-errors-1.37>
+ 1,"qualified table names are not allowed on INSERT, UPDATE, and DELETE statements within triggers"
+ -- </sql-errors-1.37>
+ })
+
+test:do_catchsql_test(
+ "sql-errors-1.38",
+ [[
+ UPDATE t0 SET (i, a) = (100,1,1);
+ ]], {
+ -- <sql-errors-1.38>
+ 1,"2 columns assigned 3 values"
+ -- </sql-errors-1.38>
+ })
+
+test:do_catchsql_test(
+ "sql-errors-1.39",
+ [[
+ SELECT * FROM t0();
+ ]], {
+ -- <sql-errors-1.39>
+ 1,"'T0' is not a function"
+ -- </sql-errors-1.39>
+ })
+
+test:do_catchsql_test(
+ "sql-errors-1.40",
+ [[
+ SELECT $0;
+ ]], {
+ -- <sql-errors-1.40>
+ 1,"Index of binding slots must start from 1"
+ -- </sql-errors-1.40>
+ })
+
+test:do_catchsql_test(
+ "sql-errors-1.41",
+ [[
+ SELECT (1,2,3) == (1,2,3,4);
+ ]], {
+ -- <sql-errors-1.41>
+ 1,"row value misused"
+ -- </sql-errors-1.41>
+ })
+
+test:do_catchsql_test(
+ "sql-errors-1.42",
+ [[
+ SELECT (1, 2);
+ ]], {
+ -- <sql-errors-1.42>
+ 1,"row value misused"
+ -- </sql-errors-1.42>
+ })
+
+test:do_catchsql_test(
+ "sql-errors-1.43",
+ [[
+ SELECT (i,a) AS m FROM t0 WHERE m < 1;
+ ]], {
+ -- <sql-errors-1.43>
+ 1,"row value misused"
+ -- </sql-errors-1.43>
+ })
+
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
]], {
-- <tkt2822-4.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"
-- </tkt2822-4.1>
})
@@ -298,7 +298,7 @@ test:do_test(
]]
end, {
-- <tkt2822-7.1>
- 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"
-- </tkt2822-7.1>
})
@@ -308,7 +308,7 @@ test:do_catchsql_test(
SELECT * FROM t7 ORDER BY 1, 0;
]], {
-- <tkt2822-7.2.1>
- 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"
-- </tkt2822-7.2.1>
})
@@ -318,7 +318,7 @@ test:do_catchsql_test(
SELECT * FROM t7 ORDER BY 1, 26;
]], {
-- <tkt2822-7.2.2>
- 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"
-- </tkt2822-7.2.2>
})
@@ -328,7 +328,7 @@ test:do_catchsql_test(
SELECT * FROM t7 ORDER BY 1, 65536;
]], {
-- <tkt2822-7.2.3>
- 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"
-- </tkt2822-7.2.3>
})
@@ -338,7 +338,7 @@ test:do_catchsql_test(
SELECT * FROM t7 ORDER BY 1, 2, 0;
]], {
-- <tkt2822-7.3>
- 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"
-- </tkt2822-7.3>
})
@@ -348,7 +348,7 @@ test:do_catchsql_test(
SELECT * FROM t7 ORDER BY 1, 2, 3, 0;
]], {
-- <tkt2822-7.4>
- 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"
-- </tkt2822-7.4>
})
@@ -358,7 +358,7 @@ test:do_catchsql_test(
SELECT * FROM t7 ORDER BY 1, 2, 3, 4, 5, 6, 7, 8, 0;
]], {
-- <tkt2822-7.9>
- 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"
-- </tkt2822-7.9>
})
@@ -368,7 +368,7 @@ test:do_catchsql_test(
SELECT * FROM t7 ORDER BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 0;
]], {
-- <tkt2822-7.10>
- 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"
-- </tkt2822-7.10>
})
@@ -378,7 +378,7 @@ test:do_catchsql_test(
SELECT * FROM t7 ORDER BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 0;
]], {
-- <tkt2822-7.11>
- 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"
-- </tkt2822-7.11>
})
@@ -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;
]], {
-- <tkt2822-7.12>
- 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"
-- </tkt2822-7.12>
})
@@ -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;
]], {
-- <tkt2822-7.13>
- 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"
-- </tkt2822-7.13>
})
@@ -409,7 +409,7 @@ test:do_catchsql_test(
11,12,13,14,15,16,17,18,19, 0
]], {
-- <tkt2822-7.20>
- 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"
-- </tkt2822-7.20>
})
@@ -420,7 +420,7 @@ test:do_catchsql_test(
11,12,13,14,15,16,17,18,19, 20, 0
]], {
-- <tkt2822-7.21>
- 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"
-- </tkt2822-7.21>
})
@@ -431,7 +431,7 @@ test:do_catchsql_test(
11,12,13,14,15,16,17,18,19, 20, 21, 0
]], {
-- <tkt2822-7.22>
- 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"
-- </tkt2822-7.22>
})
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"
-- </3.2>
})
@@ -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"
-- </10.7.1>
})
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 = {}
---
More information about the Tarantool-patches
mailing list