From 9071ecd428929ead37a6e218ecf5f1f7d82bf071 Mon Sep 17 00:00:00 2001
From: Anton Popov <pad11rus@gmail.com>
Date: Mon, 5 Jul 2021 15:44:58 +0300
Subject: [PATCH] fix alter of settings in MergeTree

---
 src/Storages/MergeTree/MergeTreeData.cpp    | 19 ++++++++-----------
 src/Storages/MergeTree/MergeTreeData.h      |  3 +++
 src/Storages/StorageMergeTree.cpp           |  5 +++++
 src/Storages/StorageMergeTree.h             |  2 ++
 src/Storages/StorageReplicatedMergeTree.cpp |  5 +++++
 src/Storages/StorageReplicatedMergeTree.h   |  2 ++
 6 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp
index 40b37f5afc4..ae3d2220936 100644
--- a/src/Storages/MergeTree/MergeTreeData.cpp
+++ b/src/Storages/MergeTree/MergeTreeData.cpp
@@ -1818,11 +1818,10 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, Context
             if (MergeTreeSettings::isPartFormatSetting(setting_name) && !new_value)
             {
                 /// Use default settings + new and check if doesn't affect part format settings
-                MergeTreeSettings copy = *getSettings();
-                copy.resetToDefault();
-                copy.applyChanges(new_changes);
+                auto copy = getDefaultSettings();
+                copy->applyChanges(new_changes);
                 String reason;
-                if (!canUsePolymorphicParts(copy, &reason) && !reason.empty())
+                if (!canUsePolymorphicParts(*copy, &reason) && !reason.empty())
                     throw Exception("Can't change settings. Reason: " + reason, ErrorCodes::NOT_IMPLEMENTED);
             }
 
@@ -1984,14 +1983,12 @@ void MergeTreeData::changeSettings(
             }
         }
 
-        MergeTreeSettings copy = *getSettings();
-        /// reset to default settings before applying existing
-        copy.resetToDefault();
-        copy.applyChanges(new_changes);
+        /// Reset to default settings before applying existing.
+        auto copy = getDefaultSettings();
+        copy->applyChanges(new_changes);
+        copy->sanityCheck(getContext()->getSettingsRef());
 
-        copy.sanityCheck(getContext()->getSettingsRef());
-
-        storage_settings.set(std::make_unique<const MergeTreeSettings>(copy));
+        storage_settings.set(std::move(copy));
         StorageInMemoryMetadata new_metadata = getInMemoryMetadata();
         new_metadata.setSettingsChanges(new_settings);
         setInMemoryMetadata(new_metadata);
diff --git a/src/Storages/MergeTree/MergeTreeData.h b/src/Storages/MergeTree/MergeTreeData.h
index da8c7fcbb65..a6ece4a7a98 100644
--- a/src/Storages/MergeTree/MergeTreeData.h
+++ b/src/Storages/MergeTree/MergeTreeData.h
@@ -1087,6 +1087,9 @@ private:
 
     // Get partition matcher for FREEZE / UNFREEZE queries.
     MatcherFn getPartitionMatcher(const ASTPtr & partition, ContextPtr context) const;
+
+    /// Returns default settings for storage with possible changes from global config.
+    virtual std::unique_ptr<MergeTreeSettings> getDefaultSettings() const = 0;
 };
 
 /// RAII struct to record big parts that are submerging or emerging.
diff --git a/src/Storages/StorageMergeTree.cpp b/src/Storages/StorageMergeTree.cpp
index 6f8b69ba419..8f387187074 100644
--- a/src/Storages/StorageMergeTree.cpp
+++ b/src/Storages/StorageMergeTree.cpp
@@ -1577,4 +1577,9 @@ void StorageMergeTree::startBackgroundMovesIfNeeded()
         background_moves_executor.start();
 }
 
+std::unique_ptr<MergeTreeSettings> StorageMergeTree::getDefaultSettings() const
+{
+    return std::make_unique<MergeTreeSettings>(getContext()->getMergeTreeSettings());
+}
+
 }
diff --git a/src/Storages/StorageMergeTree.h b/src/Storages/StorageMergeTree.h
index 6678ae06b53..a359de07c07 100644
--- a/src/Storages/StorageMergeTree.h
+++ b/src/Storages/StorageMergeTree.h
@@ -235,6 +235,8 @@ private:
 
     void startBackgroundMovesIfNeeded() override;
 
+    std::unique_ptr<MergeTreeSettings> getDefaultSettings() const override;
+
     friend class MergeTreeProjectionBlockOutputStream;
     friend class MergeTreeBlockOutputStream;
     friend class MergeTreeData;
diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp
index b51b39f7d68..b6a08afa2eb 100644
--- a/src/Storages/StorageReplicatedMergeTree.cpp
+++ b/src/Storages/StorageReplicatedMergeTree.cpp
@@ -7174,6 +7174,11 @@ void StorageReplicatedMergeTree::startBackgroundMovesIfNeeded()
         background_moves_executor.start();
 }
 
+std::unique_ptr<MergeTreeSettings> StorageReplicatedMergeTree::getDefaultSettings() const
+{
+    return std::make_unique<MergeTreeSettings>(getContext()->getReplicatedMergeTreeSettings());
+}
+
 
 void StorageReplicatedMergeTree::lockSharedData(const IMergeTreeDataPart & part) const
 {
diff --git a/src/Storages/StorageReplicatedMergeTree.h b/src/Storages/StorageReplicatedMergeTree.h
index 6f717b7c450..e03255ccd07 100644
--- a/src/Storages/StorageReplicatedMergeTree.h
+++ b/src/Storages/StorageReplicatedMergeTree.h
@@ -702,6 +702,8 @@ private:
 
     void startBackgroundMovesIfNeeded() override;
 
+    std::unique_ptr<MergeTreeSettings> getDefaultSettings() const override;
+
     std::set<String> getPartitionIdsAffectedByCommands(const MutationCommands & commands, ContextPtr query_context) const;
     PartitionBlockNumbersHolder allocateBlockNumbersInAffectedPartitions(
         const MutationCommands & commands, ContextPtr query_context, const zkutil::ZooKeeperPtr & zookeeper) const;