[tarantool-patches] Re: [PATCH v3 6/9] sql: rework six syntax errors

["n.pettik" <korablev@tarantool.org>]

> On 2 Mar 2019, at 16:07, imeevma@tarantool.org wrote:
> 
> This patch rewords six more syntax errors.

Are these errors considered to be syntax?
AFAIR we’ve classified them as semantic errors.
And I counted only three new error (and two changed tests)
codes in errcode.h

> Part of #3965
> ---
> src/box/errcode.h             |  3 +++
> src/box/sql/build.c           |  4 +--
> src/box/sql/resolve.c         | 62 +++++++++++++++++++++----------------------
> test/box/misc.result          |  3 +++
> test/sql-tap/check.test.lua   |  2 +-
> test/sql-tap/colname.test.lua |  2 +-
> 6 files changed, 40 insertions(+), 36 deletions(-)
> 
> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index d234d26..057a6d3 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -239,6 +239,9 @@ struct errcode_record {
> 	/*184 */_(ER_SQL_UNRECOGNIZED_SYNTAX,	"Syntax error near '%.*s'") \
> 	/*185 */_(ER_SQL_UNKNOWN_TOKEN,		"Syntax error: unrecognized token: '%.*s'") \
> 	/*186 */_(ER_SQL_PARSER_GENERIC,	"%s") \
> +	/*187 */_(ER_INDEX_DEF,			"%s prohibited in an index definition") \
> +	/*188 */_(ER_CHECK_CONSTRAINT_DEF,	"%s prohibited in prohibited in a CHECK constraint definition") \

Prohibited in prohibited? I guess this is typo.

> +	/*189 */_(ER_PRIMARY_KEY_DEF,		"Expressions are prohibited in a primary key definition") \

Make errcodes names be more specific:
ER_INDEX_DEF -> ER_INDEX_EXPR_UNSUPPORTED etc.

> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
> index 3215db3..02eca37 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)
> }
> 
> -/*
>  * 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,10 @@ 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, "'.' operator is");
> +				pParse->is_aborted = true;
> +			}

Is this code reachable? I don’t see any tests connected with it.
Please, figure out whether these errors can be raised or not,
and if they can be raised, add corresponding tests.

> test_run:cmd("setopt delimiter ''");
> ---
> diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
> index 0d8bf15..e1d2d48 100755
> --- a/test/sql-tap/check.test.lua
> +++ b/test/sql-tap/check.test.lua
> @@ -319,7 +319,7 @@ test:do_catchsql_test(
>         );
>     ]], {
>         -- <check-3.1>
> -        1, "Failed to create space 'T3': subqueries prohibited in CHECK constraints"
> +        1, "Failed to create space 'T3': Subqueries are prohibited in prohibited in a CHECK constraint definition"
>         -- </check-3.1>
>     })
> 
> 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”}

Ok, I see three new error codes and two changed tests.