[tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[n.pettik]

> On 18 May 2019, at 13:53, imeevma at tarantool.org wrote:
> 
> This patch reworks SQL errors of type "expected column count”.

Please, provide decent description of the patch.

> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index 9c15f33..789556a 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -247,6 +247,7 @@ struct errcode_record {
> 	/*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") \
> 	/*194 */_(ER_MULTIKEY_INDEX_MISMATCH,	"Field %s is used as multikey in one index and as single key in another") \
> +	/*195 */_(ER_SQL_COLUMN_COUNT,		"Left value column count %u does not match the expected column count (%u)") \

-> ER_SQL_ROW_EXPRESSION

I would re-phrase message to smth like this:

“Misuse of row expression: left side of row value has %u entries, but right side - %u”
“Unequal number of entries in row expression: left side has %u, but right side - %u”

> /*
>  * !IMPORTANT! Please follow instructions at start of the file
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index ba7eea5..8cf4d2e 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -2958,28 +2958,22 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
>  * Expr pIn is an IN(...) expression. This function checks that the
>  * sub-select on the RHS of the IN() operator has the same number of
>  * columns as the vector on the LHS. Or, if the RHS of the IN() is not
> - * a sub-query, that the LHS is a vector of size 1.
> + * a sub-query, that the LHS must be a vector of size 1.

What is the point of changing this comment?

>  */
> int
> sqlExprCheckIN(Parse * pParse, Expr * pIn)
> {
> 	int nVector = sqlExprVectorSize(pIn->pLeft);
> -	const char *err;
> 	if ((pIn->flags & EP_xIsSelect)) {
> 		if (nVector != pIn->x.pSelect->pEList->nExpr) {
> -			err = "sub-select returns %d columns - expected %d";
> 			int expr_count = pIn->x.pSelect->pEList->nExpr;
> -			diag_set(ClientError, ER_SQL_PARSER_GENERIC,
> -				 tt_sprintf(err, expr_count, nVector));
> +			diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector,
> +				 expr_count);
> 			pParse->is_aborted = true;
> 			return 1;
> 		}
> 	} else if (nVector != 1) {
> -		assert((pIn->pLeft->flags & EP_xIsSelect) != 0);
> -		int expr_count = pIn->pLeft->x.pSelect->pEList->nExpr;
> -		err = tt_sprintf("sub-select returns %d columns - expected 1",
> -				 expr_count);
> -		diag_set(ClientError, ER_SQL_PARSER_GENERIC, err);
> +		diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector, 1);
> 		pParse->is_aborted = true;
> 		return 1;
> 	}
>