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 246CF2968C for ; Fri, 22 Mar 2019 08:48:57 -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 jsxPIcuAAXWv for ; Fri, 22 Mar 2019 08:48:56 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 3E67129613 for ; Fri, 22 Mar 2019 08:48:56 -0400 (EDT) Date: Fri, 22 Mar 2019 15:48:52 +0300 From: Mergen Imeev Subject: [tarantool-patches] Re: [PATCH v4 7/8] sql: rework semantic errors Message-ID: <20190322124822.GA7890@tarantool.org> References: <6dca562ff5163f5579f45d175b6cf188db52a09f.1552494059.git.imeevma@gmail.com> <53DBABF2-8730-4FF6-9C23-64BF3F9986D1@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <53DBABF2-8730-4FF6-9C23-64BF3F9986D1@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 with fixes and new version below. On Fri, Mar 15, 2019 at 06:49:20PM +0300, n.pettik wrote: > > > diff --git a/src/box/errcode.h b/src/box/errcode.h > > index 7764aa3..a0d8f4ae 100644 > > --- a/src/box/errcode.h > > +++ b/src/box/errcode.h > > @@ -122,7 +122,7 @@ struct errcode_record { > > /* 67 */_(ER_REPLICA_ID_IS_RESERVED, "Can't initialize replica id with a reserved value %u") \ > > /* 68 */_(ER_INVALID_ORDER, "Invalid LSN order for instance %u: previous LSN = %llu, new lsn = %llu") \ > > /* 69 */_(ER_MISSING_REQUEST_FIELD, "Missing mandatory field '%s' in request") \ > > - /* 70 */_(ER_IDENTIFIER, "Invalid identifier '%s' (expected printable symbols only)") \ > > + /* 70 */_(ER_IDENTIFIER, "Invalid identifier '%s' (expected printable symbols only or it is too long)") \ > > /* 71 */_(ER_DROP_FUNCTION, "Can't drop function %u: %s") \ > > /* 72 */_(ER_ITERATOR_TYPE, "Unknown iterator type '%s'") \ > > /* 73 */_(ER_REPLICA_MAX, "Replica count limit reached: %u") \ > > @@ -240,6 +240,13 @@ struct errcode_record { > > /*185 */_(ER_SQL_UNKNOWN_TOKEN, "Syntax error: unrecognized token: '%.*s'") \ > > /*186 */_(ER_SQL_PARSER_GENERIC, "%s") \ > > /*187 */_(ER_SQL_ANALYZE_ARGUMENT, "ANALYZE statement argument %s is not a base table") \ > > + /*188 */_(ER_SQL_COLUMN_COUNT_MAX, "Failed to create space '%s': space column count %d exceeds the limit (%d)") \ > > + /*189 */_(ER_HEX_LITERAL_MAX, "Hex literal %s%s length %d exceeds the supported limit (%d)") \ > > + /*190 */_(ER_INT_LITERAL_MAX, "Integer literal %s%s exceeds the supported range %lld - %lld") \ > > + /*191 */_(ER_SQL_PARSER_LIMIT, "%s%.*s %d exceeds the limit (%d)") \ > > You use auxiliary params only in one place. I guess it is worth > removing them. > Fixed: diff --git a/src/box/errcode.h b/src/box/errcode.h index a0d8f4ae..b38bcc3 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -243,7 +243,7 @@ struct errcode_record { /*188 */_(ER_SQL_COLUMN_COUNT_MAX, "Failed to create space '%s': space column count %d exceeds the limit (%d)") \ /*189 */_(ER_HEX_LITERAL_MAX, "Hex literal %s%s length %d exceeds the supported limit (%d)") \ /*190 */_(ER_INT_LITERAL_MAX, "Integer literal %s%s exceeds the supported range %lld - %lld") \ - /*191 */_(ER_SQL_PARSER_LIMIT, "%s%.*s %d exceeds the limit (%d)") \ + /*191 */_(ER_SQL_PARSER_LIMIT, "%s %d exceeds the limit (%d)") \ /*192 */_(ER_INDEX_DEF, "%s prohibited in an index definition") \ /*193 */_(ER_CHECK_CONSTRAINT_DEF, "%s prohibited in a CHECK constraint definition") \ /*194 */_(ER_PRIMARY_KEY_DEF, "Expressions are prohibited in a primary key definition") \ diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 26434b1..c714a06 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2258,7 +2258,7 @@ sql_create_index(struct Parse *parse, struct Token *token, } else { if (col_list->nExpr > db->aLimit[SQL_LIMIT_COLUMN]) { diag_set(ClientError, ER_SQL_PARSER_LIMIT, - "The number of columns in index", 0, "", + "The number of columns in index", col_list->nExpr, db->aLimit[SQL_LIMIT_COLUMN]); parse->is_aborted = true; } diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 434b13f..f38c3c8 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -753,7 +753,7 @@ sqlExprCheckHeight(Parse * pParse, int nHeight) int mxHeight = pParse->db->aLimit[SQL_LIMIT_EXPR_DEPTH]; if (nHeight > mxHeight) { diag_set(ClientError, ER_SQL_PARSER_LIMIT, "Number of nodes "\ - "in expression tree", 0, "", nHeight, mxHeight); + "in expression tree", nHeight, mxHeight); pParse->is_aborted = true; return SQL_ERROR; } diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index ee8da3e..ff668e0 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -411,7 +411,7 @@ cmd ::= select(X). { cnt>mxSelect ){ diag_set(ClientError, ER_SQL_PARSER_LIMIT, "The number of UNION or "\ - "EXCEPT or INTERSECT operations", 0, "", cnt, + "EXCEPT or INTERSECT operations", cnt, pParse->db->aLimit[SQL_LIMIT_COMPOUND_SELECT]); pParse->is_aborted = true; } @@ -747,7 +747,7 @@ cmd ::= with(C) UPDATE orconf(R) fullname(X) indexed_opt(I) SET setlist(Y) sqlSrcListIndexedBy(pParse, X, &I); if (Y != NULL && Y->nExpr > pParse->db->aLimit[SQL_LIMIT_COLUMN]) { diag_set(ClientError, ER_SQL_PARSER_LIMIT, "The number of columns in set "\ - "list", 0, "", Y->nExpr, pParse->db->aLimit[SQL_LIMIT_COLUMN]); + "list", Y->nExpr, pParse->db->aLimit[SQL_LIMIT_COLUMN]); pParse->is_aborted = true; } sqlSubProgramsRemaining = SQL_MAX_COMPILING_TRIGGERS; @@ -937,8 +937,9 @@ expr(A) ::= CAST(X) LP expr(E) AS typedef(T) RP(Y). { %endif SQL_OMIT_CAST expr(A) ::= id(X) LP distinct(D) exprlist(Y) RP(E). { if( Y && Y->nExpr>pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG] ){ - diag_set(ClientError, ER_SQL_PARSER_LIMIT, "Number of "\ - "arguments to function ", X.n, X.z, Y->nExpr, + const char *err = + tt_sprintf("Number of arguments to function %.*s", X.n, X.z); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, err, Y->nExpr, pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG]); pParse->is_aborted = true; } @@ -956,8 +957,9 @@ expr(A) ::= id(X) LP distinct(D) exprlist(Y) RP(E). { type_func(A) ::= CHAR(A) . expr(A) ::= type_func(X) LP distinct(D) exprlist(Y) RP(E). { if( Y && Y->nExpr>pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG] ){ - diag_set(ClientError, ER_SQL_PARSER_LIMIT, "Number of "\ - "arguments to function ", X.n, X.z, Y->nExpr, + const char *err = + tt_sprintf("Number of arguments to function %.*s", X.n, X.z); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, err, Y->nExpr, pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG]); pParse->is_aborted = true; } diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 9171d05..c1af4e6 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -952,7 +952,7 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages #if SQL_MAX_COLUMN if (pOrderBy->nExpr > db->aLimit[SQL_LIMIT_COLUMN]) { diag_set(ClientError, ER_SQL_PARSER_LIMIT, - "The number of terms in ORDER BY clause", 0, "", + "The number of terms in ORDER BY clause", pOrderBy->nExpr, db->aLimit[SQL_LIMIT_COLUMN]); pParse->is_aborted = true; return 1; @@ -1069,7 +1069,7 @@ sqlResolveOrderGroupBy(Parse *pParse, Select *pSelect, ExprList *pOrderBy, const char *err_msg = is_order_by ? "The number of terms in ORDER BY clause" : "The number of terms in GROUP BY clause"; - diag_set(ClientError, ER_SQL_PARSER_LIMIT, err_msg, 0, "", + diag_set(ClientError, ER_SQL_PARSER_LIMIT, err_msg, pOrderBy->nExpr, db->aLimit[SQL_LIMIT_COLUMN]); pParse->is_aborted = true; return 1; diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 27a17a4..3f58f30 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -5063,7 +5063,7 @@ selectExpander(Walker * pWalker, Select * p) #if SQL_MAX_COLUMN if (p->pEList && p->pEList->nExpr > db->aLimit[SQL_LIMIT_COLUMN]) { diag_set(ClientError, ER_SQL_PARSER_LIMIT, "The number of "\ - "columns in result set", 0, "", p->pEList->nExpr, + "columns in result set", p->pEList->nExpr, db->aLimit[SQL_LIMIT_COLUMN]); pParse->is_aborted = true; return WRC_Abort; diff --git a/src/box/sql/where.c b/src/box/sql/where.c index 0de3cf0..2f83e81 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -4287,7 +4287,7 @@ sqlWhereBegin(Parse * pParse, /* The parser context */ testcase(pTabList->nSrc == BMS); if (pTabList->nSrc > BMS) { diag_set(ClientError, ER_SQL_PARSER_LIMIT, "The number of "\ - "tables in a join", 0, "", pTabList->nSrc, BMS); + "tables in a join", pTabList->nSrc, BMS); pParse->is_aborted = true; return 0; } > > + /*192 */_(ER_INDEX_DEF, "%s prohibited in an index definition") \ > > Why not ER_UNSUPPORTED? > ER_INDEX_DEF -> ER_INDEX_DEF_UNSUPPORTED > > Also, quite unnatural message pattern: it is hard to > understand diags like: > > diag_set(ClientError, ER_INDEX_DEF, > "Parameter markers are"); > Fixed, diff below. > > + /*193 */_(ER_CHECK_CONSTRAINT_DEF, "%s prohibited in a CHECK constraint definition") \ > > The same question. > ER_CHECK_CONSTRAINT_DEF -> ER_CK_DEF_UNSUPPORTED > > > + /*194 */_(ER_PRIMARY_KEY_DEF, "Expressions are prohibited in a primary key definition") \ > > Just substitute it with ER_INDEX_DEF_UNSUPPORTED: > expressions are prohibited in secondary index definition as well. > Fixed: diff --git a/src/box/errcode.h b/src/box/errcode.h index b38bcc3..3f8cb8e 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -244,9 +244,8 @@ struct errcode_record { /*189 */_(ER_HEX_LITERAL_MAX, "Hex literal %s%s length %d exceeds the supported limit (%d)") \ /*190 */_(ER_INT_LITERAL_MAX, "Integer literal %s%s exceeds the supported range %lld - %lld") \ /*191 */_(ER_SQL_PARSER_LIMIT, "%s %d exceeds the limit (%d)") \ - /*192 */_(ER_INDEX_DEF, "%s prohibited in an index definition") \ - /*193 */_(ER_CHECK_CONSTRAINT_DEF, "%s prohibited in a CHECK constraint definition") \ - /*194 */_(ER_PRIMARY_KEY_DEF, "Expressions are prohibited in a primary key definition") \ + /*192 */_(ER_INDEX_DEF_UNSUPPORTED, "%s are prohibited in an index definition") \ + /*193 */_(ER_CK_DEF_UNSUPPORTED, "%s are prohibited in a CHECK constraint definition") \ /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/sql/build.c b/src/box/sql/build.c index c714a06..7d5f3ed 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -605,7 +605,8 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */ sqlExprSkipCollate(pList->a[i].pExpr); assert(pCExpr != 0); if (pCExpr->op != TK_ID) { - diag_set(ClientError, ER_PRIMARY_KEY_DEF); + diag_set(ClientError, ER_INDEX_DEF_UNSUPPORTED, + "Expressions"); pParse->is_aborted = true; goto primary_key_exit; } diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index c1af4e6..9d5c22b 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -582,7 +582,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) /* if( pSrcList==0 ) break; */ if (pNC->ncFlags & NC_IdxExpr) { - diag_set(ClientError, ER_INDEX_DEF, "'.' operator is"); + diag_set(ClientError, ER_INDEX_DEF_UNSUPPORTED, + "Expressions"); pParse->is_aborted = true; } pRight = pExpr->pRight; @@ -675,9 +676,10 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) * in an index. */ if (pNC->ncFlags & NC_IdxExpr) { - diag_set(ClientError, ER_INDEX_DEF, - "Non-deterministic functions "\ - "are"); + diag_set(ClientError, + ER_INDEX_DEF_UNSUPPORTED, + "Non-deterministic "\ + "functions"); pParse->is_aborted = true; } } @@ -742,13 +744,14 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) if (ExprHasProperty(pExpr, EP_xIsSelect)) { int nRef = pNC->nRef; if (pNC->ncFlags & NC_IdxExpr) { - diag_set(ClientError, ER_INDEX_DEF, - "Subqueries are"); + diag_set(ClientError, + ER_INDEX_DEF_UNSUPPORTED, + "Subqueries"); pParse->is_aborted = true; } else if (pNC->ncFlags & NC_IsCheck) { diag_set(ClientError, - ER_CHECK_CONSTRAINT_DEF, - "Subqueries are"); + ER_CK_DEF_UNSUPPORTED, + "Subqueries"); pParse->is_aborted = true; } sqlWalkSelect(pWalker, pExpr->x.pSelect); @@ -763,8 +766,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) case TK_VARIABLE:{ assert((pNC->ncFlags & NC_IsCheck) == 0); if (pNC->ncFlags & NC_IdxExpr) { - diag_set(ClientError, ER_INDEX_DEF, - "Parameter markers are"); + diag_set(ClientError, ER_INDEX_DEF_UNSUPPORTED, + "Parameter markers"); pParse->is_aborted = true; } break; diff --git a/test/box/misc.result b/test/box/misc.result index 871bfa1..5dda752 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -520,9 +520,8 @@ t; 189: box.error.HEX_LITERAL_MAX 190: box.error.INT_LITERAL_MAX 191: box.error.SQL_PARSER_LIMIT - 192: box.error.INDEX_DEF - 193: box.error.CHECK_CONSTRAINT_DEF - 194: box.error.PRIMARY_KEY_DEF + 192: box.error.INDEX_DEF_UNSUPPORTED + 193: box.error.CK_DEF_UNSUPPORTED ... test_run:cmd("setopt delimiter ''"); --- diff --git a/test/sql-tap/colname.test.lua b/test/sql-tap/colname.test.lua index 77c4280..253497c 100755 --- a/test/sql-tap/colname.test.lua +++ b/test/sql-tap/colname.test.lua @@ -637,7 +637,7 @@ test:do_test( test:do_catchsql_test( "colname-11.1", [[ create table t1(a INT, b INT, c INT, primary key('A'))]], - {1, "Expressions are prohibited in a primary key definition"}) + {1, "Expressions are prohibited in an index definition"}) test:do_catchsql_test( "colname-11.2", diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua index 4a92af6..43f14cc 100755 --- a/test/sql-tap/sql-errors.test.lua +++ b/test/sql-tap/sql-errors.test.lua @@ -303,7 +303,7 @@ test:do_catchsql_test( CREATE TABLE t26 (i INT, PRIMARY KEY('i')); ]], { -- - 1,"Expressions are prohibited in a primary key definition" + 1,"Expressions are prohibited in an index definition" -- }) @@ -323,7 +323,7 @@ test:do_catchsql_test( CREATE INDEX i28 ON t0(t0.i); ]], { -- - 1,"'.' operator is prohibited in an index definition" + 1,"Expressions are prohibited in an index definition" -- }) > > > > /* > > * !IMPORTANT! Please follow instructions at start of the file > > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > > index 0c06555..26434b1 100644 > > --- a/src/box/sql/build.c > > +++ b/src/box/sql/build.c > > @@ -267,14 +267,14 @@ int > > sqlCheckIdentifierName(Parse *pParse, char *zName) > > { > > ssize_t len = strlen(zName); > > - > > - if (len > BOX_NAME_MAX || identifier_check(zName, len) != 0) { > > - sqlErrorMsg(pParse, > > - "identifier name is invalid: %s", > > - zName); > > - return SQL_ERROR; > > + if (len <= BOX_NAME_MAX && identifier_check(zName, len) == 0) > > + return SQL_OK; > > return 0; > Fixed, diff below. > > + if (len > BOX_NAME_MAX) { > > + diag_set(ClientError, ER_IDENTIFIER, > > + tt_cstr(zName, BOX_INVALID_NAME_MAX)); > > } > > - return SQL_OK; > > + pParse->is_aborted = true; > > + return SQL_ERROR; > > return -1; > > ssize_t len = strlen(zName); > - if (len <= BOX_NAME_MAX && identifier_check(zName, len) == 0) > - return SQL_OK; > if (len > BOX_NAME_MAX) { > diag_set(ClientError, ER_IDENTIFIER, > tt_cstr(zName, BOX_INVALID_NAME_MAX)); > + pParse->is_aborted = true; > + return -1; > } > - pParse->is_aborted = true; > - return SQL_ERROR; > + if (identifier_check(zName, len) != 0) { > + pParse->is_aborted = true; > + return -1; > + } > + return 0; > Fixed: diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 05d39af..0022254 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -262,19 +262,28 @@ sqlNameFromToken(sql * db, Token * pName) * (e.g. table, index, column name of a real table) * All names are legal except those that cantain non-printable * characters or have length greater than BOX_NAME_MAX. + * + * @param pParse Parser context. + * @param zName Identifier to check. + * + * @retval 0 on success. + * @retval -1 on error. */ int sqlCheckIdentifierName(Parse *pParse, char *zName) { ssize_t len = strlen(zName); - if (len <= BOX_NAME_MAX && identifier_check(zName, len) == 0) - return SQL_OK; if (len > BOX_NAME_MAX) { diag_set(ClientError, ER_IDENTIFIER, tt_cstr(zName, BOX_INVALID_NAME_MAX)); + pParse->is_aborted = true; + return -1; } - pParse->is_aborted = true; - return SQL_ERROR; + if (identifier_check(zName, len) != 0) { + pParse->is_aborted = true; + return -1; + } + return 0; } /** @@ -334,7 +343,7 @@ sqlStartTable(Parse *pParse, Token *pName, int noErr) pParse->sNameToken = *pName; if (zName == 0) return; - if (sqlCheckIdentifierName(pParse, zName) != SQL_OK) + if (sqlCheckIdentifierName(pParse, zName) != 0) goto cleanup; struct space *space = space_by_name(zName); @@ -2962,7 +2971,7 @@ sqlSavepoint(Parse * pParse, int op, Token * pName) return; } if (op == SAVEPOINT_BEGIN && - sqlCheckIdentifierName(pParse, zName) != SQL_OK) + sqlCheckIdentifierName(pParse, zName) != 0) return; sqlVdbeAddOp4(v, OP_Savepoint, op, 0, 0, zName, P4_DYNAMIC); } diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 7eacd33..b23d60a 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -86,7 +86,7 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm, if (trigger_name == NULL) goto trigger_cleanup; - if (sqlCheckIdentifierName(parse, trigger_name) != SQL_OK) + if (sqlCheckIdentifierName(parse, trigger_name) != 0) goto trigger_cleanup; const char *table_name = table->a[0].zName; > > @@ -588,9 +590,9 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */ > > if (space == NULL) > > goto primary_key_exit; > > if (sql_space_primary_key(space) != NULL) { > > - sqlErrorMsg(pParse, > > - "table \"%s\" has more than one primary key", > > - space->def->name); > > + diag_set(ClientError, ER_CREATE_SPACE, space->def->name, > > + "too many primary keys”); > > too many primary keys -> primary key already exists > Fixed, diff below (the same diff which replaces "it is a view" by "space is a view"). > > + pParse->is_aborted = true; > > goto primary_key_exit; > > } > > if (pList == NULL) { > > > > @@ -1775,6 +1778,11 @@ sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child, > > } > > if (constraint_name == NULL) > > goto exit_create_fk; > > + if (!is_self_referenced && parent_space->def->opts.is_view) { > > + diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, constraint_name, > > + "referenced space can't be VIEW"); > > + goto tnt_error; > > + } > > This check is already processed in on_replace_dd_fk_constraint() > Could you remove this check to avoid code duplication? > It can't be done here because this function wotk with parent PK and a view do not have PK. I think it is possible to remove it in alter.cc, but not sure that it would be right. > > @@ -2942,11 +2958,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 > Fixed, diff above. > > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c > > index 87d4ed4..e8a5f0d 100644 > > --- a/src/box/sql/delete.c > > +++ b/src/box/sql/delete.c > > @@ -157,8 +157,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, > > goto delete_from_cleanup; > > > > if (trigger_list == NULL) { > > - sqlErrorMsg(parse, "cannot modify %s because it is a" > > - " view", space->def->name); > > + diag_set(ClientError, ER_ALTER_SPACE, space->def->name, > > + "it is a view”); > > Nit: it is a view -> space is a view (sounds better IMHO). > Fixed, diff below. > > + parse->is_aborted = true; > > goto delete_from_cleanup; > > } > > } > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > > index 4193596..de06ee0 100644 > > --- a/src/box/sql/expr.c > > +++ b/src/box/sql/expr.c > > @@ -750,15 +750,14 @@ codeVectorCompare(Parse * pParse, /* Code generator context */ > > int > > sqlExprCheckHeight(Parse * pParse, int nHeight) > > { > > - int rc = SQL_OK; > > int mxHeight = pParse->db->aLimit[SQL_LIMIT_EXPR_DEPTH]; > > if (nHeight > mxHeight) { > > - sqlErrorMsg(pParse, > > - "Expression tree is too large (maximum depth %d)", > > - mxHeight); > > - rc = SQL_ERROR; > > + diag_set(ClientError, ER_SQL_PARSER_LIMIT, "Number of nodes "\ > > + "in expression tree", 0, "", nHeight, mxHeight); > > + pParse->is_aborted = true; > > + return SQL_ERROR; > > return -1; > > > } Fixed, diff below. > > - return rc; > > + return SQL_OK; > > return 0; > Fixed: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index f38c3c8..de993c1 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -746,6 +746,12 @@ codeVectorCompare(Parse * pParse, /* Code generator context */ * Check that argument nHeight is less than or equal to the maximum * expression depth allowed. If it is not, leave an error message in * pParse. + * + * @param pParse Parser context. + * @param zName Depth to check. + * + * @retval 0 on success. + * @retval -1 on error. */ int sqlExprCheckHeight(Parse * pParse, int nHeight) @@ -755,9 +761,9 @@ sqlExprCheckHeight(Parse * pParse, int nHeight) diag_set(ClientError, ER_SQL_PARSER_LIMIT, "Number of nodes "\ "in expression tree", nHeight, mxHeight); pParse->is_aborted = true; - return SQL_ERROR; + return -1; } - return SQL_OK; + return 0; } /* The following three functions, heightOfExpr(), heightOfExprList() > > > > @@ -690,9 +674,12 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) > > * that might change over time cannot be used > > * in an index. > > */ > > - notValid(pParse, pNC, > > - "non-deterministic functions", > > - NC_IdxExpr); > > + if (pNC->ncFlags & NC_IdxExpr) { > > + diag_set(ClientError, ER_INDEX_DEF, > > + "Non-deterministic functions "\ > > + "are”); > > This code seems to be unreachable: functional indexes > are filtered a way earlier. > Fixed, diff below. > > @@ -754,8 +741,16 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) > > testcase(pExpr->op == TK_IN); > > if (ExprHasProperty(pExpr, EP_xIsSelect)) { > > int nRef = pNC->nRef; > > - notValid(pParse, pNC, "subqueries", > > - NC_IsCheck | NC_IdxExpr); > > + if (pNC->ncFlags & NC_IdxExpr) { > > + diag_set(ClientError, ER_INDEX_DEF, > > + "Subqueries are”); > > The same: managed to grep this error only for ck constraint. > Fixed: > > @@ -1042,27 +1044,34 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages > > * field) then convert that term into a copy of the corresponding result set > > * column. > > * > > - * If any errors are detected, add an error message to pParse and > > - * return non-zero. Return zero if no errors are seen. > > + * @param pParse Parsing context. > > + * @param pSelect The SELECT statement containing the clause. > > + * @param pOrderBy The ORDER BY or GROUP BY clause to be > > + * processed. > > + * @param is_order_by True if pOrderBy is ORDER BY, false if > > + * pOrderBy is GROUP BY > > + * @retval 0 On success, not 0 elsewhere. > > */ > > int > > -sqlResolveOrderGroupBy(Parse * pParse, /* Parsing context. Leave error messages here */ > > - Select * pSelect, /* The SELECT statement containing the clause */ > > - ExprList * pOrderBy, /* The ORDER BY or GROUP BY clause to be processed */ > > - const char *zType /* "ORDER" or "GROUP" */ > > - ) > > +sqlResolveOrderGroupBy(Parse *pParse, Select *pSelect, ExprList *pOrderBy, > > + bool is_order_by) > > Why did you decide to fix code style here? Anyway, you didn't finished it > (struct prefixes, param naming and so on and so forth) > I fixed this because I thought that it is quite unreliable to differentiate names of term using first letter of its name. Should I remove these changes? > > { > > int i; > > sql *db = pParse->db; > > ExprList *pEList; > > struct ExprList_item *pItem; > > + const char *zType = is_order_by ? "ORDER" : "GROUP"; > > > > if (pOrderBy == 0 || pParse->db->mallocFailed) > > return 0; > > > > @@ -1096,22 +1105,23 @@ sqlResolveOrderGroupBy(Parse * pParse, /* Parsing context. Leave error messages > > * result-set expression. Otherwise, the expression is resolved in > > * the usual way - using sqlResolveExprNames(). > > * > > - * This routine returns the number of errors. If errors occur, then > > - * an appropriate error message might be left in pParse. (OOM errors > > - * excepted.) > > + * @param pNC The name context of the SELECT statement. > > + * @param pSelect The SELECT statement containing the clause. > > + * @param pOrderBy An ORDER BY or GROUP BY clause to resolve. > > + * @param is_order_by True if pOrderBy is ORDER BY, false if > > + * pOrderBy is GROUP BY > > + * @retval 0 On success, not 0 elsewhere. > > */ > > static int > > -resolveOrderGroupBy(NameContext * pNC, /* The name context of the SELECT statement */ > > - Select * pSelect, /* The SELECT statement holding pOrderBy */ > > - ExprList * pOrderBy, /* An ORDER BY or GROUP BY clause to resolve */ > > - const char *zType /* Either "ORDER" or "GROUP", as appropriate */ > > - ) > > The same question. > Answer above. > > +resolveOrderGroupBy(NameContext *pNC, Select *pSelect, ExprList *pOrderBy, > > + bool is_order_by) > > { > > int i, j; /* Loop counters */ > > int iCol; /* Column number */ > > struct ExprList_item *pItem; /* A term of the ORDER BY clause */ > > Parse *pParse; /* Parsing context */ > > int nResult; /* Number of terms in the result set */ > > + const char *zType = is_order_by ? "ORDER" : "GROUP"; > > > > if (pOrderBy == 0) > > return 0; > > > > /* Compute the limit registers */ > > computeLimitRegisters(pParse, p, labelEnd); > > diff --git a/src/box/sql/update.c b/src/box/sql/update.c > > index 05ceeb4..bc0ab66 100644 > > --- a/src/box/sql/update.c > > +++ b/src/box/sql/update.c > > @@ -139,8 +139,9 @@ sqlUpdate(Parse * pParse, /* The parser context */ > > goto update_cleanup; > > } > > if (is_view && tmask == 0) { > > - sqlErrorMsg(pParse, "cannot modify %s because it is a view", > > - space->def->name); > > + diag_set(ClientError, ER_ALTER_SPACE, space->def->name, > > + "it is a view”); > > -> space is a view > Fixed: diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 0022254..b6b6c24 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -603,7 +603,7 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */ goto primary_key_exit; if (sql_space_primary_key(space) != NULL) { diag_set(ClientError, ER_CREATE_SPACE, space->def->name, - "too many primary keys"); + "primary key already exists"); pParse->is_aborted = true; goto primary_key_exit; } diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index dbaf2c1..0eb28d7 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -110,8 +110,8 @@ sql_table_truncate(struct Parse *parse, struct SrcList *tab_list) } if (space->def->opts.is_view) { const char *err_msg = - tt_sprintf("can not truncate space '%s' because it is " - "a view", space->def->name); + tt_sprintf("can not truncate space '%s' because space "\ + "is a view", space->def->name); diag_set(ClientError, ER_SQL, err_msg); goto tarantool_error; } @@ -164,7 +164,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, if (trigger_list == NULL) { diag_set(ClientError, ER_ALTER_SPACE, space->def->name, - "it is a view"); + "space is a view"); parse->is_aborted = true; goto delete_from_cleanup; } diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 263df4d..0ca38ae 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -323,7 +323,7 @@ sqlInsert(Parse * pParse, /* Parser context */ /* Cannot insert into a read-only table. */ if (is_view && tmask == 0) { diag_set(ClientError, ER_ALTER_SPACE, space->def->name, - "it is a view"); + "space is a view"); pParse->is_aborted = true; goto insert_cleanup; } diff --git a/src/box/sql/update.c b/src/box/sql/update.c index bc0ab66..71a1e00 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -140,7 +140,7 @@ sqlUpdate(Parse * pParse, /* The parser context */ } if (is_view && tmask == 0) { diag_set(ClientError, ER_ALTER_SPACE, space->def->name, - "it is a view"); + "space is a view"); pParse->is_aborted = true; goto update_cleanup; } diff --git a/test/sql-tap/misc1.test.lua b/test/sql-tap/misc1.test.lua index ffef2d4..6b9f5fa 100755 --- a/test/sql-tap/misc1.test.lua +++ b/test/sql-tap/misc1.test.lua @@ -348,7 +348,7 @@ test:do_catchsql_test( ); ]], { -- - 1, [[Failed to create space 'ERROR1': too many primary keys]] + 1, [[Failed to create space 'ERROR1': primary key already exists]] -- }) @@ -361,7 +361,7 @@ test:do_catchsql_test( ); ]], { -- - 1, [[Failed to create space 'ERROR1': too many primary keys]] + 1, [[Failed to create space 'ERROR1': primary key already exists]] -- }) @@ -897,7 +897,7 @@ test:do_catchsql_test( CREATE TABLE test2(a text primary key, b text, primary key(a,b)); ]], { -- - 1, [[Failed to create space 'TEST2': too many primary keys]] + 1, [[Failed to create space 'TEST2': primary key already exists]] -- }) diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua index cd20310..ac242c4 100755 --- a/test/sql-tap/sql-errors.test.lua +++ b/test/sql-tap/sql-errors.test.lua @@ -52,7 +52,7 @@ test:do_catchsql_test( CREATE TABLE t4 (i INT PRIMARY KEY, a INT PRIMARY KEY); ]], { -- - 1,"Failed to create space 'T4': too many primary keys" + 1,"Failed to create space 'T4': primary key already exists" -- }) @@ -273,7 +273,7 @@ test:do_catchsql_test( INSERT INTO v0 VALUES (2); ]], { -- - 1,"Can't modify space 'V0': it is a view" + 1,"Can't modify space 'V0': space is a view" -- }) @@ -283,7 +283,7 @@ test:do_catchsql_test( UPDATE v0 SET i = 2 WHERE i = 1; ]], { -- - 1,"Can't modify space 'V0': it is a view" + 1,"Can't modify space 'V0': space is a view" -- }) @@ -293,7 +293,7 @@ test:do_catchsql_test( DELETE FROM v0; ]], { -- - 1,"Can't modify space 'V0': it is a view" + 1,"Can't modify space 'V0': space is a view" -- }) diff --git a/test/sql-tap/unique.test.lua b/test/sql-tap/unique.test.lua index 79bede8..a3fa1c9 100755 --- a/test/sql-tap/unique.test.lua +++ b/test/sql-tap/unique.test.lua @@ -36,7 +36,7 @@ test:do_catchsql_test( ); ]], { -- - 1, [[Failed to create space 'T1': too many primary keys]] + 1, [[Failed to create space 'T1': primary key already exists]] -- }) diff --git a/test/sql-tap/view.test.lua b/test/sql-tap/view.test.lua index e1affb6..0032e1b 100755 --- a/test/sql-tap/view.test.lua +++ b/test/sql-tap/view.test.lua @@ -189,7 +189,7 @@ test:do_catchsql_test( INSERT INTO v2 VALUES(1,2,3,4); ]], { -- - 1, "Can't modify space 'V2': it is a view" + 1, "Can't modify space 'V2': space is a view" -- }) @@ -199,7 +199,7 @@ test:do_catchsql_test( UPDATE v2 SET a=10 WHERE a=5; ]], { -- - 1, "Can't modify space 'V2': it is a view" + 1, "Can't modify space 'V2': space is a view" -- }) @@ -209,7 +209,7 @@ test:do_catchsql_test( DELETE FROM v2; ]], { -- - 1, "Can't modify space 'V2': it is a view" + 1, "Can't modify space 'V2': space is a view" -- }) diff --git a/test/sql/delete.result b/test/sql/delete.result index e024dd6..29459cc 100644 --- a/test/sql/delete.result +++ b/test/sql/delete.result @@ -93,7 +93,7 @@ box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;") ... box.sql.execute("TRUNCATE TABLE v1;") --- -- error: 'SQL error: can not truncate space ''V1'' because it is a view' +- error: 'SQL error: can not truncate space ''V1'' because space is a view' ... -- Can't truncate table with FK. box.sql.execute("CREATE TABLE t2(x INT PRIMARY KEY REFERENCES t1(id));") > > + pParse->is_aborted = true; > > goto update_cleanup; > > } > > > > > > diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua > > index 0d8bf15..dd2de92 100755 > > --- a/test/sql-tap/check.test.lua > > +++ b/test/sql-tap/check.test.lua > > @@ -319,7 +319,7 @@ test:do_catchsql_test( > > ); > > ]], { > > -- > > - 1, "Failed to create space 'T3': subqueries prohibited in CHECK constraints" > > + 1, "Failed to create space 'T3': Subqueries are prohibited in a CHECK constraint definition” > > Don’t use capital letter after ‘:’, it would allow you to avoid fixing tests. > The same is applied to the fixes below. > It can't be done because this error consist of two different errors: ER_CREATE_SPACE and ER_CK_DEF_UNSUPPORTED. The first one uses message of the second error. It is done this way in alter.cc. > > -- > > }) > > > > diff --git a/test/sql-tap/colname.test.lua b/test/sql-tap/colname.test.lua > > index 29fdf13..77c4280 100755 > > --- a/test/sql-tap/colname.test.lua > > +++ b/test/sql-tap/colname.test.lua > > @@ -637,7 +637,7 @@ test:do_test( > > test:do_catchsql_test( > > "colname-11.1", > > [[ create table t1(a INT, b INT, c INT, primary key('A'))]], > > - {1, "expressions prohibited in PRIMARY KEY"}) > > + {1, "Expressions are prohibited in a primary key definition"}) > > > > test:do_catchsql_test( > > "colname-11.2", > > diff --git a/test/sql-tap/default.test.lua b/test/sql-tap/default.test.lua > > index 67151b0..fbe285b 100755 > > --- a/test/sql-tap/default.test.lua > > +++ b/test/sql-tap/default.test.lua > > @@ -66,7 +66,7 @@ test:do_catchsql_test( > > ); > > ]], { > > -- > > - 1, "default value of column [Y] is not constant" > > + 1, "Failed to create space 'T3': default value of column is not constant” > > Is it possible to print also name of column (like in original version)? > Fixed: diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 7d5f3ed..05d39af 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -523,8 +523,11 @@ sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan) struct space_def *def = pParse->new_space->def; if (!sqlExprIsConstantOrFunction (pSpan->pExpr, db->init.busy)) { + const char *column_name = + def->fields[def->field_count - 1].name; diag_set(ClientError, ER_CREATE_SPACE, def->name, - "default value of column is not constant"); + tt_sprintf("default value of column '%s' is "\ + "not constant", column_name)); pParse->is_aborted = true; } else { assert(def != NULL); diff --git a/test/sql-tap/default.test.lua b/test/sql-tap/default.test.lua index fbe285b..564d373 100755 --- a/test/sql-tap/default.test.lua +++ b/test/sql-tap/default.test.lua @@ -66,7 +66,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "Failed to create space 'T3': default value of column is not constant" + 1, "Failed to create space 'T3': default value of column 'Y' is not constant" -- }) @@ -173,7 +173,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "Failed to create space 'T2': default value of column is not constant" + 1, "Failed to create space 'T2': default value of column 'B' is not constant" -- }) @@ -187,7 +187,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "Failed to create space 'T2': default value of column is not constant" + 1, "Failed to create space 'T2': default value of column 'B' is not constant" -- }) @@ -201,7 +201,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "Failed to create space 'T2': default value of column is not constant" + 1, "Failed to create space 'T2': default value of column 'B' is not constant" -- }) @@ -214,7 +214,7 @@ test:do_catchsql_test( CREATE TABLE t6(id INTEGER PRIMARY KEY, b TEXT DEFAULT(id)); ]], { -- - 1, "Failed to create space 'T6': default value of column is not constant" + 1, "Failed to create space 'T6': default value of column 'B' is not constant" -- }) diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua index 43f14cc..cd20310 100755 --- a/test/sql-tap/sql-errors.test.lua +++ b/test/sql-tap/sql-errors.test.lua @@ -42,7 +42,7 @@ test:do_catchsql_test( CREATE TABLE t3 (i INT PRIMARY KEY, a INT DEFAULT(MAX(i, 1))); ]], { -- - 1,"Failed to create space 'T3': default value of column is not constant" + 1,"Failed to create space 'T3': default value of column 'A' is not constant" -- }) > > diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg > > index 413dd3f..cc56592 100644 > > --- a/test/sql-tap/engine.cfg > > +++ b/test/sql-tap/engine.cfg > > @@ -11,6 +11,9 @@ > > "sort.test.lua": { > > "memtx": {"engine": "memtx"} > > }, > > + "sql-errors.test.lua": { > > + "memtx": {"engine": "memtx"} > > + }, > > I would run this test both for vinyl and memtx. > Fixed: diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg index cc56592..413dd3f 100644 --- a/test/sql-tap/engine.cfg +++ b/test/sql-tap/engine.cfg @@ -11,9 +11,6 @@ "sort.test.lua": { "memtx": {"engine": "memtx"} }, - "sql-errors.test.lua": { - "memtx": {"engine": "memtx"} - }, "*": { "memtx": {"engine": "memtx"}, "vinyl": {"engine": "vinyl"} Also, I changed one more error: diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c index de0f282..efb895f 100644 --- a/src/box/sql/tokenize.c +++ b/src/box/sql/tokenize.c @@ -481,8 +481,8 @@ sqlRunParser(Parse * pParse, const char *zSql) &pParse->sLastToken.isReserved); i += pParse->sLastToken.n; if (i > mxSqlLen) { - diag_set(ClientError, ER_SQL_PARSER_GENERIC, - "string or blob too big"); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, + "SQL command length", i, mxSqlLen); pParse->is_aborted = true; break; } I will investigate this error a bit later. New version: commit 39c071b39c0ad1d79c3d117fb69303ffcc7f5cd8 Author: Mergen Imeev Date: Thu Mar 7 21:28:06 2019 +0300 sql: rework semantic errors This patch reworks some of SQL semantic errors. Part of #3965 diff --git a/src/box/errcode.h b/src/box/errcode.h index 7764aa3..3f8cb8e 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -122,7 +122,7 @@ struct errcode_record { /* 67 */_(ER_REPLICA_ID_IS_RESERVED, "Can't initialize replica id with a reserved value %u") \ /* 68 */_(ER_INVALID_ORDER, "Invalid LSN order for instance %u: previous LSN = %llu, new lsn = %llu") \ /* 69 */_(ER_MISSING_REQUEST_FIELD, "Missing mandatory field '%s' in request") \ - /* 70 */_(ER_IDENTIFIER, "Invalid identifier '%s' (expected printable symbols only)") \ + /* 70 */_(ER_IDENTIFIER, "Invalid identifier '%s' (expected printable symbols only or it is too long)") \ /* 71 */_(ER_DROP_FUNCTION, "Can't drop function %u: %s") \ /* 72 */_(ER_ITERATOR_TYPE, "Unknown iterator type '%s'") \ /* 73 */_(ER_REPLICA_MAX, "Replica count limit reached: %u") \ @@ -240,6 +240,12 @@ struct errcode_record { /*185 */_(ER_SQL_UNKNOWN_TOKEN, "Syntax error: unrecognized token: '%.*s'") \ /*186 */_(ER_SQL_PARSER_GENERIC, "%s") \ /*187 */_(ER_SQL_ANALYZE_ARGUMENT, "ANALYZE statement argument %s is not a base table") \ + /*188 */_(ER_SQL_COLUMN_COUNT_MAX, "Failed to create space '%s': space column count %d exceeds the limit (%d)") \ + /*189 */_(ER_HEX_LITERAL_MAX, "Hex literal %s%s length %d exceeds the supported limit (%d)") \ + /*190 */_(ER_INT_LITERAL_MAX, "Integer literal %s%s exceeds the supported range %lld - %lld") \ + /*191 */_(ER_SQL_PARSER_LIMIT, "%s %d exceeds the limit (%d)") \ + /*192 */_(ER_INDEX_DEF_UNSUPPORTED, "%s are prohibited in an index definition") \ + /*193 */_(ER_CK_DEF_UNSUPPORTED, "%s are prohibited in a CHECK constraint definition") \ /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 0c06555..b6b6c24 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -262,19 +262,28 @@ sqlNameFromToken(sql * db, Token * pName) * (e.g. table, index, column name of a real table) * All names are legal except those that cantain non-printable * characters or have length greater than BOX_NAME_MAX. + * + * @param pParse Parser context. + * @param zName Identifier to check. + * + * @retval 0 on success. + * @retval -1 on error. */ int sqlCheckIdentifierName(Parse *pParse, char *zName) { ssize_t len = strlen(zName); - - if (len > BOX_NAME_MAX || identifier_check(zName, len) != 0) { - sqlErrorMsg(pParse, - "identifier name is invalid: %s", - zName); - return SQL_ERROR; + if (len > BOX_NAME_MAX) { + diag_set(ClientError, ER_IDENTIFIER, + tt_cstr(zName, BOX_INVALID_NAME_MAX)); + pParse->is_aborted = true; + return -1; + } + if (identifier_check(zName, len) != 0) { + pParse->is_aborted = true; + return -1; } - return SQL_OK; + return 0; } /** @@ -334,7 +343,7 @@ sqlStartTable(Parse *pParse, Token *pName, int noErr) pParse->sNameToken = *pName; if (zName == 0) return; - if (sqlCheckIdentifierName(pParse, zName) != SQL_OK) + if (sqlCheckIdentifierName(pParse, zName) != 0) goto cleanup; struct space *space = space_by_name(zName); @@ -432,7 +441,9 @@ sqlAddColumn(Parse * pParse, Token * pName, struct type_def *type_def) #if SQL_MAX_COLUMN if ((int)def->field_count + 1 > db->aLimit[SQL_LIMIT_COLUMN]) { - sqlErrorMsg(pParse, "too many columns on %s", def->name); + diag_set(ClientError, ER_SQL_COLUMN_COUNT_MAX, def->name, + def->field_count + 1, db->aLimit[SQL_LIMIT_COLUMN]); + pParse->is_aborted = true; return; } #endif @@ -521,9 +532,12 @@ sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan) struct space_def *def = pParse->new_space->def; if (!sqlExprIsConstantOrFunction (pSpan->pExpr, db->init.busy)) { - sqlErrorMsg(pParse, - "default value of column [%s] is not constant", - def->fields[def->field_count - 1].name); + const char *column_name = + def->fields[def->field_count - 1].name; + diag_set(ClientError, ER_CREATE_SPACE, def->name, + tt_sprintf("default value of column '%s' is "\ + "not constant", column_name)); + pParse->is_aborted = true; } else { assert(def != NULL); struct field_def *field = @@ -588,9 +602,9 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */ if (space == NULL) goto primary_key_exit; if (sql_space_primary_key(space) != NULL) { - sqlErrorMsg(pParse, - "table \"%s\" has more than one primary key", - space->def->name); + diag_set(ClientError, ER_CREATE_SPACE, space->def->name, + "primary key already exists"); + pParse->is_aborted = true; goto primary_key_exit; } if (pList == NULL) { @@ -603,8 +617,9 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */ sqlExprSkipCollate(pList->a[i].pExpr); assert(pCExpr != 0); if (pCExpr->op != TK_ID) { - sqlErrorMsg(pParse, "expressions prohibited" - " in PRIMARY KEY"); + diag_set(ClientError, ER_INDEX_DEF_UNSUPPORTED, + "Expressions"); + pParse->is_aborted = true; goto primary_key_exit; } const char *name = pCExpr->u.zToken; @@ -636,8 +651,10 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */ if (db->mallocFailed) goto primary_key_exit; } else if (autoInc) { - sqlErrorMsg(pParse, "AUTOINCREMENT is only allowed on an " - "INTEGER PRIMARY KEY or INT PRIMARY KEY"); + diag_set(ClientError, ER_CREATE_SPACE, space->def->name, + "AUTOINCREMENT is only allowed on an INTEGER PRIMARY "\ + "KEY or INT PRIMARY KEY"); + pParse->is_aborted = true; goto primary_key_exit; } else { sql_create_index(pParse, 0, 0, pList, 0, sortOrder, false, @@ -1144,9 +1161,10 @@ sqlEndTable(Parse * pParse, /* Parse context */ if (!new_space->def->opts.is_view) { if (sql_space_primary_key(new_space) == NULL) { - sqlErrorMsg(pParse, - "PRIMARY KEY missing on table %s", - new_space->def->name); + diag_set(ClientError, ER_CREATE_SPACE, + new_space->def->name, + "PRIMARY KEY missing"); + pParse->is_aborted = true; goto cleanup; } } @@ -1264,8 +1282,10 @@ sql_create_view(struct Parse *parse_context, struct Token *begin, { struct sql *db = parse_context->db; if (parse_context->nVar > 0) { - sqlErrorMsg(parse_context, - "parameters are not allowed in views"); + diag_set(ClientError, ER_CREATE_SPACE, + sqlNameFromToken(db, name), + "parameters are not allowed in views"); + parse_context->is_aborted = true; goto create_view_fail; } sqlStartTable(parse_context, name, if_exists); @@ -1279,10 +1299,10 @@ sql_create_view(struct Parse *parse_context, struct Token *begin, goto create_view_fail; if (aliases != NULL) { if ((int)select_res_space->def->field_count != aliases->nExpr) { - sqlErrorMsg(parse_context, "expected %d columns "\ - "for '%s' but got %d", aliases->nExpr, - space->def->name, - select_res_space->def->field_count); + diag_set(ClientError, ER_CREATE_SPACE, space->def->name, + "number of aliases doesn't match provided "\ + "columns"); + parse_context->is_aborted = true; goto create_view_fail; } sqlColumnsFromExprList(parse_context, aliases, space->def); @@ -1604,13 +1624,15 @@ sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list, * and DROP VIEW is not used on a table. */ if (is_view && !space->def->opts.is_view) { - sqlErrorMsg(parse_context, "use DROP TABLE to delete table %s", - space_name); + diag_set(ClientError, ER_DROP_SPACE, space_name, + "use DROP TABLE"); + parse_context->is_aborted = true; goto exit_drop_table; } if (!is_view && space->def->opts.is_view) { - sqlErrorMsg(parse_context, "use DROP VIEW to delete view %s", - space_name); + diag_set(ClientError, ER_DROP_SPACE, space_name, + "use DROP VIEW"); + parse_context->is_aborted = true; goto exit_drop_table; } /* @@ -1753,12 +1775,6 @@ sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child, diag_set(ClientError, ER_NO_SUCH_SPACE, parent_name);; goto tnt_error; } - } else { - if (parent_space->def->opts.is_view) { - sqlErrorMsg(parse_context, - "referenced table can't be view"); - goto exit_create_fk; - } } if (constraint == NULL && !is_alter) { if (parse_context->constraintName.n == 0) { @@ -1775,6 +1791,11 @@ sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child, } if (constraint_name == NULL) goto exit_create_fk; + if (!is_self_referenced && parent_space->def->opts.is_view) { + diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, constraint_name, + "referenced space can't be VIEW"); + goto tnt_error; + } const char *error_msg = "number of columns in foreign key does not " "match the number of columns in the primary " "index of referenced table"; @@ -2144,7 +2165,10 @@ sql_create_index(struct Parse *parse, struct Token *token, struct space_def *def = space->def; if (def->opts.is_view) { - sqlErrorMsg(parse, "views can not be indexed"); + diag_set(ClientError, ER_MODIFY_INDEX, + sqlNameFromToken(db, token), def->name, + "views can not be indexed"); + parse->is_aborted = true; goto exit_create_index; } /* @@ -2245,7 +2269,12 @@ sql_create_index(struct Parse *parse, struct Token *token, assert(col_list->nExpr == 1); sqlExprListSetSortOrder(col_list, sort_order); } else { - sqlExprListCheckLength(parse, col_list, "index"); + if (col_list->nExpr > db->aLimit[SQL_LIMIT_COLUMN]) { + diag_set(ClientError, ER_SQL_PARSER_LIMIT, + "The number of columns in index", + col_list->nExpr, db->aLimit[SQL_LIMIT_COLUMN]); + parse->is_aborted = true; + } } index = (struct index *) region_alloc(&parse->region, sizeof(*index)); @@ -2942,11 +2971,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) != 0) return; - } sqlVdbeAddOp4(v, OP_Savepoint, op, 0, 0, zName, P4_DYNAMIC); } } diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index f4d0334..0eb28d7 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -110,8 +110,8 @@ sql_table_truncate(struct Parse *parse, struct SrcList *tab_list) } if (space->def->opts.is_view) { const char *err_msg = - tt_sprintf("can not truncate space '%s' because it is " - "a view", space->def->name); + tt_sprintf("can not truncate space '%s' because space "\ + "is a view", space->def->name); diag_set(ClientError, ER_SQL, err_msg); goto tarantool_error; } @@ -163,8 +163,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, goto delete_from_cleanup; if (trigger_list == NULL) { - sqlErrorMsg(parse, "cannot modify %s because it is a" - " view", space->def->name); + diag_set(ClientError, ER_ALTER_SPACE, space->def->name, + "space is a view"); + parse->is_aborted = true; goto delete_from_cleanup; } } diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index a2c7093..de993c1 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -746,19 +746,24 @@ codeVectorCompare(Parse * pParse, /* Code generator context */ * Check that argument nHeight is less than or equal to the maximum * expression depth allowed. If it is not, leave an error message in * pParse. + * + * @param pParse Parser context. + * @param zName Depth to check. + * + * @retval 0 on success. + * @retval -1 on error. */ int sqlExprCheckHeight(Parse * pParse, int nHeight) { - int rc = SQL_OK; int mxHeight = pParse->db->aLimit[SQL_LIMIT_EXPR_DEPTH]; if (nHeight > mxHeight) { - sqlErrorMsg(pParse, - "Expression tree is too large (maximum depth %d)", - mxHeight); - rc = SQL_ERROR; + diag_set(ClientError, ER_SQL_PARSER_LIMIT, "Number of nodes "\ + "in expression tree", nHeight, mxHeight); + pParse->is_aborted = true; + return -1; } - return rc; + return 0; } /* The following three functions, heightOfExpr(), heightOfExprList() @@ -1197,9 +1202,9 @@ sqlExprAssignVarNumber(Parse * pParse, Expr * pExpr, u32 n) testcase(i == SQL_BIND_PARAMETER_MAX - 1); testcase(i == SQL_BIND_PARAMETER_MAX); if (!is_ok || i < 1 || i > SQL_BIND_PARAMETER_MAX) { - sqlErrorMsg(pParse, - "variable number must be between $1 and $%d", - SQL_BIND_PARAMETER_MAX); + diag_set(ClientError, ER_SQL_BIND_PARAMETER_MAX, + SQL_BIND_PARAMETER_MAX); + pParse->is_aborted = true; return; } if (x > pParse->nVar) { @@ -1227,7 +1232,9 @@ sqlExprAssignVarNumber(Parse * pParse, Expr * pExpr, u32 n) } pExpr->iColumn = x; if (x > SQL_BIND_PARAMETER_MAX) { - sqlErrorMsg(pParse, "too many SQL variables"); + diag_set(ClientError, ER_SQL_BIND_PARAMETER_MAX, + SQL_BIND_PARAMETER_MAX); + pParse->is_aborted = true; } } @@ -1907,22 +1914,6 @@ sqlExprListSetSpan(Parse * pParse, /* Parsing context */ } /* - * If the expression list pEList contains more than iLimit elements, - * leave an error message in pParse. - */ -void -sqlExprListCheckLength(Parse * pParse, - ExprList * pEList, const char *zObject) -{ - int mx = pParse->db->aLimit[SQL_LIMIT_COLUMN]; - testcase(pEList && pEList->nExpr == mx); - testcase(pEList && pEList->nExpr == mx + 1); - if (pEList && pEList->nExpr > mx) { - sqlErrorMsg(pParse, "too many columns in %s", zObject); - } -} - -/* * Delete an entire expression list. */ static SQL_NOINLINE void @@ -3355,15 +3346,15 @@ expr_code_int(struct Parse *parse, struct Expr *expr, bool is_neg, int c = sql_dec_or_hex_to_i64(z, &value); if (c == 1 || (c == 2 && !is_neg) || (is_neg && value == SMALLEST_INT64)) { + const char *sign = is_neg ? "-" : ""; if (sql_strnicmp(z, "0x", 2) == 0) { - sqlErrorMsg(parse, - "hex literal too big: %s%s", - is_neg ? "-" : "", z); + diag_set(ClientError, ER_HEX_LITERAL_MAX, sign, + z, strlen(z) - 2, 16); } else { - sqlErrorMsg(parse, - "oversized integer: %s%s", - is_neg ? "-" : "", z); + diag_set(ClientError, ER_INT_LITERAL_MAX, sign, + z, INT64_MIN, INT64_MAX); } + parse->is_aborted = true; } else { if (is_neg) value = c == 2 ? SMALLEST_INT64 : -value; diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 6f7f020..0ca38ae 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -322,8 +322,9 @@ sqlInsert(Parse * pParse, /* Parser context */ /* Cannot insert into a read-only table. */ if (is_view && tmask == 0) { - sqlErrorMsg(pParse, "cannot modify %s because it is a view", - space_def->name); + diag_set(ClientError, ER_ALTER_SPACE, space->def->name, + "space is a view"); + pParse->is_aborted = true; goto insert_cleanup; } @@ -388,9 +389,10 @@ sqlInsert(Parse * pParse, /* Parser context */ } } if (j >= (int) space_def->field_count) { - sqlErrorMsg(pParse, - "table %S has no column named %s", - pTabList, 0, pColumn->a[i].zName); + diag_set(ClientError, ER_NO_SUCH_FIELD_NAME, + pColumn->a[i].zName, + pTabList->a[0].zName); + pParse->is_aborted = true; goto insert_cleanup; } if (bit_test(used_columns, j)) { diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index b27651c..ff668e0 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -410,9 +410,10 @@ cmd ::= select(X). { (mxSelect = pParse->db->aLimit[SQL_LIMIT_COMPOUND_SELECT])>0 && cnt>mxSelect ){ - sqlErrorMsg(pParse, "Too many UNION or EXCEPT or INTERSECT " - "operations (limit %d is set)", - pParse->db->aLimit[SQL_LIMIT_COMPOUND_SELECT]); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, "The number of UNION or "\ + "EXCEPT or INTERSECT operations", cnt, + pParse->db->aLimit[SQL_LIMIT_COMPOUND_SELECT]); + pParse->is_aborted = true; } } } @@ -744,7 +745,11 @@ cmd ::= with(C) UPDATE orconf(R) fullname(X) indexed_opt(I) SET setlist(Y) where_opt(W). { sqlWithPush(pParse, C, 1); sqlSrcListIndexedBy(pParse, X, &I); - sqlExprListCheckLength(pParse,Y,"set list"); + if (Y != NULL && Y->nExpr > pParse->db->aLimit[SQL_LIMIT_COLUMN]) { + diag_set(ClientError, ER_SQL_PARSER_LIMIT, "The number of columns in set "\ + "list", Y->nExpr, pParse->db->aLimit[SQL_LIMIT_COLUMN]); + pParse->is_aborted = true; + } sqlSubProgramsRemaining = SQL_MAX_COMPILING_TRIGGERS; /* Instruct SQL to initate Tarantool's transaction. */ pParse->initiateTTrans = true; @@ -932,7 +937,11 @@ expr(A) ::= CAST(X) LP expr(E) AS typedef(T) RP(Y). { %endif SQL_OMIT_CAST expr(A) ::= id(X) LP distinct(D) exprlist(Y) RP(E). { if( Y && Y->nExpr>pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG] ){ - sqlErrorMsg(pParse, "too many arguments on function %T", &X); + const char *err = + tt_sprintf("Number of arguments to function %.*s", X.n, X.z); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, err, Y->nExpr, + pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG]); + pParse->is_aborted = true; } A.pExpr = sqlExprFunction(pParse, Y, &X); spanSet(&A,&X,&E); @@ -948,7 +957,11 @@ expr(A) ::= id(X) LP distinct(D) exprlist(Y) RP(E). { type_func(A) ::= CHAR(A) . expr(A) ::= type_func(X) LP distinct(D) exprlist(Y) RP(E). { if( Y && Y->nExpr>pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG] ){ - sqlErrorMsg(pParse, "too many arguments on function %T", &X); + const char *err = + tt_sprintf("Number of arguments to function %.*s", X.n, X.z); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, err, Y->nExpr, + pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG]); + pParse->is_aborted = true; } A.pExpr = sqlExprFunction(pParse, Y, &X); spanSet(&A,&X,&E); diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 94bb0af..8cdcfd0 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -510,30 +510,6 @@ sqlCreateColumnExpr(sql * db, SrcList * pSrc, int iSrc, int iCol) } /* - * Report an error that an expression is not valid for some set of - * pNC->ncFlags values determined by validMask. - */ -static void -notValid(Parse * pParse, /* Leave error message here */ - NameContext * pNC, /* The name context */ - const char *zMsg, /* Type of error */ - int validMask /* Set of contexts for which prohibited */ - ) -{ - assert((validMask & ~(NC_IsCheck | NC_IdxExpr)) == 0); - if ((pNC->ncFlags & validMask) != 0) { - const char *zIn; - if (pNC->ncFlags & NC_IdxExpr) - zIn = "index expressions"; - else if (pNC->ncFlags & NC_IsCheck) - zIn = "CHECK constraints"; - else - unreachable(); - sqlErrorMsg(pParse, "%s prohibited in %s", zMsg, zIn); - } -} - -/* * Expression p should encode a floating point value between 1.0 and 0.0. * Return 1024 times this value. Or return -1 if p is not a floating point * value between 1.0 and 0.0. @@ -605,7 +581,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) Expr *pRight; /* if( pSrcList==0 ) break; */ - notValid(pParse, pNC, "the \".\" operator", NC_IdxExpr); + if (pNC->ncFlags & NC_IdxExpr) { + diag_set(ClientError, ER_INDEX_DEF_UNSUPPORTED, + "Expressions"); + pParse->is_aborted = true; + } pRight = pExpr->pRight; if (pRight->op == TK_ID) { zTable = pExpr->pLeft->u.zToken; @@ -646,6 +626,9 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) } else { is_agg = pDef->xFinalize != 0; pExpr->type = pDef->ret_type; + const char *err_msg = + "second argument to likelihood() must "\ + "be a constant between 0.0 and 1.0"; if (pDef->funcFlags & SQL_FUNC_UNLIKELY) { ExprSetProperty(pExpr, EP_Unlikely | EP_Skip); @@ -654,9 +637,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) exprProbability(pList->a[1]. pExpr); if (pExpr->iTable < 0) { - sqlErrorMsg(pParse, - "second argument to likelihood() must be a " - "constant between 0.0 and 1.0"); + diag_set(ClientError, + ER_ILLEGAL_PARAMS, + err_msg); + pParse->is_aborted = + true; pNC->nErr++; } } else { @@ -690,9 +675,7 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) * that might change over time cannot be used * in an index. */ - notValid(pParse, pNC, - "non-deterministic functions", - NC_IdxExpr); + assert((pNC->ncFlags & NC_IdxExpr) == 0); } } if (is_agg && (pNC->ncFlags & NC_AllowAgg) == 0) { @@ -754,8 +737,13 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) testcase(pExpr->op == TK_IN); if (ExprHasProperty(pExpr, EP_xIsSelect)) { int nRef = pNC->nRef; - notValid(pParse, pNC, "subqueries", - NC_IsCheck | NC_IdxExpr); + assert((pNC->ncFlags & NC_IdxExpr) == 0); + if (pNC->ncFlags & NC_IsCheck) { + diag_set(ClientError, + ER_CK_DEF_UNSUPPORTED, + "Subqueries"); + pParse->is_aborted = true; + } sqlWalkSelect(pWalker, pExpr->x.pSelect); assert(pNC->nRef >= nRef); if (nRef != pNC->nRef) { @@ -766,8 +754,12 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) break; } case TK_VARIABLE:{ - notValid(pParse, pNC, "parameters", - NC_IsCheck | NC_IdxExpr); + assert((pNC->ncFlags & NC_IsCheck) == 0); + if (pNC->ncFlags & NC_IdxExpr) { + diag_set(ClientError, ER_INDEX_DEF_UNSUPPORTED, + "Parameter markers"); + pParse->is_aborted = true; + } break; } case TK_BETWEEN: @@ -952,7 +944,10 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages db = pParse->db; #if SQL_MAX_COLUMN if (pOrderBy->nExpr > db->aLimit[SQL_LIMIT_COLUMN]) { - sqlErrorMsg(pParse, "too many terms in ORDER BY clause"); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, + "The number of terms in ORDER BY clause", + pOrderBy->nExpr, db->aLimit[SQL_LIMIT_COLUMN]); + pParse->is_aborted = true; return 1; } #endif @@ -1042,27 +1037,34 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages * field) then convert that term into a copy of the corresponding result set * column. * - * If any errors are detected, add an error message to pParse and - * return non-zero. Return zero if no errors are seen. + * @param pParse Parsing context. + * @param pSelect The SELECT statement containing the clause. + * @param pOrderBy The ORDER BY or GROUP BY clause to be + * processed. + * @param is_order_by True if pOrderBy is ORDER BY, false if + * pOrderBy is GROUP BY + * @retval 0 On success, not 0 elsewhere. */ int -sqlResolveOrderGroupBy(Parse * pParse, /* Parsing context. Leave error messages here */ - Select * pSelect, /* The SELECT statement containing the clause */ - ExprList * pOrderBy, /* The ORDER BY or GROUP BY clause to be processed */ - const char *zType /* "ORDER" or "GROUP" */ - ) +sqlResolveOrderGroupBy(Parse *pParse, Select *pSelect, ExprList *pOrderBy, + bool is_order_by) { int i; sql *db = pParse->db; ExprList *pEList; struct ExprList_item *pItem; + const char *zType = is_order_by ? "ORDER" : "GROUP"; if (pOrderBy == 0 || pParse->db->mallocFailed) return 0; #if SQL_MAX_COLUMN if (pOrderBy->nExpr > db->aLimit[SQL_LIMIT_COLUMN]) { - sqlErrorMsg(pParse, "too many terms in %s BY clause", - zType); + const char *err_msg = + is_order_by ? "The number of terms in ORDER BY clause" : + "The number of terms in GROUP BY clause"; + diag_set(ClientError, ER_SQL_PARSER_LIMIT, err_msg, + pOrderBy->nExpr, db->aLimit[SQL_LIMIT_COLUMN]); + pParse->is_aborted = true; return 1; } #endif @@ -1096,22 +1098,23 @@ sqlResolveOrderGroupBy(Parse * pParse, /* Parsing context. Leave error messages * result-set expression. Otherwise, the expression is resolved in * the usual way - using sqlResolveExprNames(). * - * This routine returns the number of errors. If errors occur, then - * an appropriate error message might be left in pParse. (OOM errors - * excepted.) + * @param pNC The name context of the SELECT statement. + * @param pSelect The SELECT statement containing the clause. + * @param pOrderBy An ORDER BY or GROUP BY clause to resolve. + * @param is_order_by True if pOrderBy is ORDER BY, false if + * pOrderBy is GROUP BY + * @retval 0 On success, not 0 elsewhere. */ static int -resolveOrderGroupBy(NameContext * pNC, /* The name context of the SELECT statement */ - Select * pSelect, /* The SELECT statement holding pOrderBy */ - ExprList * pOrderBy, /* An ORDER BY or GROUP BY clause to resolve */ - const char *zType /* Either "ORDER" or "GROUP", as appropriate */ - ) +resolveOrderGroupBy(NameContext *pNC, Select *pSelect, ExprList *pOrderBy, + bool is_order_by) { int i, j; /* Loop counters */ int iCol; /* Column number */ struct ExprList_item *pItem; /* A term of the ORDER BY clause */ Parse *pParse; /* Parsing context */ int nResult; /* Number of terms in the result set */ + const char *zType = is_order_by ? "ORDER" : "GROUP"; if (pOrderBy == 0) return 0; @@ -1158,7 +1161,7 @@ resolveOrderGroupBy(NameContext * pNC, /* The name context of the SELECT stateme } } } - return sqlResolveOrderGroupBy(pParse, pSelect, pOrderBy, zType); + return sqlResolveOrderGroupBy(pParse, pSelect, pOrderBy, is_order_by); } /* @@ -1401,7 +1404,7 @@ resolveSelectStep(Walker * pWalker, Select * p) * resolve those symbols on the incorrect ORDER BY for consistency. */ if (isCompound <= nCompound /* Defer right-most ORDER BY of a compound */ - && resolveOrderGroupBy(&sNC, p, p->pOrderBy, "ORDER") + && resolveOrderGroupBy(&sNC, p, p->pOrderBy, true) ) { return WRC_Abort; } @@ -1415,7 +1418,7 @@ resolveSelectStep(Walker * pWalker, Select * p) if (pGroupBy) { struct ExprList_item *pItem; - if (resolveOrderGroupBy(&sNC, p, pGroupBy, "GROUP") + if (resolveOrderGroupBy(&sNC, p, pGroupBy, false) || db->mallocFailed) { return WRC_Abort; } diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 5195656..3f58f30 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -3396,11 +3396,9 @@ multiSelectOrderBy(Parse * pParse, /* Parsing context */ */ p->pPrior = 0; pPrior->pNext = 0; - sqlResolveOrderGroupBy(pParse, p, p->pOrderBy, "ORDER"); - if (pPrior->pPrior == 0) { - sqlResolveOrderGroupBy(pParse, pPrior, pPrior->pOrderBy, - "ORDER"); - } + sqlResolveOrderGroupBy(pParse, p, p->pOrderBy, true); + if (pPrior->pPrior == 0) + sqlResolveOrderGroupBy(pParse, pPrior, pPrior->pOrderBy, true); /* Compute the limit registers */ computeLimitRegisters(pParse, p, labelEnd); @@ -5064,7 +5062,10 @@ selectExpander(Walker * pWalker, Select * p) } #if SQL_MAX_COLUMN if (p->pEList && p->pEList->nExpr > db->aLimit[SQL_LIMIT_COLUMN]) { - sqlErrorMsg(pParse, "too many columns in result set"); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, "The number of "\ + "columns in result set", p->pEList->nExpr, + db->aLimit[SQL_LIMIT_COLUMN]); + pParse->is_aborted = true; return WRC_Abort; } #endif @@ -5525,10 +5526,10 @@ sqlSelect(Parse * pParse, /* The parser context */ * columns in the SELECT on the RHS */ if ((int)space->def->field_count != pSub->pEList->nExpr) { - sqlErrorMsg(pParse, - "expected %d columns for '%s' but got %d", - space->def->field_count, space->def->name, - pSub->pEList->nExpr); + diag_set(ClientError, ER_CREATE_SPACE, space->def->name, + "number of aliases doesn't match provided "\ + "columns"); + pParse->is_aborted = true; goto select_end; } diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 8967ea3..363ee8d 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4441,7 +4441,7 @@ int sqlMatchSpanName(const char *, const char *, const char *); int sqlResolveExprNames(NameContext *, Expr *); int sqlResolveExprListNames(NameContext *, ExprList *); void sqlResolveSelectNames(Parse *, Select *, NameContext *); -int sqlResolveOrderGroupBy(Parse *, Select *, ExprList *, const char *); +int sqlResolveOrderGroupBy(Parse *, Select *, ExprList *, bool); /** * Generate code for default value. @@ -4649,7 +4649,6 @@ sql_int64 sqlStmtCurrentTime(sql_context *); int sqlVdbeParameterIndex(Vdbe *, const char *, int); int sqlTransferBindings(sql_stmt *, sql_stmt *); int sqlReprepare(Vdbe *); -void sqlExprListCheckLength(Parse *, ExprList *, const char *); /** * This function verifies that two collations (to be more precise diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c index de0f282..efb895f 100644 --- a/src/box/sql/tokenize.c +++ b/src/box/sql/tokenize.c @@ -481,8 +481,8 @@ sqlRunParser(Parse * pParse, const char *zSql) &pParse->sLastToken.isReserved); i += pParse->sLastToken.n; if (i > mxSqlLen) { - diag_set(ClientError, ER_SQL_PARSER_GENERIC, - "string or blob too big"); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, + "SQL command length", i, mxSqlLen); pParse->is_aborted = true; break; } diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 7eacd33..b23d60a 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -86,7 +86,7 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm, if (trigger_name == NULL) goto trigger_cleanup; - if (sqlCheckIdentifierName(parse, trigger_name) != SQL_OK) + if (sqlCheckIdentifierName(parse, trigger_name) != 0) goto trigger_cleanup; const char *table_name = table->a[0].zName; diff --git a/src/box/sql/update.c b/src/box/sql/update.c index 05ceeb4..71a1e00 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -139,8 +139,9 @@ sqlUpdate(Parse * pParse, /* The parser context */ goto update_cleanup; } if (is_view && tmask == 0) { - sqlErrorMsg(pParse, "cannot modify %s because it is a view", - space->def->name); + diag_set(ClientError, ER_ALTER_SPACE, space->def->name, + "space is a view"); + pParse->is_aborted = true; goto update_cleanup; } diff --git a/src/box/sql/where.c b/src/box/sql/where.c index ebc2624..2f83e81 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -4286,7 +4286,9 @@ sqlWhereBegin(Parse * pParse, /* The parser context */ */ testcase(pTabList->nSrc == BMS); if (pTabList->nSrc > BMS) { - sqlErrorMsg(pParse, "at most %d tables in a join", BMS); + diag_set(ClientError, ER_SQL_PARSER_LIMIT, "The number of "\ + "tables in a join", pTabList->nSrc, BMS); + pParse->is_aborted = true; return 0; } diff --git a/test/box/alter.result b/test/box/alter.result index 9a1086e..37bc51c 100644 --- a/test/box/alter.result +++ b/test/box/alter.result @@ -367,7 +367,7 @@ s:drop() ... box.schema.space.create('') --- -- error: Invalid identifier '' (expected printable symbols only) +- error: Invalid identifier '' (expected printable symbols only or it is too long) ... -- valid identifiers box.schema.space.create('_Abcde'):drop() @@ -438,7 +438,7 @@ i:drop() ... space:create_index('') --- -- error: Invalid identifier '' (expected printable symbols only) +- error: Invalid identifier '' (expected printable symbols only or it is too long) ... space:drop() --- diff --git a/test/box/misc.result b/test/box/misc.result index c350bbd..5dda752 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -516,6 +516,12 @@ t; 185: box.error.SQL_UNKNOWN_TOKEN 186: box.error.SQL_PARSER_GENERIC 187: box.error.SQL_ANALYZE_ARGUMENT + 188: box.error.SQL_COLUMN_COUNT_MAX + 189: box.error.HEX_LITERAL_MAX + 190: box.error.INT_LITERAL_MAX + 191: box.error.SQL_PARSER_LIMIT + 192: box.error.INDEX_DEF_UNSUPPORTED + 193: box.error.CK_DEF_UNSUPPORTED ... test_run:cmd("setopt delimiter ''"); --- diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua index dc2f60e..7abb679 100755 --- a/test/sql-tap/autoinc.test.lua +++ b/test/sql-tap/autoinc.test.lua @@ -561,7 +561,7 @@ test:do_catchsql_test( CREATE TABLE t8(x TEXT PRIMARY KEY AUTOINCREMENT); ]], { -- - 1, "AUTOINCREMENT is only allowed on an INTEGER PRIMARY KEY or INT PRIMARY KEY" + 1, "Failed to create space 'T8': AUTOINCREMENT is only allowed on an INTEGER PRIMARY KEY or INT PRIMARY KEY" -- }) diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua index 0d8bf15..dd2de92 100755 --- a/test/sql-tap/check.test.lua +++ b/test/sql-tap/check.test.lua @@ -319,7 +319,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "Failed to create space 'T3': subqueries prohibited in CHECK constraints" + 1, "Failed to create space 'T3': Subqueries are prohibited in a CHECK constraint definition" -- }) diff --git a/test/sql-tap/colname.test.lua b/test/sql-tap/colname.test.lua index 29fdf13..253497c 100755 --- a/test/sql-tap/colname.test.lua +++ b/test/sql-tap/colname.test.lua @@ -637,7 +637,7 @@ test:do_test( test:do_catchsql_test( "colname-11.1", [[ create table t1(a INT, b INT, c INT, primary key('A'))]], - {1, "expressions prohibited in PRIMARY KEY"}) + {1, "Expressions are prohibited in an index definition"}) test:do_catchsql_test( "colname-11.2", diff --git a/test/sql-tap/default.test.lua b/test/sql-tap/default.test.lua index 67151b0..564d373 100755 --- a/test/sql-tap/default.test.lua +++ b/test/sql-tap/default.test.lua @@ -66,7 +66,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "default value of column [Y] is not constant" + 1, "Failed to create space 'T3': default value of column 'Y' is not constant" -- }) @@ -173,7 +173,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "default value of column [B] is not constant" + 1, "Failed to create space 'T2': default value of column 'B' is not constant" -- }) @@ -187,7 +187,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "default value of column [B] is not constant" + 1, "Failed to create space 'T2': default value of column 'B' is not constant" -- }) @@ -201,7 +201,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "default value of column [B] is not constant" + 1, "Failed to create space 'T2': default value of column 'B' is not constant" -- }) @@ -214,7 +214,7 @@ test:do_catchsql_test( CREATE TABLE t6(id INTEGER PRIMARY KEY, b TEXT DEFAULT(id)); ]], { -- - 1, "default value of column [B] is not constant" + 1, "Failed to create space 'T6': default value of column 'B' is not constant" -- }) diff --git a/test/sql-tap/fkey2.test.lua b/test/sql-tap/fkey2.test.lua index d347e5a..678b857 100755 --- a/test/sql-tap/fkey2.test.lua +++ b/test/sql-tap/fkey2.test.lua @@ -729,7 +729,7 @@ test:do_catchsql_test( CREATE TABLE c(x INT PRIMARY KEY REFERENCES v(y)); ]], { -- - 1, "referenced table can't be view" + 1, "Failed to create foreign key constraint 'FK_CONSTRAINT_1_C': referenced space can't be VIEW" -- }) @@ -1157,7 +1157,7 @@ test:do_catchsql_test( CREATE TABLE t1(x INT PRIMARY KEY REFERENCES v); ]], { -- - 1, "referenced table can't be view" + 1, "Failed to create foreign key constraint 'FK_CONSTRAINT_1_T1': referenced space can't be VIEW" -- }) diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua index 4ea9a5c..6d6411c 100755 --- a/test/sql-tap/func3.test.lua +++ b/test/sql-tap/func3.test.lua @@ -104,7 +104,7 @@ test:do_catchsql_test( SELECT likelihood(123, 1.000001); ]], { -- - 1, "second argument to likelihood() must be a constant between 0.0 and 1.0" + 1, "Illegal parameters, second argument to likelihood() must be a constant between 0.0 and 1.0" -- }) @@ -114,7 +114,7 @@ test:do_catchsql_test( SELECT likelihood(123, -0.000001); ]], { -- - 1, "second argument to likelihood() must be a constant between 0.0 and 1.0" + 1, "Illegal parameters, second argument to likelihood() must be a constant between 0.0 and 1.0" -- }) @@ -124,7 +124,7 @@ test:do_catchsql_test( SELECT likelihood(123, 0.5+0.3); ]], { -- - 1, "second argument to likelihood() must be a constant between 0.0 and 1.0" + 1, "Illegal parameters, second argument to likelihood() must be a constant between 0.0 and 1.0" -- }) diff --git a/test/sql-tap/gh-2549-many-columns.test.lua b/test/sql-tap/gh-2549-many-columns.test.lua index 3de4d67..ed8c9d9 100755 --- a/test/sql-tap/gh-2549-many-columns.test.lua +++ b/test/sql-tap/gh-2549-many-columns.test.lua @@ -35,7 +35,7 @@ test:do_catchsql_test( "columns-1.2", fail_statement, { -- - 1, "too many columns on T2" + 1, "Failed to create space 'T2': space column count 2001 exceeds the limit (2000)" -- }) diff --git a/test/sql-tap/gh2548-select-compound-limit.test.lua b/test/sql-tap/gh2548-select-compound-limit.test.lua index 5494a66..e8c8d95 100755 --- a/test/sql-tap/gh2548-select-compound-limit.test.lua +++ b/test/sql-tap/gh2548-select-compound-limit.test.lua @@ -58,7 +58,7 @@ test:do_catchsql_test( "gh2548-select-compound-limit-2", select_string_last, { -- - 1, "Too many UNION or EXCEPT or INTERSECT operations (limit 30 is set)" + 1, "The number of UNION or EXCEPT or INTERSECT operations 31 exceeds the limit (30)" -- }) diff --git a/test/sql-tap/hexlit.test.lua b/test/sql-tap/hexlit.test.lua index 158eda7..288d823 100755 --- a/test/sql-tap/hexlit.test.lua +++ b/test/sql-tap/hexlit.test.lua @@ -107,7 +107,7 @@ test:do_catchsql_test( SELECT 0x10000000000000000; ]], { -- - 1, "hex literal too big: 0x10000000000000000" + 1, "Hex literal 0x10000000000000000 length 17 exceeds the supported limit (16)" -- }) @@ -119,7 +119,7 @@ test:do_catchsql_test( INSERT INTO t1 VALUES(1+0x10000000000000000); ]], { -- - 1, "hex literal too big: 0x10000000000000000" + 1, "Hex literal 0x10000000000000000 length 17 exceeds the supported limit (16)" -- }) diff --git a/test/sql-tap/insert1.test.lua b/test/sql-tap/insert1.test.lua index 363fa8a..031fd22 100755 --- a/test/sql-tap/insert1.test.lua +++ b/test/sql-tap/insert1.test.lua @@ -77,7 +77,7 @@ test:do_catchsql_test("insert-1.4", [[ INSERT INTO test1(one,four) VALUES(1,2) ]], { -- - 1, "table TEST1 has no column named FOUR" + 1, "Field 'FOUR' was not found in the space 'TEST1' format" -- }) diff --git a/test/sql-tap/join.test.lua b/test/sql-tap/join.test.lua index da29f77..ef60609 100755 --- a/test/sql-tap/join.test.lua +++ b/test/sql-tap/join.test.lua @@ -1067,11 +1067,11 @@ end jointest("join-12.2", 30, {0, {1}}) jointest("join-12.3", 63, {0, {1}}) jointest("join-12.4", 64, {0, {1}}) -jointest("join-12.5", 65, {1, 'at most 64 tables in a join'}) -jointest("join-12.6", 66, {1, 'at most 64 tables in a join'}) -jointest("join-12.7", 127, {1, 'at most 64 tables in a join'}) -jointest("join-12.8", 128, {1, 'at most 64 tables in a join'}) -jointest("join-12.9", 1000, {1, 'at most 64 tables in a join'}) +jointest("join-12.5", 65, {1, 'The number of tables in a join 65 exceeds the limit (64)'}) +jointest("join-12.6", 66, {1, 'The number of tables in a join 66 exceeds the limit (64)'}) +jointest("join-12.7", 127, {1, 'The number of tables in a join 127 exceeds the limit (64)'}) +jointest("join-12.8", 128, {1, 'The number of tables in a join 128 exceeds the limit (64)'}) +jointest("join-12.9", 1000, {1, 'The number of tables in a join 1000 exceeds the limit (64)'}) -- If sql is built with sql_MEMDEBUG, then the huge number of realloc() -- calls made by the following test cases are too time consuming to run. -- Without sql_MEMDEBUG, realloc() is fast enough that these are not @@ -1079,10 +1079,10 @@ jointest("join-12.9", 1000, {1, 'at most 64 tables in a join'}) --if X(0, "X!capable", [["pragma&&compileoption_diags"]]) then -- if X(703, "X!cmd", [=[["expr","[lsearch [db eval {PRAGMA compile_options}] MEMDEBUG]<0"]]=]) -- then -jointest("join-12.10", 65534, {1, 'at most 64 tables in a join'}) -jointest("join-12.11", 65535, {1, 'at most 64 tables in a join'}) -jointest("join-12.12", 65536, {1, 'at most 64 tables in a join'}) -jointest("join-12.13", 65537, {1, 'at most 64 tables in a join'}) +jointest("join-12.10", 65534, {1, 'The number of tables in a join 65534 exceeds the limit (64)'}) +jointest("join-12.11", 65535, {1, 'The number of tables in a join 65535 exceeds the limit (64)'}) +jointest("join-12.12", 65536, {1, 'The number of tables in a join 65536 exceeds the limit (64)'}) +jointest("join-12.13", 65537, {1, 'The number of tables in a join 65537 exceeds the limit (64)'}) -- end --end diff --git a/test/sql-tap/join3.test.lua b/test/sql-tap/join3.test.lua index 6b822de..876b312 100755 --- a/test/sql-tap/join3.test.lua +++ b/test/sql-tap/join3.test.lua @@ -85,7 +85,7 @@ test:do_test( return test:catchsql(sql) end, { -- - 1, "at most "..bitmask_size.." tables in a join" + 1, "The number of tables in a join " .. bitmask_size + 1 .. " exceeds the limit (".. bitmask_size ..")" -- }) diff --git a/test/sql-tap/misc1.test.lua b/test/sql-tap/misc1.test.lua index 1b288da..6b9f5fa 100755 --- a/test/sql-tap/misc1.test.lua +++ b/test/sql-tap/misc1.test.lua @@ -348,7 +348,7 @@ test:do_catchsql_test( ); ]], { -- - 1, [[table "ERROR1" has more than one primary key]] + 1, [[Failed to create space 'ERROR1': primary key already exists]] -- }) @@ -361,7 +361,7 @@ test:do_catchsql_test( ); ]], { -- - 1, [[table "ERROR1" has more than one primary key]] + 1, [[Failed to create space 'ERROR1': primary key already exists]] -- }) @@ -897,7 +897,7 @@ test:do_catchsql_test( CREATE TABLE test2(a text primary key, b text, primary key(a,b)); ]], { -- - 1, [[table "TEST2" has more than one primary key]] + 1, [[Failed to create space 'TEST2': primary key already exists]] -- }) diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua index 4029c20..7037d20 100755 --- a/test/sql-tap/select7.test.lua +++ b/test/sql-tap/select7.test.lua @@ -179,7 +179,7 @@ test:do_catchsql_test( "select7-6.2", sql, { -- - 1, "Too many UNION or EXCEPT or INTERSECT operations (limit 30 is set)" + 1, "The number of UNION or EXCEPT or INTERSECT operations 33 exceeds the limit (30)" -- }) diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua new file mode 100755 index 0000000..ac242c4 --- /dev/null +++ b/test/sql-tap/sql-errors.test.lua @@ -0,0 +1,419 @@ +#!/usr/bin/env tarantool +test = require("sqltester") +test:plan(36) + +test:execsql([[ + CREATE TABLE t0 (i INT PRIMARY KEY); + CREATE VIEW v0 AS SELECT * FROM t0; +]]) +format = {} +for i = 1, 2001 do format[i] = {name = 'A' .. i, type = 'unsigned'} end +s0 = box.schema.space.create('S0', {format = format}) +i0 = s0:create_index('I0') + +test:do_catchsql_test( + "sql-errors-1.1", + [[ + ANALYZE v0; + ]], { + -- + 1,"ANALYZE statement argument V0 is not a base table" + -- + }) + +create_statement = 'CREATE TABLE t2 (i INT PRIMARY KEY' +for i = 1, 2001 do + create_statement = create_statement .. ', s' .. i .. ' INT' +end +create_statement = create_statement .. ');' + +test:do_catchsql_test( + "sql-errors-1.2", + create_statement, + { + -- + 1,"Failed to create space 'T2': space column count 2001 exceeds the limit (2000)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.3", + [[ + CREATE TABLE t3 (i INT PRIMARY KEY, a INT DEFAULT(MAX(i, 1))); + ]], { + -- + 1,"Failed to create space 'T3': default value of column 'A' is not constant" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.4", + [[ + CREATE TABLE t4 (i INT PRIMARY KEY, a INT PRIMARY KEY); + ]], { + -- + 1,"Failed to create space 'T4': primary key already exists" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.5", + [[ + CREATE TABLE t5 (i TEXT PRIMARY KEY AUTOINCREMENT); + ]], { + -- + 1,"Failed to create space 'T5': AUTOINCREMENT is only allowed on an INTEGER PRIMARY KEY or INT PRIMARY KEY" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.6", + [[ + CREATE TABLE t6 (i INT); + ]], { + -- + 1,"Failed to create space 'T6': PRIMARY KEY missing" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.7", + [[ + CREATE VIEW v7(a,b) AS SELECT * FROM t0; + ]], { + -- + 1,"Failed to create space 'V7': number of aliases doesn't match provided columns" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.8", + [[ + DROP VIEW t0; + ]], { + -- + 1,"Can't drop space 'T0': use DROP TABLE" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.9", + [[ + DROP TABLE v0; + ]], { + -- + 1,"Can't drop space 'V0': use DROP VIEW" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.10", + [[ + CREATE TABLE t10(i INT PRIMARY KEY REFERENCES v0); + ]], { + -- + 1,"Failed to create foreign key constraint 'FK_CONSTRAINT_1_T10': referenced space can't be VIEW" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.11", + [[ + CREATE VIEW v11 AS SELECT * FROM t0 WHERE i = ?; + ]], { + -- + 1,"Failed to create space 'V11': parameters are not allowed in views" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.12", + [[ + CREATE INDEX i12 ON v0(i); + ]], { + -- + 1,"Can't create or modify index 'I12' in space 'V0': views can not be indexed" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.13", + [[ + SELECT 9223372036854775808; + ]], { + -- + 1,"Integer literal 9223372036854775808 exceeds the supported range -9223372036854775808 - 9223372036854775807" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.14", + [[ + SELECT 0x10000000000000000; + ]], { + -- + 1,"Hex literal 0x10000000000000000 length 17 exceeds the supported limit (16)" + -- + }) + +select_statement = 'SELECT i FROM t0 WHERE i = 0' +for i = 1, 200 do + select_statement = select_statement .. ' OR i = ' .. i +end +select_statement = select_statement .. ';' + +test:do_catchsql_test( + "sql-errors-1.15", + select_statement, + { + -- + 1,"Number of nodes in expression tree 201 exceeds the limit (200)" + -- + }) + +select_statement = 'SELECT CHAR(1' +for i = 1, 127 do + select_statement = select_statement .. ', ' .. i +end +select_statement = select_statement .. ');' + +test:do_catchsql_test( + "sql-errors-1.16", + select_statement, + { + -- + 1,"Number of arguments to function CHAR 128 exceeds the limit (127)" + -- + }) + +select_statement = 'SELECT MAX(1' +for i = 1, 127 do + select_statement = select_statement .. ', ' .. i +end +select_statement = select_statement .. ');' + +test:do_catchsql_test( + "sql-errors-1.17", + select_statement, + { + -- + 1,"Number of arguments to function MAX 128 exceeds the limit (127)" + -- + }) + +select_statement = 'SELECT 0' +for i = 1, 30 do + select_statement = select_statement .. ' UNION ALL SELECT ' .. i +end +select_statement = select_statement .. ';' + +test:do_catchsql_test( + "sql-errors-1.18", + select_statement, + { + -- + 1,"The number of UNION or EXCEPT or INTERSECT operations 31 exceeds the limit (30)" + -- + }) + +select_statement = 'SELECT 0' +for i = 1, 2000 do + select_statement = select_statement .. ', ' .. i +end +select_statement = select_statement .. ';' + +test:do_catchsql_test( + "sql-errors-1.19", + select_statement, + { + -- + 1,"The number of columns in result set 2001 exceeds the limit (2000)" + -- + }) + +select_statement = 'SELECT * FROM t0' +for i = 1, 64 do + select_statement = select_statement .. ', t0 as t' .. i +end +select_statement = select_statement .. ';' + +test:do_catchsql_test( + "sql-errors-1.20", + select_statement, + { + -- + 1,"The number of tables in a join 65 exceeds the limit (64)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.21", + [[ + SELECT $65001; + ]], { + -- + 1,"SQL bind parameter limit reached: 65000" + -- + }) + +select_statement = 'SELECT '..string.rep('?, ', box.schema.SQL_BIND_PARAMETER_MAX)..'?;' + +test:do_catchsql_test( + "sql-errors-1.22", + select_statement, + { + -- + 1,"SQL bind parameter limit reached: 65000" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.23", + [[ + INSERT INTO v0 VALUES (2); + ]], { + -- + 1,"Can't modify space 'V0': space is a view" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.24", + [[ + UPDATE v0 SET i = 2 WHERE i = 1; + ]], { + -- + 1,"Can't modify space 'V0': space is a view" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.25", + [[ + DELETE FROM v0; + ]], { + -- + 1,"Can't modify space 'V0': space is a view" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.26", + [[ + CREATE TABLE t26 (i INT, PRIMARY KEY('i')); + ]], { + -- + 1,"Expressions are prohibited in an index definition" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.27", + [[ + CREATE TABLE t27 (i INT PRIMARY KEY, CHECK(i < (SELECT * FROM t0))); + ]], { + -- + 1,"Failed to create space 'T27': Subqueries are prohibited in a CHECK constraint definition" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.28", + [[ + CREATE INDEX i28 ON t0(t0.i); + ]], { + -- + 1,"Expressions are prohibited in an index definition" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.29", + [[ + CREATE INDEX i29 ON t0($1); + ]], { + -- + 1,"Parameter markers are prohibited in an index definition" + -- + }) + +create_index_statement = 'CREATE INDEX i30 on t0(i'..string.rep(', i', 2000)..');' + +test:do_catchsql_test( + "sql-errors-1.30", + create_index_statement, + { + -- + 1,"The number of columns in index 2001 exceeds the limit (2000)" + -- + }) + +update_statement = 'UPDATE s0 SET a1 = a1 + 1' +for i = 2, 2001 do + update_statement = update_statement .. ', a' .. i .. ' = a' .. i .. ' + 1' +end +update_statement = update_statement .. ';' + +test:do_catchsql_test( + "sql-errors-1.31", + update_statement, + { + -- + 1,"The number of columns in set list 2001 exceeds the limit (2000)" + -- + }) + +select_statement = 'SELECT * FROM (SELECT 1 UNION ALL SELECT 1 ORDER BY 1'..string.rep(', 1', 2000)..')' + +test:do_catchsql_test( + "sql-errors-1.32", + select_statement, + { + -- + 1,"The number of terms in ORDER BY clause 2001 exceeds the limit (2000)" + -- + }) + +select_statement = 'SELECT 1 ORDER BY 1'..string.rep(', 1', 2000) + +test:do_catchsql_test( + "sql-errors-1.33", + select_statement, + { + -- + 1,"The number of terms in ORDER BY clause 2001 exceeds the limit (2000)" + -- + }) + +select_statement = 'SELECT 1 as '..string.rep('x', 65001) + +test:do_catchsql_test( + "sql-errors-1.34", + select_statement, + { + -- + 1,"Invalid identifier 'XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX' (expected printable symbols only or it is too long)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.35", + [[ + SELECT 1 as ""; + ]], { + -- + 1,"Invalid identifier '' (expected printable symbols only or it is too long)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.36", + [[ + SELECT likelihood(1, 2); + ]], { + -- + 1,"Illegal parameters, second argument to likelihood() must be a constant between 0.0 and 1.0" + -- + }) + +test:finish_test() diff --git a/test/sql-tap/unique.test.lua b/test/sql-tap/unique.test.lua index 56ac74a..a3fa1c9 100755 --- a/test/sql-tap/unique.test.lua +++ b/test/sql-tap/unique.test.lua @@ -36,7 +36,7 @@ test:do_catchsql_test( ); ]], { -- - 1, [[table "T1" has more than one primary key]] + 1, [[Failed to create space 'T1': primary key already exists]] -- }) diff --git a/test/sql-tap/view.test.lua b/test/sql-tap/view.test.lua index 72fdab8..0032e1b 100755 --- a/test/sql-tap/view.test.lua +++ b/test/sql-tap/view.test.lua @@ -189,7 +189,7 @@ test:do_catchsql_test( INSERT INTO v2 VALUES(1,2,3,4); ]], { -- - 1, "cannot modify V2 because it is a view" + 1, "Can't modify space 'V2': space is a view" -- }) @@ -199,7 +199,7 @@ test:do_catchsql_test( UPDATE v2 SET a=10 WHERE a=5; ]], { -- - 1, "cannot modify V2 because it is a view" + 1, "Can't modify space 'V2': space is a view" -- }) @@ -209,7 +209,7 @@ test:do_catchsql_test( DELETE FROM v2; ]], { -- - 1, "cannot modify V2 because it is a view" + 1, "Can't modify space 'V2': space is a view" -- }) @@ -302,7 +302,7 @@ test:do_catchsql_test( [[ CREATE VIEW v1err(x,y) AS SELECT a, b+c, c-b FROM t1; SELECT * FROM v1err; - ]], {1, "expected 2 columns for 'V1ERR' but got 3"}) + ]], {1, "Failed to create space 'V1ERR': number of aliases doesn't match provided columns"}) test:do_catchsql_test( "view-3.3.5.2", @@ -310,7 +310,7 @@ test:do_catchsql_test( DROP VIEW IF EXISTS v1err; CREATE VIEW v1err(w,x,y,z) AS SELECT a, b+c, c-b FROM t1; SELECT * FROM v1err; - ]], {1, "expected 4 columns for 'V1ERR' but got 3"}) + ]], {1, "Failed to create space 'V1ERR': number of aliases doesn't match provided columns"}) -- #MUST_WORK_TEST no query solution -- # ifcapable compound { @@ -332,7 +332,7 @@ test:do_catchsql_test( DROP VIEW t1; ]], { -- - 1, "use DROP TABLE to delete table T1" + 1, "Can't drop space 'T1': use DROP TABLE" -- }) @@ -352,7 +352,7 @@ test:do_catchsql_test( DROP TABLE v1; ]], { -- - 1, "use DROP VIEW to delete view V1" + 1, "Can't drop space 'V1': use DROP VIEW" -- }) @@ -372,7 +372,7 @@ test:do_catchsql_test( CREATE INDEX i1v1 ON v1(xyz); ]], { -- - 1, "views can not be indexed" + 1, "Can't create or modify index 'I1V1' in space 'V1': views can not be indexed" -- }) @@ -842,7 +842,7 @@ test:do_catchsql_test( CREATE VIEW v12 AS SELECT a FROM t1 WHERE b=? ]], { -- - 1, "parameters are not allowed in views" + 1, "Failed to create space 'V12': parameters are not allowed in views" -- }) @@ -852,7 +852,7 @@ test:do_catchsql_test( CREATE VIEW v12(x) AS SELECT a FROM t1 WHERE b=? ]], { -- - 1, "parameters are not allowed in views" + 1, "Failed to create space 'V12': parameters are not allowed in views" -- }) diff --git a/test/sql-tap/where7.test.lua b/test/sql-tap/where7.test.lua index 2e6f116..ecd0d24 100755 --- a/test/sql-tap/where7.test.lua +++ b/test/sql-tap/where7.test.lua @@ -325,7 +325,7 @@ test:do_test( end return test:catchsql(sql) end, { - 1, "Expression tree is too large (maximum depth 200)" + 1, "Number of nodes in expression tree 201 exceeds the limit (200)" }) test:do_test( diff --git a/test/sql-tap/whereG.test.lua b/test/sql-tap/whereG.test.lua index 155c906..39424d4 100755 --- a/test/sql-tap/whereG.test.lua +++ b/test/sql-tap/whereG.test.lua @@ -193,7 +193,7 @@ test:do_catchsql_test( AND album.aid=track.aid; ]], { -- - 1, "second argument to likelihood() must be a constant between 0.0 and 1.0" + 1, "Illegal parameters, second argument to likelihood() must be a constant between 0.0 and 1.0" -- }) @@ -207,7 +207,7 @@ test:do_catchsql_test( AND album.aid=track.aid; ]], { -- - 1, "second argument to likelihood() must be a constant between 0.0 and 1.0" + 1, "Illegal parameters, second argument to likelihood() must be a constant between 0.0 and 1.0" -- }) @@ -221,7 +221,7 @@ test:do_catchsql_test( AND album.aid=track.aid; ]], { -- - 1, "second argument to likelihood() must be a constant between 0.0 and 1.0" + 1, "Illegal parameters, second argument to likelihood() must be a constant between 0.0 and 1.0" -- }) diff --git a/test/sql/delete.result b/test/sql/delete.result index e024dd6..29459cc 100644 --- a/test/sql/delete.result +++ b/test/sql/delete.result @@ -93,7 +93,7 @@ box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;") ... box.sql.execute("TRUNCATE TABLE v1;") --- -- error: 'SQL error: can not truncate space ''V1'' because it is a view' +- error: 'SQL error: can not truncate space ''V1'' because space is a view' ... -- Can't truncate table with FK. box.sql.execute("CREATE TABLE t2(x INT PRIMARY KEY REFERENCES t1(id));") diff --git a/test/sql/gh-2347-max-int-literals.result b/test/sql/gh-2347-max-int-literals.result index c289a80..b511440 100644 --- a/test/sql/gh-2347-max-int-literals.result +++ b/test/sql/gh-2347-max-int-literals.result @@ -20,9 +20,11 @@ box.sql.execute("select (-9223372036854775808)") ... box.sql.execute("select (9223372036854775808)") --- -- error: 'oversized integer: 9223372036854775808' +- error: Integer literal 9223372036854775808 exceeds the supported range -9223372036854775808 + - 9223372036854775807 ... box.sql.execute("select (-9223372036854775809)") --- -- error: 'oversized integer: -9223372036854775809' +- error: Integer literal -9223372036854775809 exceeds the supported range -9223372036854775808 + - 9223372036854775807 ... diff --git a/test/sql/gh-2929-primary-key.result b/test/sql/gh-2929-primary-key.result index 4052665..e36f4e4 100644 --- a/test/sql/gh-2929-primary-key.result +++ b/test/sql/gh-2929-primary-key.result @@ -18,19 +18,19 @@ box.sql.execute("CREATE TABLE t1(a INT PRIMARY KEY, b INT UNIQUE)") ... box.sql.execute("CREATE TABLE t2(a INT UNIQUE, b INT)") --- -- error: PRIMARY KEY missing on table T2 +- error: 'Failed to create space ''T2'': PRIMARY KEY missing' ... box.sql.execute("CREATE TABLE t3(a FLOAT)") --- -- error: PRIMARY KEY missing on table T3 +- error: 'Failed to create space ''T3'': PRIMARY KEY missing' ... box.sql.execute("CREATE TABLE t4(a FLOAT, b TEXT)") --- -- error: PRIMARY KEY missing on table T4 +- error: 'Failed to create space ''T4'': PRIMARY KEY missing' ... box.sql.execute("CREATE TABLE t5(a FLOAT, b FLOAT UNIQUE)") --- -- error: PRIMARY KEY missing on table T5 +- error: 'Failed to create space ''T5'': PRIMARY KEY missing' ... box.sql.execute("DROP TABLE t1") --- diff --git a/test/sql/integer-overflow.result b/test/sql/integer-overflow.result index 4754c04..09e864e 100644 --- a/test/sql/integer-overflow.result +++ b/test/sql/integer-overflow.result @@ -30,15 +30,18 @@ box.sql.execute('SELECT (9223372036854775807 + 1);') -- box.sql.execute('SELECT 9223372036854775808;') --- -- error: 'oversized integer: 9223372036854775808' +- error: Integer literal 9223372036854775808 exceeds the supported range -9223372036854775808 + - 9223372036854775807 ... box.sql.execute('SELECT -9223372036854775809;') --- -- error: 'oversized integer: -9223372036854775809' +- error: Integer literal -9223372036854775809 exceeds the supported range -9223372036854775808 + - 9223372036854775807 ... box.sql.execute('SELECT 9223372036854775808 - 1;') --- -- error: 'oversized integer: 9223372036854775808' +- error: Integer literal 9223372036854775808 exceeds the supported range -9223372036854775808 + - 9223372036854775807 ... -- Test that CAST may also leads to overflow. -- diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 938aea9..56099fa 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -363,7 +363,7 @@ sql = 'select '..string.rep('?, ', box.schema.SQL_BIND_PARAMETER_MAX)..'?' ... cn:execute(sql) --- -- error: 'Failed to execute SQL statement: too many SQL variables' +- error: 'Failed to execute SQL statement: SQL bind parameter limit reached: 65000' ... -- Try too many parameter values. sql = 'select ?' @@ -571,8 +571,7 @@ cn:execute('select ?1, ?2, ?3', {1, 2, 3}) ... cn:execute('select $name, $name2', {1, 2}) --- -- error: 'Failed to execute SQL statement: variable number must be between $1 and - $65000' +- error: 'Failed to execute SQL statement: SQL bind parameter limit reached: 65000' ... parameters = {} --- diff --git a/test/sql/transition.result b/test/sql/transition.result index ea110c0..89035e5 100644 --- a/test/sql/transition.result +++ b/test/sql/transition.result @@ -180,7 +180,8 @@ box.sql.execute("DROP TABLE barfoo") -- attempt to create a table lacking PRIMARY KEY box.sql.execute("CREATE TABLE without_rowid_lacking_primary_key(x SCALAR)") --- -- error: PRIMARY KEY missing on table WITHOUT_ROWID_LACKING_PRIMARY_KEY +- error: 'Failed to create space ''WITHOUT_ROWID_LACKING_PRIMARY_KEY'': PRIMARY KEY + missing' ... -- create a table with implicit indices (used to SEGFAULT) box.sql.execute("CREATE TABLE implicit_indices(a INT PRIMARY KEY,b INT,c INT,d TEXT UNIQUE)") diff --git a/test/sql/view.result b/test/sql/view.result index e99a9bd..32967b3 100644 --- a/test/sql/view.result +++ b/test/sql/view.result @@ -19,7 +19,7 @@ box.sql.execute("CREATE VIEW v1 AS SELECT a+b FROM t1;"); -- View can't have any indexes. box.sql.execute("CREATE INDEX i1 on v1(a);"); --- -- error: views can not be indexed +- error: 'Can''t create or modify index ''I1'' in space ''V1'': views can not be indexed' ... v1 = box.space.V1; ---