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.

Sign up for GitHub

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

clickhouse: allow restoring experimental MergeTree settings #263

Closed
wants to merge 1 commit into from
Closed

clickhouse: allow restoring experimental MergeTree settings #263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions astacus/coordinator/plugins/clickhouse/steps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,8 @@ def _create_dbs(client: ClickHouseClient) -> Iterator[Awaitable[None]]:
b"SET allow_hyperscan=true",
b"SET allow_simdjson=true",
b"SET allow_deprecated_syntax_for_merge_tree=true",
b"SET allow_experimental_block_number_column=true",
b"SET allow_experimental_replacing_merge_with_cleanup=true",
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think this helps. Those are merge tree settings and you are setting them as query level settings. Those settings do not exist and should cause UNKNOWN_SETTING error.

But on the other hand, table settings should have been preserved and should not have caused any error... I do not yet understand it...

Copy link
Contributor

Choose a reason for hiding this comment

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

main conversation here https://aiven-io.slack.com/archives/C052RDBD1TP/p1733428433123549

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, these settings should have been set at the table level.

# If a table was created with flatten_nested=0, we must be careful to not re-create the
# table with flatten_nested=1, since this would recreate the table with a different schema.
# If a table was created with flatten_nested=1, the query in system.tables.create_table_query
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/coordinator/plugins/clickhouse/test_steps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,8 @@ async def test_creates_all_replicated_databases_and_tables_in_manifest() -> None
b"SET allow_hyperscan=true",
b"SET allow_simdjson=true",
b"SET allow_deprecated_syntax_for_merge_tree=true",
b"SET allow_experimental_block_number_column=true",
b"SET allow_experimental_replacing_merge_with_cleanup=true",
b"SET flatten_nested=0",
b"CREATE TABLE db-one.table-uno ...",
b"CREATE TABLE db-one.table-dos ...",
Expand Down Expand Up @@ -964,6 +966,8 @@ async def test_creates_all_replicated_databases_and_tables_in_manifest_with_cust
b"SET allow_hyperscan=true",
b"SET allow_simdjson=true",
b"SET allow_deprecated_syntax_for_merge_tree=true",
b"SET allow_experimental_block_number_column=true",
b"SET allow_experimental_replacing_merge_with_cleanup=true",
b"SET flatten_nested=0",
b"CREATE TABLE db-one.table-uno ...",
b"CREATE TABLE db-one.table-dos ...",
Expand Down Expand Up @@ -1023,6 +1027,8 @@ async def test_drops_each_database_on_all_servers_before_recreating_it() -> None
b"SET allow_hyperscan=true",
b"SET allow_simdjson=true",
b"SET allow_deprecated_syntax_for_merge_tree=true",
b"SET allow_experimental_block_number_column=true",
b"SET allow_experimental_replacing_merge_with_cleanup=true",
b"SET flatten_nested=0",
b"CREATE TABLE db-one.table-uno ...",
b"CREATE TABLE db-one.table-dos ...",
Expand Down