Refactoring of using ExternalLoader in dictionary DDL.

dbms/src/Common/ErrorCodes.cpp

@@ -474,7 +474,8 @@ namespace ErrorCodes
     extern const int NOT_ENOUGH_PRIVILEGES = 497;
     extern const int LIMIT_BY_WITH_TIES_IS_NOT_SUPPORTED = 498;
     extern const int S3_ERROR = 499;
-    extern const int CANNOT_CREATE_DATABASE = 500;
+    extern const int CANNOT_CREATE_DICTIONARY_FROM_METADATA = 500;
+    extern const int CANNOT_CREATE_DATABASE = 501;
 
     extern const int KEEPER_EXCEPTION = 999;
     extern const int POCO_EXCEPTION = 1000;

dbms/src/Databases/DatabaseDictionary.cpp

@@ -102,13 +102,12 @@ void DatabaseDictionary::removeDictionary(
 }
 
 void DatabaseDictionary::attachDictionary(
-    const String & /*dictionary_name*/, const Context & /*context*/, bool /*reload*/)
+    const String & /*dictionary_name*/, const Context & /*context*/)
 {
     throw Exception("Dictionary engine doesn't support dictionaries.", ErrorCodes::UNSUPPORTED_METHOD);
 }
 
-void DatabaseDictionary::detachDictionary(
-    const String & /*dictionary_name*/, const Context & /*context*/, bool /*reload*/)
+void DatabaseDictionary::detachDictionary(const String & /*dictionary_name*/, const Context & /*context*/)
 {
     throw Exception("Dictionary engine doesn't support dictionaries.", ErrorCodes::UNSUPPORTED_METHOD);
 }

dbms/src/Databases/DatabaseDictionary.h

@@ -89,9 +89,9 @@ class DatabaseDictionary : public IDatabase
     ASTPtr tryGetCreateDictionaryQuery(const Context & context, const String & table_name) const override;
 
 
-    void attachDictionary(const String & dictionary_name, const Context & context, bool reload) override;
+    void attachDictionary(const String & dictionary_name, const Context & context) override;
 
-    void detachDictionary(const String & dictionary_name, const Context & context, bool reload) override;
+    void detachDictionary(const String & dictionary_name, const Context & context) override;
 
     void shutdown() override;
 

dbms/src/Databases/DatabaseLazy.cpp

@@ -123,13 +123,12 @@ DatabaseDictionariesIteratorPtr DatabaseLazy::getDictionariesIterator(
 
 void DatabaseLazy::attachDictionary(
     const String & /*dictionary_name*/,
-    const Context & /*context*/,
-    bool /*load*/)
+    const Context & /*context*/)
 {
     throw Exception("Lazy engine can be used only with *Log tables.", ErrorCodes::UNSUPPORTED_METHOD);
 }
 
-void DatabaseLazy::detachDictionary(const String & /*dictionary_name*/, const Context & /*context*/, bool /*reload*/)
+void DatabaseLazy::detachDictionary(const String & /*dictionary_name*/, const Context & /*context*/)
 {
     throw Exception("Lazy engine can be used only with *Log tables.", ErrorCodes::UNSUPPORTED_METHOD);
 }

dbms/src/Databases/DatabaseLazy.h

@@ -111,9 +111,9 @@ class DatabaseLazy : public IDatabase
 
     StoragePtr detachTable(const String & table_name) override;
 
-    void attachDictionary(const String & dictionary_name, const Context & context, bool reload) override;
+    void attachDictionary(const String & dictionary_name, const Context & context) override;
 
-    void detachDictionary(const String & dictionary_name, const Context & context, bool reload) override;
+    void detachDictionary(const String & dictionary_name, const Context & context) override;
 
     void shutdown() override;
 

dbms/src/Databases/DatabaseMemory.cpp

@@ -35,7 +35,7 @@ void DatabaseMemory::createTable(
 }
 
 
-void DatabaseMemory::attachDictionary(const String & /*name*/, const Context & /*context*/, bool /*reload*/)
+void DatabaseMemory::attachDictionary(const String & /*name*/, const Context & /*context*/)
 {
     throw Exception("There is no ATTACH DICTIONARY query for DatabaseMemory", ErrorCodes::UNSUPPORTED_METHOD);
 }
@@ -57,7 +57,7 @@ void DatabaseMemory::removeTable(
 }
 
 
-void DatabaseMemory::detachDictionary(const String & /*name*/, const Context & /*context*/, bool /*reload*/)
+void DatabaseMemory::detachDictionary(const String & /*name*/, const Context & /*context*/)
 {
     throw Exception("There is no DETACH DICTIONARY query for DatabaseMemory", ErrorCodes::UNSUPPORTED_METHOD);
 }

