Fix 'APPLY lambda' parsing

[Avogar]

Changelog category (leave one):

• Bug Fix (user-visible misbehaviour in official stable or prestable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix 'APPLY lambda' parsing which could lead to client/server crash.

Detailed description / Documentation draft:
...

[nikitamikhaylov]

Do we need to backport this bugfix?

[Avogar]

Do we need to backport this bugfix?

It reproduces only on 21.10 and 21.11, on 21.9 and lower versions there is no such bug, so I think we should backport it.

[Avogar]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[alexey-milovidov]

We don't expect exceptions in parser, better to return false (syntax error).

[Avogar]

We don't expect exceptions in parser, better to return false (syntax error).

I'm not sure that we should treat such cases as syntax error. You can look at the queries in the test, everything is ok in terms of syntax, instead we can see just the wrong use of function/wrong arguments/wrong type

[alexey-milovidov]

Ok. But AST fuzzer is assuming that parser does not throw exceptions.
Maybe change AST fuzzer?

[Avogar]

Ok. But AST fuzzer is assuming that parser does not throw exceptions.

I checked our code in Parsers folder, there are several places where we can find throwing exceptions (not SYNTAX_ERROR)(ParserCreateQuery.cpp/ParserSelectQuery.cpp/ExpressionElementParsers.cpp). So I guess we just don't have tests for such cases (otherwise AST fuzzer could faild).

Maybe change AST fuzzer?

What do you suggest? Skipping all exceptions sounds bad. Now we skip exceptions with two error codes: SYNTAX_ERROR/TOO_DEEP_RECURSION. I see two variants. We can change all error codes in parsers to SYNTAX_ERROR or we can create the list of error codes that parsers could throw and skip it.

[alexey-milovidov]

Simply change the code to SYNTAX_ERROR and merge this PR 🚀

[Avogar]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated