Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not allow SETTINGS after FORMAT for INSERT queries #35883

Merged
merged 2 commits into from
Apr 8, 2022

Conversation

azat
Copy link
Collaborator

@azat azat commented Apr 3, 2022

Changelog category (leave one):

  • Backward Incompatible Change

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Do not allow SETTINGS after FORMAT for INSERT queries (there is compatibility setting parser_settings_after_format_compact to accept such queries, but it is turned OFF by default)

Parsing SETTINGS after FORMAT, that has been introduced in 1, can
interpret SETTING as some values, which is misleading.

Note, that we are touching only INSERT queries, not SELECT, since this
is a backward incompatible change, and in case of modifying SELECT it
can break too much.

Fixes: #35100
Fixes: #20343
Cc: @alexey-milovidov

@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-backward-incompatible Pull request with backwards incompatible changes label Apr 3, 2022
@azat
Copy link
Collaborator Author

azat commented Apr 3, 2022

@vitlibar vitlibar self-assigned this Apr 3, 2022
@azat azat force-pushed the format-settings branch 4 times, most recently from 23572c7 to a3d94c0 Compare April 4, 2022 08:14
@azat azat force-pushed the format-settings branch 2 times, most recently from 223864b to 2611065 Compare April 4, 2022 19:22
@azat
Copy link
Collaborator Author

azat commented Apr 5, 2022

src/Parsers/IParser.h Outdated Show resolved Hide resolved
src/Parsers/ParserQuery.h Outdated Show resolved Hide resolved
src/Parsers/ParserQuery.h Show resolved Hide resolved
azat added 2 commits April 7, 2022 16:29
Parsing SETTINGS after FORMAT, that has been introduced in [1], can
interpret SETTING as some values, which is misleading.

  [1]: https://github.com/ClickHouse/ClickHouse/pull/4174/files#diff-ba7bd0657630b1cd94cf6ed364bd857338096f49f66dc82918438d6745753775R106

Note, that we are touching only INSERT queries, not SELECT, since this
is a backward incompatible change, and in case of modifying SELECT it
can break too much.

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
Fixes: ClickHouse#35100
Fixes: ClickHouse#20343
Add allow_settings_after_format_in_insert setting, OFF by default.

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
v2: s/parser_settings_after_format_compact/allow_settings_after_format_in_insert/ (suggested by vitlibar)
v3: replace ParserSettings with a flag (requested by vitlibar)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-backward-incompatible Pull request with backwards incompatible changes
Projects
None yet
3 participants