dbms/src/Databases/DatabaseMemory.h

@@ -40,8 +40,7 @@ class DatabaseMemory : public DatabaseWithOwnTablesBase
 
     void attachDictionary(
         const String & name,
-        const Context & context,
-        bool reload) override;
+        const Context & context) override;
 
     void removeTable(
         const Context & context,
@@ -53,8 +52,7 @@ class DatabaseMemory : public DatabaseWithOwnTablesBase
 
     void detachDictionary(
         const String & name,
-        const Context & context,
-        bool reload) override;
+        const Context & context) override;
 
     time_t getObjectMetadataModificationTime(const Context & context, const String & table_name) override;
 

dbms/src/Databases/DatabaseMySQL.h

@@ -62,6 +62,11 @@ class DatabaseMySQL : public IDatabase
 
     void drop() override;
 
+    void detachDictionary(const String &, const Context &) override
+    {
+        throw Exception("MySQL database engine does not support detach dictionary.", ErrorCodes::NOT_IMPLEMENTED);
+    }
+
     String getMetadataPath() const override;
 
     void createTable(const Context &, const String & table_name, const StoragePtr & storage, const ASTPtr & create_query) override;
@@ -74,7 +79,7 @@ class DatabaseMySQL : public IDatabase
 
     void attachTable(const String & table_name, const StoragePtr & storage) override;
 
-    void detachDictionary(const String &, const Context &, bool) override
+    void detachDictionary(const String &, const Context &) override
     {
         throw Exception("MySQL database engine does not support detach dictionary.", ErrorCodes::NOT_IMPLEMENTED);
     }
@@ -84,7 +89,12 @@ class DatabaseMySQL : public IDatabase
         throw Exception("MySQL database engine does not support remove dictionary.", ErrorCodes::NOT_IMPLEMENTED);
     }
 
-    void attachDictionary(const String &, const Context &, bool) override
+    void attachTable(const String &, const StoragePtr &) override
+    {
+        throw Exception("MySQL database engine does not support attach table.", ErrorCodes::NOT_IMPLEMENTED);
+    }
+
+    void attachDictionary(const String &, const Context &) override
     {
         throw Exception("MySQL database engine does not support attach dictionary.", ErrorCodes::NOT_IMPLEMENTED);
     }

dbms/src/Databases/DatabaseOnDisk.cpp

@@ -7,6 +7,7 @@
 #include <Interpreters/Context.h>
 #include <Interpreters/InterpreterCreateQuery.h>
 #include <Interpreters/ExternalDictionariesLoader.h>
+#include <Interpreters/ExternalLoaderPresetConfigRepository.h>
 #include <Dictionaries/getDictionaryConfigurationFromAST.h>
 #include <Parsers/ASTCreateQuery.h>
 #include <Parsers/ParserCreateQuery.h>
@@ -18,6 +19,7 @@
 #include <Common/escapeForFileName.h>
 
 #include <common/logger_useful.h>
+#include <ext/scope_guard.h>
 #include <Poco/DirectoryIterator.h>
 
 
