Fix CREATE OR REPLACE DICTIONARY

src/Common/Config/AbstractConfigurationComparison.cpp

@@ -19,6 +19,56 @@ namespace
         result += subkey;
         return result;
     }
+
+    bool isSameConfigurationImpl(const Poco::Util::AbstractConfiguration & left, const String & left_key,
+                                 const Poco::Util::AbstractConfiguration & right, const String & right_key,
+                                 const std::unordered_set<std::string_view> * ignore_keys)
+    {
+        if (&left == &right && left_key == right_key)
+            return true;
+
+        /// Get the subkeys of the left and right configurations.
+        Poco::Util::AbstractConfiguration::Keys left_subkeys;
+        Poco::Util::AbstractConfiguration::Keys right_subkeys;
+        left.keys(left_key, left_subkeys);
+        right.keys(right_key, right_subkeys);
+
+        if (ignore_keys)
+        {
+            std::erase_if(left_subkeys, [&](const String & key) { return ignore_keys->contains(key); });
+            std::erase_if(right_subkeys, [&](const String & key) { return ignore_keys->contains(key); });
+        }
+
+        /// Check that the right configuration has the same set of subkeys as the left configuration.
+        if (left_subkeys.size() != right_subkeys.size())
+            return false;
+
+        if (left_subkeys.empty())
+        {
+            if (left.hasProperty(left_key))
+            {
+                return right.hasProperty(right_key) && (left.getRawString(left_key) == right.getRawString(right_key));
+            }
+            else
+            {
+                return !right.hasProperty(right_key);
+            }
+        }
+        else
+        {
+            /// Go through all the subkeys and compare corresponding parts of the configurations.
+            std::unordered_set<std::string_view> left_subkeys_set{left_subkeys.begin(), left_subkeys.end()};
+            for (const auto & subkey : right_subkeys)
+            {
+                if (!left_subkeys_set.contains(subkey))
+                    return false;
+
+                if (!isSameConfiguration(left, concatKeyAndSubKey(left_key, subkey), right, concatKeyAndSubKey(right_key, subkey)))
+                    return false;
+            }
+            return true;
+        }
+    }
 }
 
 
@@ -52,41 +102,14 @@ bool isSameConfigurationWithMultipleKeys(const Poco::Util::AbstractConfiguration
 bool isSameConfiguration(const Poco::Util::AbstractConfiguration & left, const String & left_key,
                          const Poco::Util::AbstractConfiguration & right, const String & right_key)
 {
-    if (&left == &right && left_key == right_key)
-        return true;
-
-    bool has_property = left.hasProperty(left_key);
-    if (has_property != right.hasProperty(right_key))
-        return false;
-    if (has_property)
-    {
-        /// The left and right configurations contains values so we can compare them.
-        if (left.getRawString(left_key) != right.getRawString(right_key))
-            return false;
-    }
-
-    /// Get the subkeys of the left and right configurations.
-    Poco::Util::AbstractConfiguration::Keys subkeys;
-    left.keys(left_key, subkeys);
-
-    {
-        /// Check that the right configuration has the same set of subkeys as the left configuration.
-        Poco::Util::AbstractConfiguration::Keys right_subkeys;
-        right.keys(right_key, right_subkeys);
-        std::unordered_set<std::string_view> left_subkeys{subkeys.begin(), subkeys.end()};
-        if ((left_subkeys.size() != right_subkeys.size()) || (left_subkeys.size() != subkeys.size()))
-            return false;
-        for (const auto & right_subkey : right_subkeys)
-            if (!left_subkeys.contains(right_subkey))
-                return false;
-    }
-
-    /// Go through all the subkeys and compare corresponding parts of the configurations.
-    for (const auto & subkey : subkeys)
-        if (!isSameConfiguration(left, concatKeyAndSubKey(left_key, subkey), right, concatKeyAndSubKey(right_key, subkey)))
-            return false;
+    return isSameConfigurationImpl(left, left_key, right, right_key, /* ignore_keys= */ nullptr);
+}
 
-    return true;
+bool isSameConfigurationIgnoringKeys(const Poco::Util::AbstractConfiguration & left, const String & left_key,
+                                     const Poco::Util::AbstractConfiguration & right, const String & right_key,
+                                     const std::unordered_set<std::string_view> & ignore_keys)
+{
+    return isSameConfigurationImpl(left, left_key, right, right_key, !ignore_keys.empty() ? &ignore_keys : nullptr);
 }
 
 }

