From c6f182163ac6db6dd2d122ed752e5412d708c2a0 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 5 Apr 2022 10:57:39 +0300 Subject: [PATCH 1/2] Require mutations for per-table TTL only when it had been changed Before this patch only per-column TTL did not requires mutation if it had not been changed, after per-table TTL will also check whether it had been changed or not. Signed-off-by: Azat Khuzhin --- src/Storages/AlterCommands.cpp | 13 ++++++++--- ...per_table_ttl_mutation_on_change.reference | 22 +++++++++++++++++++ ...02265_per_table_ttl_mutation_on_change.sql | 22 +++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 tests/queries/0_stateless/02265_per_table_ttl_mutation_on_change.reference create mode 100644 tests/queries/0_stateless/02265_per_table_ttl_mutation_on_change.sql diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index edd9dad2c02..5132fc9244c 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -818,22 +818,29 @@ bool AlterCommand::isCommentAlter() const bool AlterCommand::isTTLAlter(const StorageInMemoryMetadata & metadata) const { if (type == MODIFY_TTL) + { + if (!metadata.table_ttl.definition_ast) + return true; + /// If TTL had not been changed, do not require mutations + if (queryToString(metadata.table_ttl.definition_ast) == queryToString(ttl)) + return false; return true; + } if (!ttl || type != MODIFY_COLUMN) return false; - bool ttl_changed = true; + bool column_ttl_changed = true; for (const auto & [name, ttl_ast] : metadata.columns.getColumnTTLs()) { if (name == column_name && queryToString(*ttl) == queryToString(*ttl_ast)) { - ttl_changed = false; + column_ttl_changed = false; break; } } - return ttl_changed; + return column_ttl_changed; } bool AlterCommand::isRemovingProperty() const diff --git a/tests/queries/0_stateless/02265_per_table_ttl_mutation_on_change.reference b/tests/queries/0_stateless/02265_per_table_ttl_mutation_on_change.reference new file mode 100644 index 00000000000..740b4edf189 --- /dev/null +++ b/tests/queries/0_stateless/02265_per_table_ttl_mutation_on_change.reference @@ -0,0 +1,22 @@ +-- { echoOn } +alter table per_table_ttl_02265 modify TTL date + interval 1 month; +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +1 +alter table per_table_ttl_02265 modify TTL date + interval 1 month; +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +1 +alter table per_table_ttl_02265 modify TTL date + interval 2 month; +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +2 +alter table per_table_ttl_02265 modify TTL date + interval 2 month group by key set value = argMax(value, date); +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +3 +alter table per_table_ttl_02265 modify TTL date + interval 2 month group by key set value = argMax(value, date); +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +3 +alter table per_table_ttl_02265 modify TTL date + interval 2 month recompress codec(ZSTD(17)); +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +4 +alter table per_table_ttl_02265 modify TTL date + interval 2 month recompress codec(ZSTD(17)); +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +4 diff --git a/tests/queries/0_stateless/02265_per_table_ttl_mutation_on_change.sql b/tests/queries/0_stateless/02265_per_table_ttl_mutation_on_change.sql new file mode 100644 index 00000000000..53e2e72228a --- /dev/null +++ b/tests/queries/0_stateless/02265_per_table_ttl_mutation_on_change.sql @@ -0,0 +1,22 @@ +drop table if exists per_table_ttl_02265; +create table per_table_ttl_02265 (key Int, date Date, value String) engine=MergeTree() order by key; +insert into per_table_ttl_02265 values (1, today(), '1'); + +-- { echoOn } +alter table per_table_ttl_02265 modify TTL date + interval 1 month; +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +alter table per_table_ttl_02265 modify TTL date + interval 1 month; +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +alter table per_table_ttl_02265 modify TTL date + interval 2 month; +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +alter table per_table_ttl_02265 modify TTL date + interval 2 month group by key set value = argMax(value, date); +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +alter table per_table_ttl_02265 modify TTL date + interval 2 month group by key set value = argMax(value, date); +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +alter table per_table_ttl_02265 modify TTL date + interval 2 month recompress codec(ZSTD(17)); +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; +alter table per_table_ttl_02265 modify TTL date + interval 2 month recompress codec(ZSTD(17)); +select count() from system.mutations where database = currentDatabase() and table = 'per_table_ttl_02265'; + +-- { echoOff } +drop table per_table_ttl_02265; From 8ebaf8498951bb9e866aa4f3ee4764e8146c5013 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 5 Apr 2022 13:39:05 +0300 Subject: [PATCH 2/2] Fix clang-tidy readability-simplify-boolean-expr warning in AlterCommands.cpp I want to make the code more readable before, but clang-tidy is too smart... Signed-off-by: Azat Khuzhin --- src/Storages/AlterCommands.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 5132fc9244c..286f58739f0 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -822,9 +822,7 @@ bool AlterCommand::isTTLAlter(const StorageInMemoryMetadata & metadata) const if (!metadata.table_ttl.definition_ast) return true; /// If TTL had not been changed, do not require mutations - if (queryToString(metadata.table_ttl.definition_ast) == queryToString(ttl)) - return false; - return true; + return queryToString(metadata.table_ttl.definition_ast) != queryToString(ttl); } if (!ttl || type != MODIFY_COLUMN)