[tarantool-patches] Re: [PATCH v1 0/4] sql: store regular identifiers in case-normal form

[Kirill Shcherbatov]

> Talking about the patchset - I do not understand a point of
> the first 3 patches. I see, that the only reason why you
> pass the parser is a wish to set nErr. Why instead not to just
> do diag_set, return -1, and check for that in all upper functions,
> until parser is already in one of them? This how all box functions
> work with SQL right now - they do not take Vdbe, nor Parse. They
> just set diag and return -1.

The problem is that when the family of functions calling sql_normalize_name return NULL,
it is assumed that a memory allocation error has occurred, although this is not the case.
The API for working with expressions is heterogeneous, and in some cases already accepts Parser -
for example sqlPExpr, sqlPExprAddSelect.
Moreover, in the same way, to set a third-party error it is used in sql Expr: see sqlExprCheckHeight
that set 
sqlErrorMsg(pParse, "Expression tree is too large (maximum depth %d)", mxHeight);
Thus, I do not introduce new practices to the API, but on the contrary, bring it to a more consistent state.

The alternative that you propose to use the forced setting SQL_TARANTOOL_ERROR (in some cases) three functions.
the stack if the function returned NULL and !db.mallocFailed. This will work, however, for the same sqlPExpr analogy
will be incorrect: it is not true that if the expression construction function returned NULL, a Tarantool error occurred.

This will confuse the already conflicting SQL code.