src/Common/Config/AbstractConfigurationComparison.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <base/types.h>
+#include <unordered_set>
 
 namespace Poco::Util
 {
@@ -33,6 +34,11 @@ namespace DB
     bool isSameConfiguration(const Poco::Util::AbstractConfiguration & left, const String & left_key,
                              const Poco::Util::AbstractConfiguration & right, const String & right_key);
 
+    /// Returns true if specified subviews of the two configurations contains the same keys and values, but without checking specified keys.
+    bool isSameConfigurationIgnoringKeys(const Poco::Util::AbstractConfiguration & left, const String & left_key,
+                                         const Poco::Util::AbstractConfiguration & right, const String & right_key,
+                                         const std::unordered_set<std::string_view> & ignore_keys);
+
     inline bool operator==(const Poco::Util::AbstractConfiguration & left, const Poco::Util::AbstractConfiguration & right)
     {
         return isSameConfiguration(left, right);

src/Common/Config/ConfigHelper.cpp

@@ -1,12 +1,41 @@
 #include <Common/Config/ConfigHelper.h>
 #include <Poco/Util/AbstractConfiguration.h>
+#include <Poco/Util/XMLConfiguration.h>
+
 
 namespace DB
 {
 
 namespace ConfigHelper
 {
 
+namespace
+{
+    void cloneImpl(Poco::Util::AbstractConfiguration & dest, const Poco::Util::AbstractConfiguration & src, const std::string & prefix = "")
+    {
+        std::vector<std::string> keys;
+        src.keys(prefix, keys);
+        if (!keys.empty())
+        {
+            std::string prefix_with_dot = prefix + ".";
+            for (const auto & key : keys)
+                cloneImpl(dest, src, prefix_with_dot + key);
+        }
+        else if (!prefix.empty())
+        {
+            dest.setString(prefix, src.getRawString(prefix));
+        }
+    }
+}
+
+
+Poco::AutoPtr<Poco::Util::AbstractConfiguration> clone(const Poco::Util::AbstractConfiguration & src)
+{
+    Poco::AutoPtr<Poco::Util::AbstractConfiguration> res(new Poco::Util::XMLConfiguration());
+    cloneImpl(*res, src);
+    return res;
+}
+
 bool getBool(const Poco::Util::AbstractConfiguration & config, const std::string & key, bool default_, bool empty_as)
 {
     if (!config.has(key))

src/Common/Config/ConfigHelper.h

@@ -1,18 +1,21 @@
 #pragma once
 
+#include <Poco/AutoPtr.h>
 #include <string>
 
-namespace Poco
+
+namespace Poco::Util
 {
-    namespace Util
-    {
-        class AbstractConfiguration;
-    }
+    class AbstractConfiguration;
 }
 
+
 namespace DB::ConfigHelper
 {
 
+/// Clones a configuration.
+Poco::AutoPtr<Poco::Util::AbstractConfiguration> clone(const Poco::Util::AbstractConfiguration & src);
+
 /// The behavior is like `config.getBool(key, default_)`,
 /// except when the tag is empty (aka. self-closing), `empty_as` will be used instead of throwing Poco::Exception.
 bool getBool(const Poco::Util::AbstractConfiguration & config, const std::string & key, bool default_ = false, bool empty_as = true);

src/Dictionaries/CacheDictionary.h

@@ -98,7 +98,7 @@ class CacheDictionary final : public IDictionary
 
     bool supportUpdates() const override { return false; }
 
-    std::shared_ptr<const IExternalLoadable> clone() const override
+    std::shared_ptr<IExternalLoadable> clone() const override
     {
         return std::make_shared<CacheDictionary>(
                 getDictionaryID(),

src/Dictionaries/DictionaryFactory.cpp

@@ -55,11 +55,7 @@ DictionaryPtr DictionaryFactory::create(
         if (found != registered_layouts.end())
         {
             const auto & layout_creator = found->second.layout_create_function;
-            auto result = layout_creator(name, dict_struct, config, config_prefix, std::move(source_ptr), global_context, created_from_ddl);
-            if (config.hasProperty(config_prefix + ".comment"))
-                result->setDictionaryComment(config.getString(config_prefix + ".comment"));
-
-            return result;
+            return layout_creator(name, dict_struct, config, config_prefix, std::move(source_ptr), global_context, created_from_ddl);
         }
     }
 

src/Dictionaries/DirectDictionary.h

@@ -50,7 +50,7 @@ class DirectDictionary final : public IDictionary
 
     double getLoadFactor() const override { return 0; }
 
-    std::shared_ptr<const IExternalLoadable> clone() const override
+    std::shared_ptr<IExternalLoadable> clone() const override
     {
         return std::make_shared<DirectDictionary>(getDictionaryID(), dict_struct, source_ptr->clone());
     }

src/Dictionaries/FlatDictionary.h

@@ -57,7 +57,7 @@ class FlatDictionary final : public IDictionary
 
     double getLoadFactor() const override { return static_cast<double>(element_count) / bucket_count; }
 
-    std::shared_ptr<const IExternalLoadable> clone() const override
+    std::shared_ptr<IExternalLoadable> clone() const override
     {
         return std::make_shared<FlatDictionary>(getDictionaryID(), dict_struct, source_ptr->clone(), configuration, update_field_loaded_block);
     }

src/Dictionaries/HashedArrayDictionary.h

@@ -73,7 +73,7 @@ class HashedArrayDictionary final : public IDictionary
 
     double getLoadFactor() const override { return static_cast<double>(total_element_count) / bucket_count; }
 
-    std::shared_ptr<const IExternalLoadable> clone() const override
+    std::shared_ptr<IExternalLoadable> clone() const override
     {
         return std::make_shared<HashedArrayDictionary<dictionary_key_type, sharded>>(getDictionaryID(), dict_struct, source_ptr->clone(), configuration, update_field_loaded_block);
     }

src/Dictionaries/HashedDictionary.h

@@ -116,7 +116,7 @@ class HashedDictionary final : public IDictionary
 
     double getLoadFactor() const override { return static_cast<double>(element_count) / bucket_count; }
 
-    std::shared_ptr<const IExternalLoadable> clone() const override
+    std::shared_ptr<IExternalLoadable> clone() const override
     {
         return std::make_shared<HashedDictionary<dictionary_key_type, sparse, sharded>>(
             getDictionaryID(),

src/Dictionaries/IDictionary.h

@@ -75,13 +75,6 @@ class IDictionary : public IExternalLoadable, public IKeyValueEntity
         return dictionary_id;
     }
 
-    void updateDictionaryName(const StorageID & new_name) const
-    {
-        std::lock_guard lock{mutex};
-        assert(new_name.uuid == dictionary_id.uuid && dictionary_id.uuid != UUIDHelpers::Nil);
-        dictionary_id = new_name;
-    }
-
     std::string getLoadableName() const final
     {
         std::lock_guard lock{mutex};
@@ -339,12 +332,6 @@ class IDictionary : public IExternalLoadable, public IKeyValueEntity
         return std::static_pointer_cast<const IDictionary>(IExternalLoadable::shared_from_this());
     }
 
-    void setDictionaryComment(String new_comment)
-    {
-        std::lock_guard lock{mutex};
-        dictionary_comment = std::move(new_comment);
-    }
-
     String getDictionaryComment() const
     {
         std::lock_guard lock{mutex};
@@ -452,9 +439,26 @@ class IDictionary : public IExternalLoadable, public IKeyValueEntity
         return sample_block;
     }
 
+    /// Internally called by ExternalDictionariesLoader.
+    /// In order to update the dictionary ID change its configuration first and then call ExternalDictionariesLoader::reloadConfig().
+    void updateDictionaryID(const StorageID & new_dictionary_id)
+    {
+        std::lock_guard lock{mutex};
+        assert((new_dictionary_id.uuid == dictionary_id.uuid) && (dictionary_id.uuid != UUIDHelpers::Nil));
+        dictionary_id = new_dictionary_id;
+    }
+
+    /// Internally called by ExternalDictionariesLoader.
+    /// In order to update the dictionary comment change its configuration first and then call ExternalDictionariesLoader::reloadConfig().
+    void updateDictionaryComment(const String & new_dictionary_comment)
+    {
+        std::lock_guard lock{mutex};
+        dictionary_comment = new_dictionary_comment;
+    }
+
 private:
     mutable std::mutex mutex;
-    mutable StorageID dictionary_id TSA_GUARDED_BY(mutex);
+    StorageID dictionary_id TSA_GUARDED_BY(mutex);
     String dictionary_comment TSA_GUARDED_BY(mutex);
 };
 

src/Dictionaries/IPAddressDictionary.h

@@ -57,7 +57,7 @@ class IPAddressDictionary final : public IDictionary
 
     double getLoadFactor() const override { return static_cast<double>(element_count) / bucket_count; }
 
-    std::shared_ptr<const IExternalLoadable> clone() const override
+    std::shared_ptr<IExternalLoadable> clone() const override
     {
         return std::make_shared<IPAddressDictionary>(getDictionaryID(), dict_struct, source_ptr->clone(), configuration);
     }

src/Dictionaries/PolygonDictionaryImplementations.cpp

@@ -29,7 +29,7 @@ PolygonDictionarySimple::PolygonDictionarySimple(
 {
 }
 
-std::shared_ptr<const IExternalLoadable> PolygonDictionarySimple::clone() const
+std::shared_ptr<IExternalLoadable> PolygonDictionarySimple::clone() const
 {
     return std::make_shared<PolygonDictionarySimple>(
             this->getDictionaryID(),
@@ -76,7 +76,7 @@ PolygonDictionaryIndexEach::PolygonDictionaryIndexEach(
     }
 }
 
-std::shared_ptr<const IExternalLoadable> PolygonDictionaryIndexEach::clone() const
+std::shared_ptr<IExternalLoadable> PolygonDictionaryIndexEach::clone() const
 {
     return std::make_shared<PolygonDictionaryIndexEach>(
             this->getDictionaryID(),
@@ -126,7 +126,7 @@ PolygonDictionaryIndexCell::PolygonDictionaryIndexCell(
 {
 }
 
-std::shared_ptr<const IExternalLoadable> PolygonDictionaryIndexCell::clone() const
+std::shared_ptr<IExternalLoadable> PolygonDictionaryIndexCell::clone() const
 {
     return std::make_shared<PolygonDictionaryIndexCell>(
             this->getDictionaryID(),

src/Dictionaries/PolygonDictionaryImplementations.h

@@ -23,7 +23,7 @@ class PolygonDictionarySimple : public IPolygonDictionary
             DictionaryLifetime dict_lifetime_,
             Configuration configuration_);
 
-    std::shared_ptr<const IExternalLoadable> clone() const override;
+    std::shared_ptr<IExternalLoadable> clone() const override;
 
 private:
     bool find(const Point & point, size_t & polygon_index) const override;
@@ -47,7 +47,7 @@ class PolygonDictionaryIndexEach : public IPolygonDictionary
             int min_intersections_,
             int max_depth_);
 
-    std::shared_ptr<const IExternalLoadable> clone() const override;
+    std::shared_ptr<IExternalLoadable> clone() const override;
 
     static constexpr size_t kMinIntersectionsDefault = 1;
     static constexpr size_t kMaxDepthDefault = 5;
@@ -75,7 +75,7 @@ class PolygonDictionaryIndexCell : public IPolygonDictionary
             size_t min_intersections_,
             size_t max_depth_);
 
-    std::shared_ptr<const IExternalLoadable> clone() const override;
+    std::shared_ptr<IExternalLoadable> clone() const override;
 
     static constexpr size_t kMinIntersectionsDefault = 1;
     static constexpr size_t kMaxDepthDefault = 5;

src/Dictionaries/RangeHashedDictionary.h

@@ -101,7 +101,7 @@ class RangeHashedDictionary final : public IDictionary
 
     double getLoadFactor() const override { return static_cast<double>(element_count) / bucket_count; }
 
-    std::shared_ptr<const IExternalLoadable> clone() const override
+    std::shared_ptr<IExternalLoadable> clone() const override
     {
         auto result = std::make_shared<RangeHashedDictionary>(
             getDictionaryID(),

src/Dictionaries/RegExpTreeDictionary.h

@@ -86,7 +86,7 @@ class RegExpTreeDictionary : public IDictionary
 
     bool hasHierarchy() const override { return false; }
 
-    std::shared_ptr<const IExternalLoadable> clone() const override
+    std::shared_ptr<IExternalLoadable> clone() const override
     {
         return std::make_shared<RegExpTreeDictionary>(
             getDictionaryID(), structure, source_ptr->clone(), configuration, use_vectorscan, flag_case_insensitive, flag_dotall);

src/Functions/UserDefined/ExternalUserDefinedExecutableFunctionsLoader.cpp

@@ -120,7 +120,7 @@ void ExternalUserDefinedExecutableFunctionsLoader::reloadFunction(const std::str
     loadOrReload(user_defined_function_name);
 }
 
-ExternalLoader::LoadablePtr ExternalUserDefinedExecutableFunctionsLoader::create(const std::string & name,
+ExternalLoader::LoadableMutablePtr ExternalUserDefinedExecutableFunctionsLoader::createObject(const std::string & name,
     const Poco::Util::AbstractConfiguration & config,
     const std::string & key_in_config,
     const std::string &) const