@@ -267,9 +269,11 @@ void DatabaseOnDisk::createDictionary(
 {
     const auto & settings = context.getSettingsRef();
 
-    /** The code is based on the assumption that all threads share the same order of operations
-      * - creating the .sql.tmp file;
-      * - adding a dictionary to `dictionaries`;
+    /** The code is based on the assumption that all threads share the same order of operations:
+      * - create the .sql.tmp file;
+      * - add the dictionary to ExternalDictionariesLoader;
+      * - load the dictionary in case dictionaries_lazy_load == false;
+      * - attach the dictionary;
       * - rename .sql.tmp to .sql.
       */
 
@@ -278,17 +282,20 @@ void DatabaseOnDisk::createDictionary(
     if (database.isDictionaryExist(context, dictionary_name))
         throw Exception("Dictionary " + backQuote(database.getDatabaseName()) + "." + backQuote(dictionary_name) + " already exists.", ErrorCodes::DICTIONARY_ALREADY_EXISTS);
 
+    /// A dictionary with the same full name could be defined in *.xml config files.
+    String full_name = database.getDatabaseName() + "." + dictionary_name;
+    auto & external_loader = const_cast<ExternalDictionariesLoader &>(context.getExternalDictionariesLoader());
+    if (external_loader.getCurrentStatus(full_name) != ExternalLoader::Status::NOT_EXIST)
+        throw Exception("Dictionary " + backQuote(full_name) + " already exists.", ErrorCodes::DICTIONARY_ALREADY_EXISTS);
+
     if (database.isTableExist(context, dictionary_name))
         throw Exception("Table " + backQuote(database.getDatabaseName()) + "." + backQuote(dictionary_name) + " already exists.", ErrorCodes::TABLE_ALREADY_EXISTS);
 
-
     String dictionary_metadata_path = database.getObjectMetadataPath(dictionary_name);
     String dictionary_metadata_tmp_path = dictionary_metadata_path + ".tmp";
-    String statement;
+    String statement = getObjectDefinitionFromCreateQuery(query);
 
     {
-        statement = getObjectDefinitionFromCreateQuery(query);
-
         /// Exclusive flags guarantees, that table is not created right now in another thread. Otherwise, exception will be thrown.
         WriteBufferFromFile out(dictionary_metadata_tmp_path, statement.size(), O_WRONLY | O_CREAT | O_EXCL);
         writeString(statement, out);
@@ -298,27 +305,48 @@ void DatabaseOnDisk::createDictionary(
         out.close();
     }
 
-    try
+    bool succeeded = false;
+    SCOPE_EXIT({
+        if (!succeeded)
+            Poco::File(dictionary_metadata_tmp_path).remove();
+    });
+
+    /// Add a temporary repository containing the dictionary.
+    /// We need this temp repository to try loading the dictionary before actually attaching it to the database.
+    static std::atomic<size_t> counter = 0;
+    String temp_repository_name = String(IExternalLoaderConfigRepository::INTERNAL_REPOSITORY_NAME_PREFIX) + " creating " + full_name + " "
+        + std::to_string(++counter);
+    external_loader.addConfigRepository(
+        temp_repository_name,
+        std::make_unique<ExternalLoaderPresetConfigRepository>(
+            std::vector{std::pair{dictionary_metadata_tmp_path,
+                                  getDictionaryConfigurationFromAST(query->as<const ASTCreateQuery &>(), database.getDatabaseName())}}));
+    SCOPE_EXIT({ external_loader.removeConfigRepository(temp_repository_name); });
+
+    bool lazy_load = context.getConfigRef().getBool("dictionaries_lazy_load", true);
+    if (!lazy_load)
     {
-        /// Do not load it now because we want more strict loading
-        database.attachDictionary(dictionary_name, context, false);
-        /// Load dictionary
-        bool lazy_load = context.getConfigRef().getBool("dictionaries_lazy_load", true);
-        String dict_name = database.getDatabaseName() + "." + dictionary_name;
-        context.getExternalDictionariesLoader().addDictionaryWithConfig(
-            dict_name, database.getDatabaseName(), query->as<const ASTCreateQuery &>(), !lazy_load);
-
-        /// If it was ATTACH query and file with dictionary metadata already exist
-        /// (so, ATTACH is done after DETACH), then rename atomically replaces old file with new one.
-        Poco::File(dictionary_metadata_tmp_path).renameTo(dictionary_metadata_path);
-
-    }
-    catch (...)
-    {
-        database.detachDictionary(dictionary_name, context);
-        Poco::File(dictionary_metadata_tmp_path).remove();
-        throw;
+        /// loadStrict() is called here to force loading the dictionary, wait until the loading is finished,
+        /// and throw an exception if the loading is failed.
+        external_loader.loadStrict(full_name);
     }
+
+    database.attachDictionary(dictionary_name, context);
+    SCOPE_EXIT({
+        if (!succeeded)
+            database.detachDictionary(dictionary_name, context);
+    });
+
+    /// If it was ATTACH query and file with dictionary metadata already exist
+    /// (so, ATTACH is done after DETACH), then rename atomically replaces old file with new one.
+    Poco::File(dictionary_metadata_tmp_path).renameTo(dictionary_metadata_path);
+
+    /// ExternalDictionariesLoader doesn't know we renamed the metadata path.
+    /// So we have to manually call reloadConfig() here.
+    external_loader.reloadConfig(database.getDatabaseName(), full_name);
+
+    /// Everything's ok.
+    succeeded = true;
 }
 
 
@@ -362,16 +390,18 @@ void DatabaseOnDisk::removeDictionary(
     database.detachDictionary(dictionary_name, context);
 
     String dictionary_metadata_path = database.getObjectMetadataPath(dictionary_name);
-
-    try
-    {
-        Poco::File(dictionary_metadata_path).remove();
-    }
-    catch (...)
+    if (Poco::File(dictionary_metadata_path).exists())
     {
-        /// If remove was not possible for some reason
-        database.attachDictionary(dictionary_name, context);
-        throw;
+        try
+        {
+            Poco::File(dictionary_metadata_path).remove();
+        }
+        catch (...)
+        {
+            /// If remove was not possible for some reason
+            database.attachDictionary(dictionary_name, context);
+            throw;
+        }
     }
 }