From 90602b869a37c66ec5ffea4f99522638a9e52704 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Wed, 22 Jul 2020 15:02:47 +0300 Subject: [PATCH] Make SettingsChanges a class. --- src/Access/EnabledSettings.cpp | 1 + src/Access/EnabledSettings.h | 1 - src/Access/SettingsConstraints.cpp | 1 + src/Access/SettingsConstraints.h | 3 +- src/Access/SettingsProfileElement.cpp | 1 + src/Access/SettingsProfileElement.h | 3 +- src/Access/SettingsProfilesCache.cpp | 1 + src/Common/SettingsChanges.cpp | 50 +++++++++++++++++++ src/Common/SettingsChanges.h | 19 ++++--- src/Common/ya.make | 1 + src/Core/SettingsCollection.h | 2 +- .../QueryWithOutputSettingsPushDownVisitor.h | 2 +- src/Storages/IStorage.h | 2 - src/Storages/Kafka/StorageKafka.h | 1 + src/Storages/MergeTree/MergeTreeData.cpp | 24 ++++----- src/Storages/MergeTree/MergeTreeSettings.h | 1 - 16 files changed, 85 insertions(+), 28 deletions(-) create mode 100644 src/Common/SettingsChanges.cpp diff --git a/src/Access/EnabledSettings.cpp b/src/Access/EnabledSettings.cpp index 65e38e4827..f913acb015 100644 --- a/src/Access/EnabledSettings.cpp +++ b/src/Access/EnabledSettings.cpp @@ -1,4 +1,5 @@ #include +#include namespace DB diff --git a/src/Access/EnabledSettings.h b/src/Access/EnabledSettings.h index 8e92298328..cc30e4481f 100644 --- a/src/Access/EnabledSettings.h +++ b/src/Access/EnabledSettings.h @@ -2,7 +2,6 @@ #include #include -#include #include #include #include diff --git a/src/Access/SettingsConstraints.cpp b/src/Access/SettingsConstraints.cpp index 11dbd016e6..8ca2262f8c 100644 --- a/src/Access/SettingsConstraints.cpp +++ b/src/Access/SettingsConstraints.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include diff --git a/src/Access/SettingsConstraints.h b/src/Access/SettingsConstraints.h index 6553725095..f8267b2b47 100644 --- a/src/Access/SettingsConstraints.h +++ b/src/Access/SettingsConstraints.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include @@ -18,6 +17,8 @@ namespace Util namespace DB { struct Settings; +struct SettingChange; +class SettingsChanges; /** Checks if specified changes of settings are allowed or not. * If the changes are not allowed (i.e. violates some constraints) this class throws an exception. diff --git a/src/Access/SettingsProfileElement.cpp b/src/Access/SettingsProfileElement.cpp index d4f6ff5d0f..4fbe4aec2f 100644 --- a/src/Access/SettingsProfileElement.cpp +++ b/src/Access/SettingsProfileElement.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include diff --git a/src/Access/SettingsProfileElement.h b/src/Access/SettingsProfileElement.h index f64317344b..88c8178b42 100644 --- a/src/Access/SettingsProfileElement.h +++ b/src/Access/SettingsProfileElement.h @@ -9,8 +9,7 @@ namespace DB { struct Settings; -struct SettingChange; -using SettingsChanges = std::vector; +class SettingsChanges; class SettingsConstraints; class ASTSettingsProfileElement; class ASTSettingsProfileElements; diff --git a/src/Access/SettingsProfilesCache.cpp b/src/Access/SettingsProfilesCache.cpp index 95074ff7d0..e663ee564a 100644 --- a/src/Access/SettingsProfilesCache.cpp +++ b/src/Access/SettingsProfilesCache.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include diff --git a/src/Common/SettingsChanges.cpp b/src/Common/SettingsChanges.cpp new file mode 100644 index 0000000000..e7c769ad55 --- /dev/null +++ b/src/Common/SettingsChanges.cpp @@ -0,0 +1,50 @@ +#include + + +namespace DB +{ +namespace +{ + SettingChange * find(SettingsChanges & changes, const std::string_view & name) + { + auto it = std::find_if(changes.begin(), changes.end(), [&name](const SettingChange & change) { return change.name == name; }); + if (it == changes.end()) + return nullptr; + return &*it; + } + + const SettingChange * find(const SettingsChanges & changes, const std::string_view & name) + { + auto it = std::find_if(changes.begin(), changes.end(), [&name](const SettingChange & change) { return change.name == name; }); + if (it == changes.end()) + return nullptr; + return &*it; + } +} + +bool SettingsChanges::tryGet(const std::string_view & name, Field & out_value) const +{ + const auto * change = find(*this, name); + if (!change) + return false; + out_value = change->value; + return true; +} + +const Field * SettingsChanges::tryGet(const std::string_view & name) const +{ + const auto * change = find(*this, name); + if (!change) + return nullptr; + return &change->value; +} + +Field * SettingsChanges::tryGet(const std::string_view & name) +{ + auto * change = find(*this, name); + if (!change) + return nullptr; + return &change->value; +} + +} diff --git a/src/Common/SettingsChanges.h b/src/Common/SettingsChanges.h index 004a08c3b4..734d8ecb22 100644 --- a/src/Common/SettingsChanges.h +++ b/src/Common/SettingsChanges.h @@ -5,21 +5,28 @@ namespace DB { - struct SettingChange { String name; Field value; - SettingChange() {} - SettingChange(const String & name_, const Field value_) - : name(name_) - , value(value_) {} + SettingChange() {} + SettingChange(const std::string_view & name_, const Field & value_) : name(name_), value(value_) {} + SettingChange(const std::string_view & name_, Field && value_) : name(name_), value(std::move(value_)) {} friend bool operator ==(const SettingChange & lhs, const SettingChange & rhs) { return (lhs.name == rhs.name) && (lhs.value == rhs.value); } friend bool operator !=(const SettingChange & lhs, const SettingChange & rhs) { return !(lhs == rhs); } }; -using SettingsChanges = std::vector; + +class SettingsChanges : public std::vector +{ +public: + using std::vector::vector; + + bool tryGet(const std::string_view & name, Field & out_value) const; + const Field * tryGet(const std::string_view & name) const; + Field * tryGet(const std::string_view & name); +}; } diff --git a/src/Common/ya.make b/src/Common/ya.make index 32cb55adf4..f1b2ba0470 100644 --- a/src/Common/ya.make +++ b/src/Common/ya.make @@ -75,6 +75,7 @@ SRCS( RWLock.cpp SensitiveDataMasker.cpp setThreadName.cpp + SettingsChanges.cpp SharedLibrary.cpp ShellCommand.cpp StackTrace.cpp diff --git a/src/Core/SettingsCollection.h b/src/Core/SettingsCollection.h index b2fc2a479e..62a3e2764d 100644 --- a/src/Core/SettingsCollection.h +++ b/src/Core/SettingsCollection.h @@ -14,7 +14,7 @@ namespace DB class Field; struct SettingChange; -using SettingsChanges = std::vector; +class SettingsChanges; class ReadBuffer; class WriteBuffer; enum class SettingsBinaryFormat; diff --git a/src/Parsers/QueryWithOutputSettingsPushDownVisitor.h b/src/Parsers/QueryWithOutputSettingsPushDownVisitor.h index 3746d1eb0a..2a7ed0125f 100644 --- a/src/Parsers/QueryWithOutputSettingsPushDownVisitor.h +++ b/src/Parsers/QueryWithOutputSettingsPushDownVisitor.h @@ -8,7 +8,7 @@ namespace DB class ASTSelectQuery; struct SettingChange; -using SettingsChanges = std::vector; +class SettingsChanges; /// Pushdown SETTINGS clause that goes after FORMAT to the SELECT query: /// (since settings after FORMAT parsed separatelly not in the ParserSelectQuery but in ParserQueryWithOutput) diff --git a/src/Storages/IStorage.h b/src/Storages/IStorage.h index 5e91429de3..91c05175f8 100644 --- a/src/Storages/IStorage.h +++ b/src/Storages/IStorage.h @@ -37,8 +37,6 @@ using StorageActionBlockType = size_t; class ASTCreateQuery; struct Settings; -struct SettingChange; -using SettingsChanges = std::vector; class AlterCommands; class MutationCommands; diff --git a/src/Storages/Kafka/StorageKafka.h b/src/Storages/Kafka/StorageKafka.h index b7e6ea2a7e..dd35f8dabb 100644 --- a/src/Storages/Kafka/StorageKafka.h +++ b/src/Storages/Kafka/StorageKafka.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 740d44605e..67a00d7cd0 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -1472,24 +1472,22 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const S const auto & new_changes = new_metadata.settings_changes->as().changes; for (const auto & changed_setting : new_changes) { - if (MergeTreeSettings::findIndex(changed_setting.name) == MergeTreeSettings::npos) - throw Exception{"Storage '" + getName() + "' doesn't have setting '" + changed_setting.name + "'", + const auto & setting_name = changed_setting.name; + const auto & new_value = changed_setting.value; + if (MergeTreeSettings::findIndex(setting_name) == MergeTreeSettings::npos) + throw Exception{"Storage '" + getName() + "' doesn't have setting '" + setting_name + "'", ErrorCodes::UNKNOWN_SETTING}; - auto comparator = [&changed_setting](const auto & change) { return change.name == changed_setting.name; }; + const Field * current_value = current_changes.tryGet(setting_name); - auto current_setting_it - = std::find_if(current_changes.begin(), current_changes.end(), comparator); - - if ((current_setting_it == current_changes.end() || *current_setting_it != changed_setting) - && MergeTreeSettings::isReadonlySetting(changed_setting.name)) + if ((!current_value || *current_value != new_value) + && MergeTreeSettings::isReadonlySetting(setting_name)) { - throw Exception{"Setting '" + changed_setting.name + "' is readonly for storage '" + getName() + "'", + throw Exception{"Setting '" + setting_name + "' is readonly for storage '" + getName() + "'", ErrorCodes::READONLY_SETTING}; } - if (current_setting_it == current_changes.end() - && MergeTreeSettings::isPartFormatSetting(changed_setting.name)) + if (!current_value && MergeTreeSettings::isPartFormatSetting(setting_name)) { MergeTreeSettings copy = *getSettings(); copy.applyChange(changed_setting); @@ -1498,8 +1496,8 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const S throw Exception("Can't change settings. Reason: " + reason, ErrorCodes::NOT_IMPLEMENTED); } - if (changed_setting.name == "storage_policy") - checkStoragePolicy(global_context.getStoragePolicy(changed_setting.value.safeGet())); + if (setting_name == "storage_policy") + checkStoragePolicy(global_context.getStoragePolicy(new_value.safeGet())); } } diff --git a/src/Storages/MergeTree/MergeTreeSettings.h b/src/Storages/MergeTree/MergeTreeSettings.h index 410b8caee6..31436121ad 100644 --- a/src/Storages/MergeTree/MergeTreeSettings.h +++ b/src/Storages/MergeTree/MergeTreeSettings.h @@ -2,7 +2,6 @@ #include #include -#include namespace Poco -- GitLab