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

Add columns for table system.storage_policies #48167

Merged

Conversation

lzydmxy
Copy link
Contributor

@lzydmxy lzydmxy commented Mar 29, 2023

Changelog category (leave one):

  • Improvement

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

add columns perform_ttl_move_on_insert, load_balancing for table system.storage_policies, modify column volume_type type to Enum8.

@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-improvement Pull request with some product improvements label Mar 29, 2023
@evillique evillique added the can be tested Allows running workflows for external contributors label Mar 29, 2023
@kssenii kssenii self-assigned this Mar 29, 2023
Copy link
Member

@kssenii kssenii left a comment

Choose a reason for hiding this comment

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

Could you please update the integration test test_multiple_disks/test.py::test_system_tables? It fails now
See https://s3.amazonaws.com/clickhouse-test-reports/48167/2232d5ad14513273e9dff7fe096caac140c398f1/integration_tests__asan__[4/6].html
If here you tap on integration_run_parallel3_0.log
you'll see

>       assert sorted(clickhouse_policies_data, key=key) == sorted(
            expected_policies_data, key=key
        )
E       AssertionError: assert 

Or you can make some of the changed fields back to String type to fix the test, I am personally not sure if using Enum is better here.

@lzydmxy
Copy link
Contributor Author

lzydmxy commented Apr 6, 2023

Could you please update the integration test test_multiple_disks/test.py::test_system_tables?

Surely, and i will check other system.storage_policies useges in tests.

Or you can make some of the changed fields back to String type to fix the test, I am personally not sure if using Enum is better here.

I think using Enum is better for column volume_type, but i can change fields back to String type if you want.

@lzydmxy
Copy link
Contributor Author

lzydmxy commented Apr 10, 2023

@kssenii Please review when you have a chance : )

@kssenii kssenii merged commit 65259d9 into ClickHouse:master Apr 10, 2023
145 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can be tested Allows running workflows for external contributors pr-improvement Pull request with some product improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants