Skip to content

Fix function parameters parsing#43350

Merged
evillique merged 1 commit intoClickHouse:masterfrom
evillique:fix-parser-5
Nov 21, 2022
Merged

Fix function parameters parsing#43350
evillique merged 1 commit intoClickHouse:masterfrom
evillique:fix-parser-5

Conversation

@evillique
Copy link
Copy Markdown
Member

@evillique evillique commented Nov 18, 2022

Changelog category (leave one):

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

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fix a bug that allowed FucntionParser to parse an unlimited amount of round brackets into one function if allow_function_parameters is set.

Also, disable allow_function_parameters for table functions in the FROM section of the SELECT query. This fixes #43331

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Nov 18, 2022
@devcrafter devcrafter self-assigned this Nov 18, 2022
Copy link
Copy Markdown
Member

@devcrafter devcrafter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared new and old behavior, - LGTM

In general, I was surprised that we can provide arguments to function as (arg1)(arg2) in addition to usual way (arg1, arg2). Did it come with the new parser, or it's long existing thing?

@evillique
Copy link
Copy Markdown
Member Author

It is a long-existing thing, and actually func(param1, ...)(arg1, ...) is not the same thing as func(param1, ..., arg1, ...), they are two different lists in ASTFunction.

It is usually used in aggregate functions, for example quantileTiming

@evillique
Copy link
Copy Markdown
Member Author

AST fuzzer (tsan): #43202
Other test failures are not related.

@evillique evillique merged commit 65b5523 into ClickHouse:master Nov 21, 2022
@evillique evillique deleted the fix-parser-5 branch November 21, 2022 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logical error: 'Table function 'values' must have arguments'.

3 participants