Merge pull request #126157 from ClickHouse/register-xml-dictionaries-before-tables

Register XML dictionaries before loading tables at startup

programs/server/Server.cpp

@@ -3649,6 +3649,9 @@ try
         auto & database_catalog = DatabaseCatalog::instance();
         /// We load temporary database first, because projections need it.
         database_catalog.initializeAndLoadTemporaryDatabase();
+        /// The tables (including the ones in the `system` database) can depend on the dictionaries from the configuration files,
+        /// so register them before loading the databases. They start loading later, in `loadOrReloadDictionaries`.
+        global_context->registerDictionariesBeforeLoadingMetadata(config());
         load_system_metadata_tasks = loadMetadataSystem(global_context, server_settings[ServerSetting::async_load_system_database]);
         maybeConvertSystemDatabase(global_context, load_system_metadata_tasks);
 

src/Interpreters/Context.cpp

@@ -4726,16 +4726,37 @@ void Context::tryCreateEmbeddedDictionaries() const
         static_cast<void>(getEmbeddedDictionariesImpl(true));
 }
 
+void Context::registerDictionariesBeforeLoadingMetadata(const Poco::Util::AbstractConfiguration & config)
+{
+    std::lock_guard lock(shared->external_dictionaries_mutex);
+
+    /// The tables are attached before the dictionaries are allowed to load, but they can refer to dictionaries
+    /// declared in configuration files: in a projection, in a column default, or in their loading dependencies.
+    /// So the definitions are registered now, while no dictionary starts loading until `loadOrReloadDictionaries`:
+    /// a dictionary may read from a table which is not attached yet, or from this server, which does not listen yet.
+    auto & external_dictionaries_loader = getExternalDictionariesLoaderWithLock(lock);
+    external_dictionaries_loader.enableAutomaticLoading(false);
+    addDictionariesConfigRepository(config, lock);
+}
+
 void Context::loadOrReloadDictionaries(const Poco::Util::AbstractConfiguration & config)
 {
     bool dictionaries_lazy_load = shared->server_settings[ServerSetting::dictionaries_lazy_load];
-    auto patterns_values = getMultipleValuesFromConfig(config, "", "dictionaries_config");
-    std::unordered_set<std::string> patterns(patterns_values.begin(), patterns_values.end());
 
     std::lock_guard lock(shared->external_dictionaries_mutex);
 
     auto & external_dictionaries_loader = getExternalDictionariesLoaderWithLock(lock);
     external_dictionaries_loader.enableAlwaysLoadEverything(!dictionaries_lazy_load);
+    addDictionariesConfigRepository(config, lock);
+    external_dictionaries_loader.enableAutomaticLoading(true);
+}
+
+void Context::addDictionariesConfigRepository(const Poco::Util::AbstractConfiguration & config, const std::lock_guard<std::mutex> & lock) TSA_REQUIRES(shared->external_dictionaries_mutex)
+{
+    auto patterns_values = getMultipleValuesFromConfig(config, "", "dictionaries_config");
+    std::unordered_set<std::string> patterns(patterns_values.begin(), patterns_values.end());
+
+    auto & external_dictionaries_loader = getExternalDictionariesLoaderWithLock(lock);
 
     if (shared->external_dictionaries_config_repository)
     {

src/Interpreters/Context.h

@@ -1386,6 +1386,8 @@ public:
     const EmbeddedDictionaries & getEmbeddedDictionaries() const;
     EmbeddedDictionaries & getEmbeddedDictionaries();
     void tryCreateEmbeddedDictionaries() const;
+    /// Registers the dictionaries from the configuration files without loading them, so the tables referring to them can be attached.
+    void registerDictionariesBeforeLoadingMetadata(const Poco::Util::AbstractConfiguration & config);
     void loadOrReloadDictionaries(const Poco::Util::AbstractConfiguration & config);
     void waitForDictionariesLoad() const;
 
@@ -2309,6 +2311,7 @@ private:
     void checkMergeTreeSettingsConstraintsWithLock(const MergeTreeSettings & merge_tree_settings, const SettingsChanges & changes) const;
 
     ExternalDictionariesLoader & getExternalDictionariesLoaderWithLock(const std::lock_guard<std::mutex> & lock);
+    void addDictionariesConfigRepository(const Poco::Util::AbstractConfiguration & config, const std::lock_guard<std::mutex> & lock);
 
     ExternalUserDefinedExecutableFunctionsLoader & getExternalUserDefinedExecutableFunctionsLoaderWithLock(const std::lock_guard<std::mutex> & lock);
 

src/Interpreters/ExternalLoader.cpp

@@ -495,7 +495,7 @@ public:
             if (!infos.contains(name))
             {
                 Info & info = infos.emplace(name, Info{name, config}).first->second;
-                if (!isObjectLazy(*config))
+                if (automatic_loading_enabled && !isObjectLazy(*config))
                 {
                     LOG_TRACE(log, "Will load '{}' because it is not loaded lazily.", name);
                     startLoading(info);
@@ -530,13 +530,23 @@ public:
 
         always_load_everything = enable;
 
+        if (enable && automatic_loading_enabled)
+            startLoadingNotLazy();
+    }
+
+    /// Sets whether the objects which are not loaded lazily start loading on their own, when they are added
+    /// to the configuration. When disabled, the objects are registered and can be found, but they are loaded
+    /// only when requested explicitly. When enabled again, the loading of such objects is started.
+    void enableAutomaticLoading(bool enable)
+    {
+        std::lock_guard lock{mutex};
+        if (automatic_loading_enabled == enable)
+            return;
+
+        automatic_loading_enabled = enable;
+
         if (enable)
-        {
-            /// Start loading all the objects which were not loaded yet and are not configured to load lazily.
-            for (auto & [name, info] : infos)
-                if (!info.triedToLoad() && !isObjectLazy(*info.config))
-                    startLoading(info);
-        }
+            startLoadingNotLazy();
     }
 
     /// Sets whether the objects should be loaded asynchronously, each loading in a new thread (from the thread pool).
@@ -769,6 +779,13 @@ public:
     }
 
 private:
+    /// Starts loading all the objects which were not loaded yet and are not configured to load lazily.
+    void startLoadingNotLazy()
+    {
+        for (auto & [name, info] : infos)
+            if (!info.triedToLoad() && !isObjectLazy(*info.config))
+                startLoading(info);
+    }
 
     struct Info
     {
@@ -1323,6 +1340,7 @@ private:
     ObjectConfigsPtr configs;
     std::unordered_map<String, Info> infos;
     std::atomic<bool> always_load_everything = false;
+    std::atomic<bool> automatic_loading_enabled = true;
     std::atomic<bool> enable_async_loading = false;
     std::unordered_map<size_t, ThreadFromGlobalPool> loading_threads;
     std::vector<size_t> recently_finished_loadings;
@@ -1466,6 +1484,11 @@ void ExternalLoader::enableAlwaysLoadEverything(bool enable)
     loading_dispatcher->enableAlwaysLoadEverything(enable);
 }
 
+void ExternalLoader::enableAutomaticLoading(bool enable)
+{
+    loading_dispatcher->enableAutomaticLoading(enable);
+}
+
 void ExternalLoader::enableAsyncLoading(bool enable)
 {
     loading_dispatcher->enableAsyncLoading(enable);

src/Interpreters/ExternalLoader.h

@@ -100,6 +100,10 @@ public:
     /// Sets whether all the objects from the configuration should be always loaded (even those which are never used).
     void enableAlwaysLoadEverything(bool enable);
 
+    /// Sets whether the objects which are not loaded lazily start loading on their own when they are added to the configuration.
+    /// When disabled, the objects are registered, but loaded only on explicit request.
+    void enableAutomaticLoading(bool enable);
+
     /// Sets whether the objects should be loaded asynchronously, each loading in a new thread (from the thread pool).
     void enableAsyncLoading(bool enable);
 

tests/integration/test_dictionaries_dependency_xml/configs/no_async_load_databases.xml

@@ -0,0 +1,4 @@
+<clickhouse>
+    <!-- Attach the tables before the server starts, so a table is never attached after the dictionaries are loaded by chance. -->
+    <async_load_databases>false</async_load_databases>
+</clickhouse>

tests/integration/test_dictionaries_dependency_xml/test.py

@@ -12,7 +12,10 @@ DICTIONARY_FILES = [
 
 cluster = ClickHouseCluster(__file__)
 instance = cluster.add_instance(
-    "instance", dictionaries=DICTIONARY_FILES, stay_alive=True
+    "instance",
+    main_configs=["configs/no_async_load_databases.xml"],
+    dictionaries=DICTIONARY_FILES,
+    stay_alive=True,
 )
 
 
@@ -180,3 +183,27 @@ def test_xml_dict_same_name(started_cluster):
     instance.restart_clickhouse()
     assert "node" in instance.query("show tables from default")
     instance.query("drop table default.node")
+
+
+@pytest.mark.parametrize("database", ["default", "system"])
+def test_projection_with_xml_dict(started_cluster, database):
+    # The dictionaries from the configuration files have to be known when the tables are attached at startup,
+    # otherwise the projection cannot be analyzed and is skipped. The `system` database is loaded before the others.
+    table = f"{database}.prj"
+    instance.query(f"DROP TABLE IF EXISTS {table}")
+    instance.query(
+        f"CREATE TABLE {table} (x UInt64, "
+        "PROJECTION p (SELECT x, dictGet('node', 'name', x % 2) AS v GROUP BY x, v)) "
+        "ENGINE = MergeTree ORDER BY x"
+    )
+    instance.query(f"INSERT INTO {table} SELECT number FROM numbers(10)")
+    instance.restart_clickhouse()
+    assert (
+        instance.query(
+            f"SELECT name FROM system.projections WHERE database = '{database}' AND table = 'prj'"
+        )
+        == "p\n"
+    )
+    instance.query(f"ALTER TABLE {table} ADD COLUMN y UInt64")
+    assert instance.query(f"SELECT count() FROM {table}") == "10\n"
+    instance.query(f"DROP TABLE {table}")