From b387f05d9cfc5c78efb133dfe42b005a8942c56e Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Thu, 26 Aug 2021 16:19:52 +0300 Subject: [PATCH 01/10] resolve table dependencies on metadata loading --- src/Core/QualifiedTableName.h | 22 +- src/Databases/DDLDependencyVisitor.cpp | 84 ++++++++ src/Databases/DDLDependencyVisitor.h | 35 ++++ src/Databases/DatabaseOnDisk.cpp | 2 +- src/Databases/DatabaseOrdinary.cpp | 194 ++++++++++-------- src/Databases/DatabaseOrdinary.h | 6 + src/Databases/IDatabase.h | 15 ++ src/Databases/TablesLoader.cpp | 153 ++++++++++++++ src/Databases/TablesLoader.h | 63 ++++++ src/Interpreters/DatabaseCatalog.cpp | 9 - src/Interpreters/InterpreterCreateQuery.cpp | 10 +- src/Interpreters/InterpreterCreateQuery.h | 6 + src/Interpreters/loadMetadata.cpp | 9 + .../01160_table_dependencies.reference | 5 + .../0_stateless/01160_table_dependencies.sh | 43 ++++ 15 files changed, 555 insertions(+), 101 deletions(-) create mode 100644 src/Databases/DDLDependencyVisitor.cpp create mode 100644 src/Databases/DDLDependencyVisitor.h create mode 100644 src/Databases/TablesLoader.cpp create mode 100644 src/Databases/TablesLoader.h create mode 100644 tests/queries/0_stateless/01160_table_dependencies.reference create mode 100755 tests/queries/0_stateless/01160_table_dependencies.sh diff --git a/src/Core/QualifiedTableName.h b/src/Core/QualifiedTableName.h index 453d55d85c7..2b48d38ca2f 100644 --- a/src/Core/QualifiedTableName.h +++ b/src/Core/QualifiedTableName.h @@ -3,6 +3,8 @@ #include #include #include +#include +#include namespace DB { @@ -47,5 +49,23 @@ template <> struct hash return qualified_table.hash(); } }; - } + +namespace fmt +{ + template <> + struct formatter + { + constexpr auto parse(format_parse_context & ctx) + { + return ctx.begin(); + } + + template + auto format(const DB::QualifiedTableName & name, FormatContext & ctx) + { + return format_to(ctx.out(), "{}.{}", DB::backQuoteIfNeed(name.database), DB::backQuoteIfNeed(name.table)); + } + }; +} + diff --git a/src/Databases/DDLDependencyVisitor.cpp b/src/Databases/DDLDependencyVisitor.cpp new file mode 100644 index 00000000000..7408e74f012 --- /dev/null +++ b/src/Databases/DDLDependencyVisitor.cpp @@ -0,0 +1,84 @@ +#include +#include +#include +#include +#include + +namespace DB +{ + +void DDLDependencyVisitor::visit(const ASTPtr & ast, Data & data) +{ + if (const auto * function = ast->as()) + visit(*function, data); +} + +bool DDLDependencyVisitor::needChildVisit(const ASTPtr & node, const ASTPtr & /*child*/) +{ + return !node->as(); +} + +void DDLDependencyVisitor::visit(const ASTFunction & function, Data & data) +{ + if (function.name == "joinGet" || + function.name == "dictHas" || + function.name == "dictIsIn" || + function.name.starts_with("dictGet")) + { + extractTableNameFromArgument(function, data, 0); + } +} + +void DDLDependencyVisitor::extractTableNameFromArgument(const ASTFunction & function, Data & data, size_t arg_idx) +{ + /// Just ignore incorrect arguments, proper exception will be thrown later + if (!function.arguments || function.arguments->children.size() <= arg_idx) + return; + + String database_name; + String table_name; + + const auto * arg = function.arguments->as()->children[arg_idx].get(); + if (const auto * literal = arg->as()) + { + if (literal->value.getType() != Field::Types::String) + return; + + String maybe_qualified_name = literal->value.get(); + auto pos = maybe_qualified_name.find('.'); + if (pos == 0 || pos == (maybe_qualified_name.size() - 1)) + { + /// Most likely name is invalid + return; + } + else if (pos == std::string::npos) + { + table_name = std::move(maybe_qualified_name); + } + else + { + database_name = maybe_qualified_name.substr(0, pos); + table_name = maybe_qualified_name.substr(pos + 1); + } + } + else if (const auto * identifier = arg->as()) + { + auto table_identifier = identifier->createTable(); + if (!table_identifier) + return; + + database_name = table_identifier->getDatabaseName(); + table_name = table_identifier->shortName(); + } + else + { + assert(false); + return; + } + + if (database_name.empty()) + database_name = data.default_database; + data.dependencies.push_back(QualifiedTableName{std::move(database_name), std::move(table_name)}); +} + +} diff --git a/src/Databases/DDLDependencyVisitor.h b/src/Databases/DDLDependencyVisitor.h new file mode 100644 index 00000000000..708e0bca66e --- /dev/null +++ b/src/Databases/DDLDependencyVisitor.h @@ -0,0 +1,35 @@ +#pragma once +#include +#include +#include + +namespace DB +{ + +class ASTFunction; + + +class DDLDependencyVisitor +{ +public: + struct Data + { + using TableDependencies = std::vector; + String default_database; + TableDependencies dependencies; + }; + + using Visitor = ConstInDepthNodeVisitor; + + static void visit(const ASTPtr & ast, Data & data); + static bool needChildVisit(const ASTPtr & node, const ASTPtr & child); + +private: + static void visit(const ASTFunction & function, Data & data); + + static void extractTableNameFromArgument(const ASTFunction & function, Data & data, size_t arg_idx); +}; + +using TableLoadingDependenciesVisitor = DDLDependencyVisitor::Visitor; + +} diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index 620e560b64c..dad059a2008 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -608,7 +608,7 @@ void DatabaseOnDisk::iterateMetadataFiles(ContextPtr local_context, const Iterat } /// Read and parse metadata in parallel - ThreadPool pool; + ThreadPool pool{1}; for (const auto & file : metadata_files) { pool.scheduleOrThrowOnError([&]() diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index bfe5de4c95f..f82db868ac8 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -4,6 +4,8 @@ #include #include #include +#include +#include #include #include #include @@ -27,8 +29,6 @@ namespace fs = std::filesystem; namespace DB { -static constexpr size_t PRINT_MESSAGE_EACH_N_OBJECTS = 256; -static constexpr size_t PRINT_MESSAGE_EACH_N_SECONDS = 5; static constexpr size_t METADATA_FILE_BUFFER_SIZE = 32768; namespace @@ -60,15 +60,6 @@ namespace throw; } } - - void logAboutProgress(Poco::Logger * log, size_t processed, size_t total, AtomicStopwatch & watch) - { - if (processed % PRINT_MESSAGE_EACH_N_OBJECTS == 0 || watch.compareAndRestart(PRINT_MESSAGE_EACH_N_SECONDS)) - { - LOG_INFO(log, "{}%", processed * 100.0 / total); - watch.restart(); - } - } } @@ -90,14 +81,82 @@ void DatabaseOrdinary::loadStoredObjects( * Otherwise (for the ext4 filesystem), `DirectoryIterator` iterates through them in some order, * which does not correspond to order tables creation and does not correspond to order of their location on disk. */ - using FileNames = std::map; - std::mutex file_names_mutex; - FileNames file_names; - size_t total_dictionaries = 0; + ParsedTablesMetadata metadata; + loadTablesMetadata(local_context, metadata); - auto process_metadata = [&file_names, &total_dictionaries, &file_names_mutex, this]( - const String & file_name) + size_t total_tables = metadata.metadata.size() - metadata.total_dictionaries; + + AtomicStopwatch watch; + std::atomic dictionaries_processed{0}; + std::atomic tables_processed{0}; + + ThreadPool pool; + + /// We must attach dictionaries before attaching tables + /// because while we're attaching tables we may need to have some dictionaries attached + /// (for example, dictionaries can be used in the default expressions for some tables). + /// On the other hand we can attach any dictionary (even sourced from ClickHouse table) + /// without having any tables attached. It is so because attaching of a dictionary means + /// loading of its config only, it doesn't involve loading the dictionary itself. + + /// Attach dictionaries. + for (const auto & name_with_path_and_query : metadata.metadata) + { + const auto & name = name_with_path_and_query.first; + const auto & path = name_with_path_and_query.second.first; + const auto & ast = name_with_path_and_query.second.second; + const auto & create_query = ast->as(); + + if (create_query.is_dictionary) + { + pool.scheduleOrThrowOnError([&]() + { + loadTableFromMetadata(local_context, path, name, ast, has_force_restore_data_flag); + + /// Messages, so that it's not boring to wait for the server to load for a long time. + logAboutProgress(log, ++dictionaries_processed, metadata.total_dictionaries, watch); + }); + } + } + + pool.wait(); + + /// Attach tables. + for (const auto & name_with_path_and_query : metadata.metadata) + { + const auto & name = name_with_path_and_query.first; + const auto & path = name_with_path_and_query.second.first; + const auto & ast = name_with_path_and_query.second.second; + const auto & create_query = ast->as(); + + if (!create_query.is_dictionary) + { + pool.scheduleOrThrowOnError([&]() + { + loadTableFromMetadata(local_context, path, name, ast, has_force_restore_data_flag); + + /// Messages, so that it's not boring to wait for the server to load for a long time. + logAboutProgress(log, ++tables_processed, total_tables, watch); + }); + } + } + + pool.wait(); + + if (!skip_startup_tables) + { + /// After all tables was basically initialized, startup them. + startupTablesImpl(pool); + } +} + +void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTablesMetadata & metadata) +{ + size_t prev_tables_count = metadata.metadata.size(); + size_t prev_total_dictionaries = metadata.total_dictionaries; + + auto process_metadata = [&metadata, this](const String & file_name) { fs::path path(getMetadataPath()); fs::path file_path(file_name); @@ -122,9 +181,19 @@ void DatabaseOrdinary::loadStoredObjects( return; } - std::lock_guard lock{file_names_mutex}; - file_names[file_name] = ast; - total_dictionaries += create_query->is_dictionary; + TableLoadingDependenciesVisitor::Data data; + data.default_database = metadata.default_database; + TableLoadingDependenciesVisitor visitor{data}; + visitor.visit(ast); + QualifiedTableName qualified_name{database_name, create_query->table}; + + std::lock_guard lock{metadata.mutex}; + metadata.metadata[qualified_name] = std::make_pair(full_path.string(), std::move(ast)); + if (data.dependencies.empty()) + metadata.independent_tables.insert(std::move(qualified_name)); + else + metadata.table_dependencies.insert({std::move(qualified_name), std::move(data.dependencies)}); + metadata.total_dictionaries += create_query->is_dictionary; } } catch (Exception & e) @@ -136,77 +205,28 @@ void DatabaseOrdinary::loadStoredObjects( iterateMetadataFiles(local_context, process_metadata); - size_t total_tables = file_names.size() - total_dictionaries; + size_t objects_in_database = metadata.metadata.size() - prev_tables_count; + size_t dictionaries_in_database = metadata.total_dictionaries - prev_total_dictionaries; + size_t tables_in_database = objects_in_database - dictionaries_in_database; - LOG_INFO(log, "Total {} tables and {} dictionaries.", total_tables, total_dictionaries); + LOG_INFO(log, "Total {} tables and {} dictionaries.", tables_in_database, dictionaries_in_database); +} - AtomicStopwatch watch; - std::atomic tables_processed{0}; +void DatabaseOrdinary::loadTableFromMetadata(ContextMutablePtr local_context, const String & file_path, const QualifiedTableName & name, const ASTPtr & ast, bool force_restore) +{ + assert(name.database == database_name); + const auto & create_query = ast->as(); - ThreadPool pool; + tryAttachTable( + local_context, + create_query, + *this, + name.database, + file_path, + force_restore); - /// We must attach dictionaries before attaching tables - /// because while we're attaching tables we may need to have some dictionaries attached - /// (for example, dictionaries can be used in the default expressions for some tables). - /// On the other hand we can attach any dictionary (even sourced from ClickHouse table) - /// without having any tables attached. It is so because attaching of a dictionary means - /// loading of its config only, it doesn't involve loading the dictionary itself. - - /// Attach dictionaries. - for (const auto & name_with_query : file_names) - { - const auto & create_query = name_with_query.second->as(); - - if (create_query.is_dictionary) - { - pool.scheduleOrThrowOnError([&]() - { - tryAttachTable( - local_context, - create_query, - *this, - database_name, - getMetadataPath() + name_with_query.first, - has_force_restore_data_flag); - - /// Messages, so that it's not boring to wait for the server to load for a long time. - logAboutProgress(log, ++tables_processed, total_tables, watch); - }); - } - } - - pool.wait(); - - /// Attach tables. - for (const auto & name_with_query : file_names) - { - const auto & create_query = name_with_query.second->as(); - - if (!create_query.is_dictionary) - { - pool.scheduleOrThrowOnError([&]() - { - tryAttachTable( - local_context, - create_query, - *this, - database_name, - getMetadataPath() + name_with_query.first, - has_force_restore_data_flag); - - /// Messages, so that it's not boring to wait for the server to load for a long time. - logAboutProgress(log, ++tables_processed, total_tables, watch); - }); - } - } - - pool.wait(); - - if (!skip_startup_tables) - { - /// After all tables was basically initialized, startup them. - startupTablesImpl(pool); - } + /// Messages, so that it's not boring to wait for the server to load for a long time. + //logAboutProgress(log, ++tables_processed, total_tables, watch); } void DatabaseOrdinary::startupTables() diff --git a/src/Databases/DatabaseOrdinary.h b/src/Databases/DatabaseOrdinary.h index 7832377ccae..08ed79ad0ec 100644 --- a/src/Databases/DatabaseOrdinary.h +++ b/src/Databases/DatabaseOrdinary.h @@ -22,6 +22,12 @@ public: void loadStoredObjects(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) override; + bool supportsLoadingInTopologicalOrder() const override { return true; } + + void loadTablesMetadata(ContextPtr context, ParsedTablesMetadata & metadata) override; + + void loadTableFromMetadata(ContextMutablePtr local_context, const String & file_path, const QualifiedTableName & name, const ASTPtr & ast, bool force_restore) override; + void startupTables() override; void alterTable( diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index bd9605dca71..4c9350905c3 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -7,6 +7,8 @@ #include #include +#include //FIXME + #include #include #include @@ -25,6 +27,7 @@ struct StorageInMemoryMetadata; struct StorageID; class ASTCreateQuery; using DictionariesWithID = std::vector>; +struct ParsedTablesMetadata; namespace ErrorCodes { @@ -131,6 +134,18 @@ public: { } + virtual bool supportsLoadingInTopologicalOrder() const { return false; } + + virtual void loadTablesMetadata(ContextPtr /*local_context*/, ParsedTablesMetadata & /*metadata*/) + { + throw Exception(ErrorCodes::LOGICAL_ERROR, "Not implemented"); + } + + virtual void loadTableFromMetadata(ContextMutablePtr /*local_context*/, const String & /*file_path*/, const QualifiedTableName & /*name*/, const ASTPtr & /*ast*/, bool /*force_restore*/) + { + throw Exception(ErrorCodes::LOGICAL_ERROR, "Not implemented"); + } + virtual void startupTables() {} /// Check the existence of the table. diff --git a/src/Databases/TablesLoader.cpp b/src/Databases/TablesLoader.cpp new file mode 100644 index 00000000000..e1d79a0b826 --- /dev/null +++ b/src/Databases/TablesLoader.cpp @@ -0,0 +1,153 @@ +#include + +namespace DB +{ + +static constexpr size_t PRINT_MESSAGE_EACH_N_OBJECTS = 256; +static constexpr size_t PRINT_MESSAGE_EACH_N_SECONDS = 5; + + +void logAboutProgress(Poco::Logger * log, size_t processed, size_t total, AtomicStopwatch & watch) +{ + if (processed % PRINT_MESSAGE_EACH_N_OBJECTS == 0 || watch.compareAndRestart(PRINT_MESSAGE_EACH_N_SECONDS)) + { + LOG_INFO(log, "{}%", processed * 100.0 / total); + watch.restart(); + } +} + +TablesLoader::TablesLoader(ContextMutablePtr global_context_, Databases databases_, bool force_restore_, bool force_attach_) +: global_context(global_context_) +, databases(std::move(databases_)) +, force_restore(force_restore_) +, force_attach(force_attach_) +{ + all_tables.default_database = global_context->getCurrentDatabase(); + log = &Poco::Logger::get("TablesLoader"); +} + + +void TablesLoader::loadTables() +{ + for (auto & database : databases) + { + if (database->supportsLoadingInTopologicalOrder()) + databases_to_load.emplace(database->getDatabaseName(), database); + else + database->loadStoredObjects(global_context, force_restore, force_attach, true); + } + + for (auto & database : databases_to_load) + database.second->loadTablesMetadata(global_context, all_tables); + + auto table_does_not_exist = [&](const QualifiedTableName & table_name, const QualifiedTableName & dependency_name) + { + if (all_tables.metadata.contains(dependency_name)) + return false; + if (DatabaseCatalog::instance().isTableExist(StorageID(dependency_name.database, dependency_name.table), global_context)) + return false; + /// FIXME if XML dict + + LOG_WARNING(log, "Table {} depends on {}, but seems like the second one does not exist", table_name, dependency_name); + return true; + }; + + removeDependencies(table_does_not_exist, all_tables.independent_tables); + + //LOG_TRACE(log, "Independent database objects: {}", fmt::join(all_tables.independent_tables, ", ")); + //for (const auto & dependencies : all_tables.table_dependencies) + // LOG_TRACE(log, "Database object {} depends on: {}", dependencies.first, fmt::join(dependencies.second, ", ")); + + auto is_dependency_loaded = [&](const QualifiedTableName & /*table_name*/, const QualifiedTableName & dependency_name) + { + return all_tables.independent_tables.contains(dependency_name); + }; + + AtomicStopwatch watch; + ThreadPool pool; + size_t level = 0; + do + { + assert(all_tables.metadata.size() == tables_processed + all_tables.independent_tables.size() + all_tables.table_dependencies.size()); + startLoadingIndependentTables(pool, watch, level); + std::unordered_set new_independent_tables; + removeDependencies(is_dependency_loaded, new_independent_tables); + pool.wait(); + all_tables.independent_tables = std::move(new_independent_tables); + checkCyclicDependencies(); + ++level; + assert(all_tables.metadata.size() == tables_processed + all_tables.independent_tables.size() + all_tables.table_dependencies.size()); + } while (!all_tables.independent_tables.empty()); + + for (auto & database : databases_to_load) + { + database.second->startupTables(); + } +} + +void TablesLoader::removeDependencies(RemoveDependencyPredicate need_remove_dependency, std::unordered_set & independent_tables) +{ + auto table_it = all_tables.table_dependencies.begin(); + while (table_it != all_tables.table_dependencies.end()) + { + auto & dependencies = table_it->second; + assert(!dependencies.empty()); + auto dependency_it = dependencies.begin(); + while (dependency_it != dependencies.end()) + { + if (need_remove_dependency(table_it->first, *dependency_it)) + dependency_it = dependencies.erase(dependency_it); + else + ++dependency_it; + } + + if (dependencies.empty()) + { + independent_tables.emplace(std::move(table_it->first)); + table_it = all_tables.table_dependencies.erase(table_it); + } + else + { + ++table_it; + } + } +} + +void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, AtomicStopwatch & watch, size_t level) +{ + size_t total_tables = all_tables.metadata.size(); + + LOG_INFO(log, "Loading {} tables with {} dependency level", all_tables.independent_tables.size(), level); + + for (const auto & table_name : all_tables.independent_tables) + { + pool.scheduleOrThrowOnError([&]() + { + const auto & path_and_query = all_tables.metadata[table_name]; + const auto & path = path_and_query.first; + const auto & ast = path_and_query.second; + databases_to_load[table_name.database]->loadTableFromMetadata(global_context, path, table_name, ast, force_restore); + logAboutProgress(log, ++tables_processed, total_tables, watch); + }); + } +} + +void TablesLoader::checkCyclicDependencies() const +{ + if (!all_tables.independent_tables.empty()) + return; + if (all_tables.table_dependencies.empty()) + return; + + for (const auto & dependencies : all_tables.table_dependencies) + { + LOG_WARNING(log, "Cannot resolve dependencies: Table {} depends on {}", + dependencies.first, fmt::join(dependencies.second, ", ")); + } + + throw Exception(ErrorCodes::INFINITE_LOOP, "Cannot attach {} tables due to cyclic dependencies. " + "See server log for details.", all_tables.table_dependencies.size()); +} + +} + diff --git a/src/Databases/TablesLoader.h b/src/Databases/TablesLoader.h new file mode 100644 index 00000000000..f46929ec179 --- /dev/null +++ b/src/Databases/TablesLoader.h @@ -0,0 +1,63 @@ +#pragma once +#include +#include +#include +#include +#include +#include +#include + +namespace DB +{ + +namespace ErrorCodes +{ + extern const int INFINITE_LOOP; +} + +void logAboutProgress(Poco::Logger * log, size_t processed, size_t total, AtomicStopwatch & watch); + +struct ParsedTablesMetadata +{ + String default_database; + + using ParsedMetadata = std::map>; + std::mutex mutex; + ParsedMetadata metadata; + size_t total_dictionaries = 0; + std::unordered_set independent_tables; + std::unordered_map> table_dependencies; +}; + +class TablesLoader +{ +public: + + using Databases = std::vector; + + TablesLoader(ContextMutablePtr global_context_, Databases databases_, bool force_restore_ = false, bool force_attach_ = false); + + void loadTables(); + +private: + ContextMutablePtr global_context; + Databases databases; + bool force_restore; + bool force_attach; + + std::map databases_to_load; + ParsedTablesMetadata all_tables; + Poco::Logger * log; + std::atomic tables_processed{0}; + + + using RemoveDependencyPredicate = std::function; + void removeDependencies(RemoveDependencyPredicate need_remove_dependency, std::unordered_set & independent_tables); + + void startLoadingIndependentTables(ThreadPool & pool, AtomicStopwatch & watch, size_t level); + + void checkCyclicDependencies() const; + +}; + +} diff --git a/src/Interpreters/DatabaseCatalog.cpp b/src/Interpreters/DatabaseCatalog.cpp index 6e0ca97df1d..99ab3cabd31 100644 --- a/src/Interpreters/DatabaseCatalog.cpp +++ b/src/Interpreters/DatabaseCatalog.cpp @@ -157,15 +157,6 @@ void DatabaseCatalog::loadDatabases() /// Another background thread which drops temporary LiveViews. /// We should start it after loadMarkedAsDroppedTables() to avoid race condition. TemporaryLiveViewCleaner::instance().startup(); - - /// Start up tables after all databases are loaded. - for (const auto & [database_name, database] : databases) - { - if (database_name == DatabaseCatalog::TEMPORARY_DATABASE) - continue; - - database->startupTables(); - } } void DatabaseCatalog::shutdownImpl() diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index 7e061662534..5bddcb9fe1d 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -53,6 +53,7 @@ #include #include #include +#include #include @@ -271,9 +272,12 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create) renamed = true; } - /// We use global context here, because storages lifetime is bigger than query context lifetime - database->loadStoredObjects( - getContext()->getGlobalContext(), has_force_restore_data_flag, create.attach && force_attach, skip_startup_tables); //-V560 + if (!load_database_without_tables) + { + /// We use global context here, because storages lifetime is bigger than query context lifetime + TablesLoader loader{getContext()->getGlobalContext(), {database}, has_force_restore_data_flag, create.attach && force_attach}; + loader.loadTables(); + } } catch (...) { diff --git a/src/Interpreters/InterpreterCreateQuery.h b/src/Interpreters/InterpreterCreateQuery.h index 1ef5e0470fc..47d0e2f492d 100644 --- a/src/Interpreters/InterpreterCreateQuery.h +++ b/src/Interpreters/InterpreterCreateQuery.h @@ -57,6 +57,11 @@ public: skip_startup_tables = skip_startup_tables_; } + void setLoadDatabaseWithoutTables(bool load_database_without_tables_) + { + load_database_without_tables = load_database_without_tables_; + } + /// Obtain information about columns, their types, default values and column comments, /// for case when columns in CREATE query is specified explicitly. static ColumnsDescription getColumnsDescription(const ASTExpressionList & columns, ContextPtr context, bool attach); @@ -100,6 +105,7 @@ private: bool internal = false; bool force_attach = false; bool skip_startup_tables = false; + bool load_database_without_tables = false; mutable String as_database_saved; mutable String as_table_saved; diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index 458e17ac16b..e4c73e7d4e5 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -44,6 +45,7 @@ static void executeCreateQuery( interpreter.setForceAttach(true); interpreter.setForceRestoreData(has_force_restore_data_flag); interpreter.setSkipStartupTables(true); + interpreter.setLoadDatabaseWithoutTables(database != DatabaseCatalog::SYSTEM_DATABASE); interpreter.execute(); } @@ -155,8 +157,15 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam if (create_default_db_if_not_exists && !metadata_dir_for_default_db_already_exists) databases.emplace(default_database_name, path + "/" + escapeForFileName(default_database_name)); + TablesLoader::Databases loaded_databases; for (const auto & [name, db_path] : databases) + { loadDatabase(context, name, db_path, has_force_restore_data_flag); + loaded_databases.emplace_back(DatabaseCatalog::instance().getDatabase(name)); + } + + TablesLoader loader{context, std::move(loaded_databases), has_force_restore_data_flag, /* force_attach */ true}; + loader.loadTables(); if (has_force_restore_data_flag) { diff --git a/tests/queries/0_stateless/01160_table_dependencies.reference b/tests/queries/0_stateless/01160_table_dependencies.reference new file mode 100644 index 00000000000..6691df07cb9 --- /dev/null +++ b/tests/queries/0_stateless/01160_table_dependencies.reference @@ -0,0 +1,5 @@ +dict1 +dict2 +dict_src +join +t diff --git a/tests/queries/0_stateless/01160_table_dependencies.sh b/tests/queries/0_stateless/01160_table_dependencies.sh new file mode 100755 index 00000000000..ecd941a09b1 --- /dev/null +++ b/tests/queries/0_stateless/01160_table_dependencies.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + + +$CLICKHOUSE_CLIENT -q "drop table if exists dict_src;" +$CLICKHOUSE_CLIENT -q "drop dictionary if exists dict1;" +$CLICKHOUSE_CLIENT -q "drop dictionary if exists dict2;" +$CLICKHOUSE_CLIENT -q "drop table if exists join;" +$CLICKHOUSE_CLIENT -q "drop table if exists t;" + +$CLICKHOUSE_CLIENT -q "create table dict_src (n int, m int, s String) engine=MergeTree order by n;" + +$CLICKHOUSE_CLIENT -q "create dictionary dict1 (n int default 0, m int default 1, s String default 'qqq') +PRIMARY KEY n +SOURCE(CLICKHOUSE(HOST 'localhost' PORT tcpPort() USER 'default' TABLE 'dict_src' PASSWORD '' DB '$CLICKHOUSE_DATABASE')) +LIFETIME(MIN 1 MAX 10) LAYOUT(FLAT());" + +$CLICKHOUSE_CLIENT -q "create table join(n int, m int default dictGet('$CLICKHOUSE_DATABASE.dict1', 'm', 42::UInt64)) engine=Join(any, left, n);" + +$CLICKHOUSE_CLIENT -q "create dictionary dict2 (n int default 0, m int DEFAULT 2, s String default 'asd') +PRIMARY KEY n +SOURCE(CLICKHOUSE(HOST 'localhost' PORT tcpPort() USER 'default' TABLE 'join' PASSWORD '' DB '$CLICKHOUSE_DATABASE')) +LIFETIME(MIN 1 MAX 10) LAYOUT(FLAT());" + +$CLICKHOUSE_CLIENT -q "create table t (n int, m int default joinGet($CLICKHOUSE_DATABASE.join, 'm', 42::int), +s String default dictGet($CLICKHOUSE_DATABASE.dict1, 's', 42::UInt64)) engine=MergeTree order by n;" + +CLICKHOUSE_CLIENT_DEFAULT_DB=$(echo ${CLICKHOUSE_CLIENT} | sed 's/'"--database=${CLICKHOUSE_DATABASE}"'/--database=default/g') + +for i in {1..10}; do + $CLICKHOUSE_CLIENT_DEFAULT_DB -q "detach database $CLICKHOUSE_DATABASE;" + $CLICKHOUSE_CLIENT_DEFAULT_DB -q "attach database $CLICKHOUSE_DATABASE;" +done +$CLICKHOUSE_CLIENT -q "show tables from $CLICKHOUSE_DATABASE;" + +$CLICKHOUSE_CLIENT -q "drop table dict_src;" +$CLICKHOUSE_CLIENT -q "drop dictionary dict1;" +$CLICKHOUSE_CLIENT -q "drop dictionary dict2;" +$CLICKHOUSE_CLIENT -q "drop table join;" +$CLICKHOUSE_CLIENT -q "drop table t;" From c8d8f0a38c7748131e07038570ffc793b93f63eb Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Tue, 31 Aug 2021 11:53:48 +0300 Subject: [PATCH 02/10] fix --- src/Databases/DatabaseAtomic.cpp | 59 +++++++++++-------- src/Databases/DatabaseAtomic.h | 4 ++ src/Databases/DatabaseOrdinary.cpp | 15 +---- src/Databases/DatabaseOrdinary.h | 4 +- src/Databases/DatabaseReplicated.cpp | 12 +++- src/Databases/DatabaseReplicated.h | 5 ++ src/Databases/IDatabase.h | 11 +++- .../MySQL/DatabaseMaterializedMySQL.cpp | 6 +- .../MySQL/DatabaseMaterializedMySQL.h | 2 +- .../DatabaseMaterializedPostgreSQL.cpp | 6 +- .../DatabaseMaterializedPostgreSQL.h | 2 +- src/Databases/TablesLoader.cpp | 37 ++++++++---- src/Databases/TablesLoader.h | 25 +++++--- src/Interpreters/InterpreterCreateQuery.cpp | 2 +- .../0_stateless/01160_table_dependencies.sh | 2 +- 15 files changed, 118 insertions(+), 74 deletions(-) diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index 2dbcd652004..83763ccd856 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -416,38 +416,47 @@ UUID DatabaseAtomic::tryGetTableUUID(const String & table_name) const return UUIDHelpers::Nil; } +void DatabaseAtomic::beforeLoadingMetadata(ContextMutablePtr /*context*/, bool has_force_restore_data_flag, bool /*force_attach*/) +{ + if (!has_force_restore_data_flag) + return; + + /// Recreate symlinks to table data dirs in case of force restore, because some of them may be broken + for (const auto & table_path : fs::directory_iterator(path_to_table_symlinks)) + { + if (!fs::is_symlink(table_path)) + { + throw Exception(ErrorCodes::ABORTED, + "'{}' is not a symlink. Atomic database should contains only symlinks.", std::string(table_path.path())); + } + + fs::remove(table_path); + } +} + void DatabaseAtomic::loadStoredObjects( ContextMutablePtr local_context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) { - /// Recreate symlinks to table data dirs in case of force restore, because some of them may be broken - if (has_force_restore_data_flag) - { - for (const auto & table_path : fs::directory_iterator(path_to_table_symlinks)) - { - if (!fs::is_symlink(table_path)) - { - throw Exception(ErrorCodes::ABORTED, - "'{}' is not a symlink. Atomic database should contains only symlinks.", std::string(table_path.path())); - } - - fs::remove(table_path); - } - } - + beforeLoadingMetadata(local_context, has_force_restore_data_flag, force_attach); DatabaseOrdinary::loadStoredObjects(local_context, has_force_restore_data_flag, force_attach, skip_startup_tables); +} - if (has_force_restore_data_flag) +void DatabaseAtomic::startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) +{ + DatabaseOrdinary::startupTables(thread_pool, force_restore, force_attach); + + if (!force_restore) + return; + + NameToPathMap table_names; { - NameToPathMap table_names; - { - std::lock_guard lock{mutex}; - table_names = table_name_to_path; - } - - fs::create_directories(path_to_table_symlinks); - for (const auto & table : table_names) - tryCreateSymlink(table.first, table.second, true); + std::lock_guard lock{mutex}; + table_names = table_name_to_path; } + + fs::create_directories(path_to_table_symlinks); + for (const auto & table : table_names) + tryCreateSymlink(table.first, table.second, true); } void DatabaseAtomic::tryCreateSymlink(const String & table_name, const String & actual_data_path, bool if_data_path_exist) diff --git a/src/Databases/DatabaseAtomic.h b/src/Databases/DatabaseAtomic.h index 8be009cd6ca..db9cef4dbc6 100644 --- a/src/Databases/DatabaseAtomic.h +++ b/src/Databases/DatabaseAtomic.h @@ -49,6 +49,10 @@ public: void loadStoredObjects(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) override; + void beforeLoadingMetadata(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach) override; + + void startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) override; + /// Atomic database cannot be detached if there is detached table which still in use void assertCanBeDetached(bool cleanup) override; diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index f82db868ac8..567bf8726e3 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -75,7 +75,7 @@ DatabaseOrdinary::DatabaseOrdinary( } void DatabaseOrdinary::loadStoredObjects( - ContextMutablePtr local_context, bool has_force_restore_data_flag, bool /*force_attach*/, bool skip_startup_tables) + ContextMutablePtr local_context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) { /** Tables load faster if they are loaded in sorted (by name) order. * Otherwise (for the ext4 filesystem), `DirectoryIterator` iterates through them in some order, @@ -147,7 +147,7 @@ void DatabaseOrdinary::loadStoredObjects( if (!skip_startup_tables) { /// After all tables was basically initialized, startup them. - startupTablesImpl(pool); + startupTables(pool, has_force_restore_data_flag, force_attach); } } @@ -224,18 +224,9 @@ void DatabaseOrdinary::loadTableFromMetadata(ContextMutablePtr local_context, co name.database, file_path, force_restore); - - /// Messages, so that it's not boring to wait for the server to load for a long time. - //logAboutProgress(log, ++tables_processed, total_tables, watch); } -void DatabaseOrdinary::startupTables() -{ - ThreadPool pool; - startupTablesImpl(pool); -} - -void DatabaseOrdinary::startupTablesImpl(ThreadPool & thread_pool) +void DatabaseOrdinary::startupTables(ThreadPool & thread_pool, bool /*force_restore*/, bool /*force_attach*/) { LOG_INFO(log, "Starting up tables."); diff --git a/src/Databases/DatabaseOrdinary.h b/src/Databases/DatabaseOrdinary.h index 08ed79ad0ec..3f300bfb3eb 100644 --- a/src/Databases/DatabaseOrdinary.h +++ b/src/Databases/DatabaseOrdinary.h @@ -28,7 +28,7 @@ public: void loadTableFromMetadata(ContextMutablePtr local_context, const String & file_path, const QualifiedTableName & name, const ASTPtr & ast, bool force_restore) override; - void startupTables() override; + void startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) override; void alterTable( ContextPtr context, @@ -42,8 +42,6 @@ protected: const String & table_metadata_path, const String & statement, ContextPtr query_context); - - void startupTablesImpl(ThreadPool & thread_pool); }; } diff --git a/src/Databases/DatabaseReplicated.cpp b/src/Databases/DatabaseReplicated.cpp index da03eb6aba6..9aebc701aa9 100644 --- a/src/Databases/DatabaseReplicated.cpp +++ b/src/Databases/DatabaseReplicated.cpp @@ -305,13 +305,21 @@ void DatabaseReplicated::createReplicaNodesInZooKeeper(const zkutil::ZooKeeperPt createEmptyLogEntry(current_zookeeper); } +void DatabaseReplicated::beforeLoadingMetadata(ContextMutablePtr /*context*/, bool /*has_force_restore_data_flag*/, bool force_attach) +{ + tryConnectToZooKeeperAndInitDatabase(force_attach); +} + void DatabaseReplicated::loadStoredObjects( ContextMutablePtr local_context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) { - tryConnectToZooKeeperAndInitDatabase(force_attach); - + beforeLoadingMetadata(local_context, has_force_restore_data_flag, force_attach); DatabaseAtomic::loadStoredObjects(local_context, has_force_restore_data_flag, force_attach, skip_startup_tables); +} +void DatabaseReplicated::startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) +{ + DatabaseAtomic::startupTables(thread_pool, force_restore, force_attach); ddl_worker = std::make_unique(this, getContext()); ddl_worker->startup(); } diff --git a/src/Databases/DatabaseReplicated.h b/src/Databases/DatabaseReplicated.h index 1e0daeed07e..daba7dad17b 100644 --- a/src/Databases/DatabaseReplicated.h +++ b/src/Databases/DatabaseReplicated.h @@ -58,6 +58,11 @@ public: void drop(ContextPtr /*context*/) override; void loadStoredObjects(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) override; + + void beforeLoadingMetadata(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach) override; + + void startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) override; + void shutdown() override; friend struct DatabaseReplicatedTask; diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index 4c9350905c3..dc8c24e0bcc 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include //FIXME @@ -33,6 +34,7 @@ namespace ErrorCodes { extern const int NOT_IMPLEMENTED; extern const int CANNOT_GET_CREATE_TABLE_QUERY; + extern const int LOGICAL_ERROR; } class IDatabaseTablesIterator @@ -136,6 +138,13 @@ public: virtual bool supportsLoadingInTopologicalOrder() const { return false; } + virtual void beforeLoadingMetadata( + ContextMutablePtr /*context*/, + bool /*has_force_restore_data_flag*/, + bool /*force_attach*/) + { + } + virtual void loadTablesMetadata(ContextPtr /*local_context*/, ParsedTablesMetadata & /*metadata*/) { throw Exception(ErrorCodes::LOGICAL_ERROR, "Not implemented"); @@ -146,7 +155,7 @@ public: throw Exception(ErrorCodes::LOGICAL_ERROR, "Not implemented"); } - virtual void startupTables() {} + virtual void startupTables(ThreadPool & /*thread_pool*/, bool /*force_restore*/, bool /*force_attach*/) {} /// Check the existence of the table. virtual bool isTableExist(const String & name, ContextPtr context) const = 0; diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp index 0d81a4e1a98..87ec461026e 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp @@ -94,10 +94,10 @@ void DatabaseMaterializedMySQL::setException(const std::exception_ptr & ex } template -void DatabaseMaterializedMySQL::loadStoredObjects( - ContextMutablePtr context_, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) +void DatabaseMaterializedMySQL::startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) { - Base::loadStoredObjects(context_, has_force_restore_data_flag, force_attach, skip_startup_tables); + Base::startupTables(thread_pool, force_attach, force_restore); + if (!force_attach) materialize_thread.assertMySQLAvailable(); diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.h b/src/Databases/MySQL/DatabaseMaterializedMySQL.h index 292edc97878..ac32607a22c 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.h +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.h @@ -43,7 +43,7 @@ protected: public: String getEngineName() const override { return "MaterializedMySQL"; } - void loadStoredObjects(ContextMutablePtr context_, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) override; + void startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) override; void createTable(ContextPtr context_, const String & name, const StoragePtr & table, const ASTPtr & query) override; diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index c9ea8d12ef2..3e0d8e1d300 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -109,11 +109,9 @@ void DatabaseMaterializedPostgreSQL::startSynchronization() } -void DatabaseMaterializedPostgreSQL::loadStoredObjects( - ContextMutablePtr local_context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) +void DatabaseMaterializedPostgreSQL::startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) { - DatabaseAtomic::loadStoredObjects(local_context, has_force_restore_data_flag, force_attach, skip_startup_tables); - + DatabaseAtomic::startupTables(thread_pool, force_restore, force_attach); try { startSynchronization(); diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h index 915bf44f1f2..c5b3c9fcede 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h @@ -43,7 +43,7 @@ public: String getMetadataPath() const override { return metadata_path; } - void loadStoredObjects(ContextMutablePtr, bool, bool force_attach, bool skip_startup_tables) override; + void startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) override; DatabaseTablesIteratorPtr getTablesIterator(ContextPtr context, const DatabaseOnDisk::FilterByNameFunction & filter_by_table_name) const override; diff --git a/src/Databases/TablesLoader.cpp b/src/Databases/TablesLoader.cpp index e1d79a0b826..ded359790cb 100644 --- a/src/Databases/TablesLoader.cpp +++ b/src/Databases/TablesLoader.cpp @@ -1,8 +1,20 @@ #include +#include +#include +#include +#include +#include +#include +#include namespace DB { +namespace ErrorCodes +{ + extern const int INFINITE_LOOP; +} + static constexpr size_t PRINT_MESSAGE_EACH_N_OBJECTS = 256; static constexpr size_t PRINT_MESSAGE_EACH_N_SECONDS = 5; @@ -29,19 +41,23 @@ TablesLoader::TablesLoader(ContextMutablePtr global_context_, Databases database void TablesLoader::loadTables() { + bool need_resolve_dependencies = !global_context->getConfigRef().has("ignore_table_dependencies_on_metadata_loading"); for (auto & database : databases) { - if (database->supportsLoadingInTopologicalOrder()) + if (need_resolve_dependencies && database->supportsLoadingInTopologicalOrder()) databases_to_load.emplace(database->getDatabaseName(), database); else database->loadStoredObjects(global_context, force_restore, force_attach, true); } for (auto & database : databases_to_load) + { + database.second->beforeLoadingMetadata(global_context, force_restore, force_attach); database.second->loadTablesMetadata(global_context, all_tables); + } - auto table_does_not_exist = [&](const QualifiedTableName & table_name, const QualifiedTableName & dependency_name) - { + auto table_does_not_exist = [this](const QualifiedTableName & table_name, const QualifiedTableName & dependency_name) + { if (all_tables.metadata.contains(dependency_name)) return false; if (DatabaseCatalog::instance().isTableExist(StorageID(dependency_name.database, dependency_name.table), global_context)) @@ -50,7 +66,7 @@ void TablesLoader::loadTables() LOG_WARNING(log, "Table {} depends on {}, but seems like the second one does not exist", table_name, dependency_name); return true; - }; + }; removeDependencies(table_does_not_exist, all_tables.independent_tables); @@ -58,10 +74,10 @@ void TablesLoader::loadTables() //for (const auto & dependencies : all_tables.table_dependencies) // LOG_TRACE(log, "Database object {} depends on: {}", dependencies.first, fmt::join(dependencies.second, ", ")); - auto is_dependency_loaded = [&](const QualifiedTableName & /*table_name*/, const QualifiedTableName & dependency_name) - { + auto is_dependency_loaded = [this](const QualifiedTableName & /*table_name*/, const QualifiedTableName & dependency_name) + { return all_tables.independent_tables.contains(dependency_name); - }; + }; AtomicStopwatch watch; ThreadPool pool; @@ -81,7 +97,7 @@ void TablesLoader::loadTables() for (auto & database : databases_to_load) { - database.second->startupTables(); + database.second->startupTables(pool, force_restore, force_attach); } } @@ -121,7 +137,7 @@ void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, AtomicStopwa for (const auto & table_name : all_tables.independent_tables) { - pool.scheduleOrThrowOnError([&]() + pool.scheduleOrThrowOnError([this, total_tables, &table_name, &watch]() { const auto & path_and_query = all_tables.metadata[table_name]; const auto & path = path_and_query.first; @@ -141,8 +157,7 @@ void TablesLoader::checkCyclicDependencies() const for (const auto & dependencies : all_tables.table_dependencies) { - LOG_WARNING(log, "Cannot resolve dependencies: Table {} depends on {}", - dependencies.first, fmt::join(dependencies.second, ", ")); + LOG_WARNING(log, "Cannot resolve dependencies: Table {} depends on {}", dependencies.first, fmt::join(dependencies.second, ", ")); } throw Exception(ErrorCodes::INFINITE_LOOP, "Cannot attach {} tables due to cyclic dependencies. " diff --git a/src/Databases/TablesLoader.h b/src/Databases/TablesLoader.h index f46929ec179..fface310bb6 100644 --- a/src/Databases/TablesLoader.h +++ b/src/Databases/TablesLoader.h @@ -1,19 +1,26 @@ #pragma once -#include -#include -#include +#include #include -#include -#include +#include +#include #include +#include +#include +#include +#include + +namespace Poco +{ + class Logger; +} + +class AtomicStopwatch; namespace DB { -namespace ErrorCodes -{ - extern const int INFINITE_LOOP; -} +class IDatabase; +using DatabasePtr = std::shared_ptr; void logAboutProgress(Poco::Logger * log, size_t processed, size_t total, AtomicStopwatch & watch); diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index 5bddcb9fe1d..d885b9a2ac5 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -275,7 +275,7 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create) if (!load_database_without_tables) { /// We use global context here, because storages lifetime is bigger than query context lifetime - TablesLoader loader{getContext()->getGlobalContext(), {database}, has_force_restore_data_flag, create.attach && force_attach}; + TablesLoader loader{getContext()->getGlobalContext(), {database}, has_force_restore_data_flag, create.attach && force_attach}; //-V560 loader.loadTables(); } } diff --git a/tests/queries/0_stateless/01160_table_dependencies.sh b/tests/queries/0_stateless/01160_table_dependencies.sh index ecd941a09b1..0ea213ba5ff 100755 --- a/tests/queries/0_stateless/01160_table_dependencies.sh +++ b/tests/queries/0_stateless/01160_table_dependencies.sh @@ -30,7 +30,7 @@ s String default dictGet($CLICKHOUSE_DATABASE.dict1, 's', 42::UInt64)) engine=Me CLICKHOUSE_CLIENT_DEFAULT_DB=$(echo ${CLICKHOUSE_CLIENT} | sed 's/'"--database=${CLICKHOUSE_DATABASE}"'/--database=default/g') -for i in {1..10}; do +for _ in {1..10}; do $CLICKHOUSE_CLIENT_DEFAULT_DB -q "detach database $CLICKHOUSE_DATABASE;" $CLICKHOUSE_CLIENT_DEFAULT_DB -q "attach database $CLICKHOUSE_DATABASE;" done From 024a24aaf781598fe5b42abde8ff244b5f71c696 Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Wed, 1 Sep 2021 22:42:49 +0300 Subject: [PATCH 03/10] better code, moar logging --- programs/local/LocalServer.cpp | 1 + programs/server/Server.cpp | 1 + src/Databases/DDLDependencyVisitor.cpp | 2 +- src/Databases/DDLDependencyVisitor.h | 4 +- src/Databases/DatabaseOnDisk.cpp | 2 +- src/Databases/DatabaseOrdinary.cpp | 12 +- .../MySQL/DatabaseMaterializedMySQL.cpp | 2 +- src/Databases/TablesLoader.cpp | 203 ++++++++++++------ src/Databases/TablesLoader.h | 56 ++++- .../ExternalDictionariesLoader.cpp | 4 + src/Interpreters/ExternalDictionariesLoader.h | 2 + src/Interpreters/ExternalLoader.cpp | 2 + src/Interpreters/InterpreterCreateQuery.cpp | 3 +- src/Interpreters/InterpreterCreateQuery.h | 6 - src/Interpreters/loadMetadata.cpp | 17 +- src/Interpreters/loadMetadata.h | 3 + 16 files changed, 230 insertions(+), 90 deletions(-) diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index 2b1b6185321..284fbc9f66c 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -303,6 +303,7 @@ try loadMetadataSystem(global_context); attachSystemTables(global_context); loadMetadata(global_context); + startupSystemTables(); DatabaseCatalog::instance().loadDatabases(); LOG_DEBUG(log, "Loaded metadata."); } diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index ddbc4c4e433..1371d36ce5c 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -1127,6 +1127,7 @@ if (ThreadFuzzer::instance().isEffective()) attachSystemTablesServer(*database_catalog.getSystemDatabase(), has_zookeeper); /// Then, load remaining databases loadMetadata(global_context, default_database); + startupSystemTables(); database_catalog.loadDatabases(); /// After loading validate that default database exists database_catalog.assertDatabaseExists(default_database); diff --git a/src/Databases/DDLDependencyVisitor.cpp b/src/Databases/DDLDependencyVisitor.cpp index 7408e74f012..e11d4739604 100644 --- a/src/Databases/DDLDependencyVisitor.cpp +++ b/src/Databases/DDLDependencyVisitor.cpp @@ -78,7 +78,7 @@ void DDLDependencyVisitor::extractTableNameFromArgument(const ASTFunction & func if (database_name.empty()) database_name = data.default_database; - data.dependencies.push_back(QualifiedTableName{std::move(database_name), std::move(table_name)}); + data.dependencies.emplace(QualifiedTableName{std::move(database_name), std::move(table_name)}); } } diff --git a/src/Databases/DDLDependencyVisitor.h b/src/Databases/DDLDependencyVisitor.h index 708e0bca66e..43dbbef9e25 100644 --- a/src/Databases/DDLDependencyVisitor.h +++ b/src/Databases/DDLDependencyVisitor.h @@ -14,9 +14,9 @@ class DDLDependencyVisitor public: struct Data { - using TableDependencies = std::vector; + using TableNamesSet = std::set; String default_database; - TableDependencies dependencies; + TableNamesSet dependencies; }; using Visitor = ConstInDepthNodeVisitor; diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index dad059a2008..620e560b64c 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -608,7 +608,7 @@ void DatabaseOnDisk::iterateMetadataFiles(ContextPtr local_context, const Iterat } /// Read and parse metadata in parallel - ThreadPool pool{1}; + ThreadPool pool; for (const auto & file : metadata_files) { pool.scheduleOrThrowOnError([&]() diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 567bf8726e3..77bde83aa65 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -190,9 +190,17 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables std::lock_guard lock{metadata.mutex}; metadata.metadata[qualified_name] = std::make_pair(full_path.string(), std::move(ast)); if (data.dependencies.empty()) - metadata.independent_tables.insert(std::move(qualified_name)); + { + metadata.independent_tables.emplace_back(std::move(qualified_name)); + } else - metadata.table_dependencies.insert({std::move(qualified_name), std::move(data.dependencies)}); + { + for (const auto & dependency : data.dependencies) + { + metadata.dependencies_info[dependency].dependent_tables.push_back(qualified_name); + ++metadata.dependencies_info[qualified_name].dependencies_count; + } + } metadata.total_dictionaries += create_query->is_dictionary; } } diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp index 87ec461026e..2b4649c275a 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp @@ -96,7 +96,7 @@ void DatabaseMaterializedMySQL::setException(const std::exception_ptr & ex template void DatabaseMaterializedMySQL::startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) { - Base::startupTables(thread_pool, force_attach, force_restore); + Base::startupTables(thread_pool, force_restore, force_attach); if (!force_attach) materialize_thread.assertMySQLAvailable(); diff --git a/src/Databases/TablesLoader.cpp b/src/Databases/TablesLoader.cpp index ded359790cb..4aa8f422043 100644 --- a/src/Databases/TablesLoader.cpp +++ b/src/Databases/TablesLoader.cpp @@ -2,10 +2,11 @@ #include #include #include +#include #include #include -#include #include +#include namespace DB { @@ -42,94 +43,146 @@ TablesLoader::TablesLoader(ContextMutablePtr global_context_, Databases database void TablesLoader::loadTables() { bool need_resolve_dependencies = !global_context->getConfigRef().has("ignore_table_dependencies_on_metadata_loading"); + + /// Load all Lazy, MySQl, PostgreSQL, SQLite, etc databases first. for (auto & database : databases) { - if (need_resolve_dependencies && database->supportsLoadingInTopologicalOrder()) - databases_to_load.emplace(database->getDatabaseName(), database); + if (need_resolve_dependencies && database.second->supportsLoadingInTopologicalOrder()) + databases_to_load.push_back(database.first); else - database->loadStoredObjects(global_context, force_restore, force_attach, true); + database.second->loadStoredObjects(global_context, force_restore, force_attach, true); } - for (auto & database : databases_to_load) + /// Read and parse metadata from Ordinary, Atomic, Materialized*, Replicated, etc databases. Build dependency graph. + for (auto & database_name : databases_to_load) { - database.second->beforeLoadingMetadata(global_context, force_restore, force_attach); - database.second->loadTablesMetadata(global_context, all_tables); + databases[database_name]->beforeLoadingMetadata(global_context, force_restore, force_attach); + databases[database_name]->loadTablesMetadata(global_context, all_tables); } - auto table_does_not_exist = [this](const QualifiedTableName & table_name, const QualifiedTableName & dependency_name) + LOG_INFO(log, "Parsed metadata of {} tables in {} sec", all_tables.metadata.size(), stopwatch.elapsedSeconds()); + stopwatch.restart(); + + logDependencyGraph(); + + /// Some tables were loaded by database with loadStoredObjects(...). Remove them from graph if necessary. + removeUnresolvableDependencies(); + + loadTablesInTopologicalOrder(pool); +} + +void TablesLoader::startupTables() +{ + /// Startup tables after all tables are loaded. Background tasks (merges, mutations, etc) may slow down data parts loading. + for (auto & database : databases) + database.second->startupTables(pool, force_restore, force_attach); +} + + +void TablesLoader::removeUnresolvableDependencies() +{ + auto need_exclude_dependency = [this](const QualifiedTableName & dependency_name, const DependenciesInfo & info) { + /// Table exists and will be loaded if (all_tables.metadata.contains(dependency_name)) return false; + /// Table exists and it's already loaded if (DatabaseCatalog::instance().isTableExist(StorageID(dependency_name.database, dependency_name.table), global_context)) return false; - /// FIXME if XML dict + /// It's XML dictionary. It was loaded before tables and DDL dictionaries. + if (dependency_name.database == all_tables.default_database && + global_context->getExternalDictionariesLoader().has(dependency_name.table)) + return false; + + /// Some tables depends on table "dependency_name", but there is no such table in DatabaseCatalog and we don't have its metadata. + /// We will ignore it and try to load dependent tables without "dependency_name" + /// (but most likely dependent tables will fail to load). + LOG_WARNING(log, "Tables {} depend on {}, but seems like the it does not exist. Will ignore it and try to load existing tables", + fmt::join(info.dependent_tables, ", "), dependency_name); + + if (info.dependencies_count) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Table {} does not exist, but we have seen its AST and found {} dependencies." + "It's a bug", dependency_name, info.dependencies_count); + if (info.dependent_tables.empty()) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Table {} does not have dependencies and dependent tables as it expected to." + "It's a bug", dependency_name); - LOG_WARNING(log, "Table {} depends on {}, but seems like the second one does not exist", table_name, dependency_name); return true; }; - removeDependencies(table_does_not_exist, all_tables.independent_tables); - - //LOG_TRACE(log, "Independent database objects: {}", fmt::join(all_tables.independent_tables, ", ")); - //for (const auto & dependencies : all_tables.table_dependencies) - // LOG_TRACE(log, "Database object {} depends on: {}", dependencies.first, fmt::join(dependencies.second, ", ")); - - auto is_dependency_loaded = [this](const QualifiedTableName & /*table_name*/, const QualifiedTableName & dependency_name) + auto table_it = all_tables.dependencies_info.begin(); + while (table_it != all_tables.dependencies_info.end()) { - return all_tables.independent_tables.contains(dependency_name); - }; + auto & info = table_it->second; + if (need_exclude_dependency(table_it->first, info)) + table_it = removeResolvedDependency(table_it, all_tables.independent_tables); + else + ++table_it; + } +} - AtomicStopwatch watch; - ThreadPool pool; +void TablesLoader::loadTablesInTopologicalOrder(ThreadPool & pool) +{ + /// While we have some independent tables to load, load them in parallel. + /// Then remove independent tables from graph and find new ones. size_t level = 0; do { - assert(all_tables.metadata.size() == tables_processed + all_tables.independent_tables.size() + all_tables.table_dependencies.size()); - startLoadingIndependentTables(pool, watch, level); - std::unordered_set new_independent_tables; - removeDependencies(is_dependency_loaded, new_independent_tables); + assert(all_tables.metadata.size() == tables_processed + all_tables.independent_tables.size() + getNumberOfTablesWithDependencies()); + logDependencyGraph(); + + startLoadingIndependentTables(pool, level); + + TableNames new_independent_tables; + for (const auto & table_name : all_tables.independent_tables) + { + auto info_it = all_tables.dependencies_info.find(table_name); + if (info_it == all_tables.dependencies_info.end()) + { + /// No tables depend on table_name and it was not even added to dependencies_info + continue; + } + removeResolvedDependency(info_it, new_independent_tables); + } + pool.wait(); + all_tables.independent_tables = std::move(new_independent_tables); - checkCyclicDependencies(); ++level; - assert(all_tables.metadata.size() == tables_processed + all_tables.independent_tables.size() + all_tables.table_dependencies.size()); } while (!all_tables.independent_tables.empty()); - for (auto & database : databases_to_load) - { - database.second->startupTables(pool, force_restore, force_attach); - } + checkCyclicDependencies(); } -void TablesLoader::removeDependencies(RemoveDependencyPredicate need_remove_dependency, std::unordered_set & independent_tables) +DependenciesInfosIter TablesLoader::removeResolvedDependency(const DependenciesInfosIter & info_it, TableNames & independent_tables) { - auto table_it = all_tables.table_dependencies.begin(); - while (table_it != all_tables.table_dependencies.end()) - { - auto & dependencies = table_it->second; - assert(!dependencies.empty()); - auto dependency_it = dependencies.begin(); - while (dependency_it != dependencies.end()) - { - if (need_remove_dependency(table_it->first, *dependency_it)) - dependency_it = dependencies.erase(dependency_it); - else - ++dependency_it; - } + auto & info = info_it->second; + if (info.dependencies_count) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Table {} is in list of independent tables, but dependencies count is {}." + "It's a bug", info_it->first, info.dependencies_count); + if (info.dependent_tables.empty()) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Table {} does not have dependent tables. It's a bug", info_it->first); - if (dependencies.empty()) + /// Decrement number of dependencies for each dependent table + for (auto & dependent_table : info.dependent_tables) + { + auto & dependent_info = all_tables.dependencies_info[dependent_table]; + auto & dependencies_count = dependent_info.dependencies_count; + if (dependencies_count == 0) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Trying to decrement 0 dependencies counter for {}. It's a bug", dependent_table); + --dependencies_count; + if (dependencies_count == 0) { - independent_tables.emplace(std::move(table_it->first)); - table_it = all_tables.table_dependencies.erase(table_it); - } - else - { - ++table_it; + independent_tables.push_back(dependent_table); + if (dependent_info.dependent_tables.empty()) + all_tables.dependencies_info.erase(dependent_table); } } + + return all_tables.dependencies_info.erase(info_it); } -void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, AtomicStopwatch & watch, size_t level) +void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, size_t level) { size_t total_tables = all_tables.metadata.size(); @@ -137,32 +190,56 @@ void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, AtomicStopwa for (const auto & table_name : all_tables.independent_tables) { - pool.scheduleOrThrowOnError([this, total_tables, &table_name, &watch]() + pool.scheduleOrThrowOnError([this, total_tables, &table_name]() { const auto & path_and_query = all_tables.metadata[table_name]; const auto & path = path_and_query.first; const auto & ast = path_and_query.second; - databases_to_load[table_name.database]->loadTableFromMetadata(global_context, path, table_name, ast, force_restore); - logAboutProgress(log, ++tables_processed, total_tables, watch); + databases[table_name.database]->loadTableFromMetadata(global_context, path, table_name, ast, force_restore); + logAboutProgress(log, ++tables_processed, total_tables, stopwatch); }); } } +size_t TablesLoader::getNumberOfTablesWithDependencies() const +{ + size_t number_of_tables_with_dependencies = 0; + for (const auto & info : all_tables.dependencies_info) + if (info.second.dependencies_count) + ++number_of_tables_with_dependencies; + return number_of_tables_with_dependencies; +} + void TablesLoader::checkCyclicDependencies() const { - if (!all_tables.independent_tables.empty()) - return; - if (all_tables.table_dependencies.empty()) + /// Loading is finished if all dependencies are resolved + if (all_tables.dependencies_info.empty()) return; - for (const auto & dependencies : all_tables.table_dependencies) + for (const auto & info : all_tables.dependencies_info) { - LOG_WARNING(log, "Cannot resolve dependencies: Table {} depends on {}", dependencies.first, fmt::join(dependencies.second, ", ")); + LOG_WARNING(log, "Cannot resolve dependencies: Table {} have {} dependencies and {} dependent tables. List of dependent tables: {}", + info.first, info.second.dependencies_count, + info.second.dependent_tables.size(), fmt::join(info.second.dependent_tables, ", ")); + assert(info.second.dependencies_count == 0); } throw Exception(ErrorCodes::INFINITE_LOOP, "Cannot attach {} tables due to cyclic dependencies. " - "See server log for details.", all_tables.table_dependencies.size()); + "See server log for details.", all_tables.dependencies_info.size()); +} + +void TablesLoader::logDependencyGraph() const +{ + LOG_TRACE(log, "Have {} independent tables: {}", all_tables.independent_tables.size(), fmt::join(all_tables.independent_tables, ", ")); + for (const auto & dependencies : all_tables.dependencies_info) + { + LOG_TRACE(log, + "Table {} have {} dependencies and {} dependent tables. List of dependent tables: {}", + dependencies.first, + dependencies.second.dependencies_count, + dependencies.second.dependent_tables.size(), + fmt::join(dependencies.second.dependent_tables, ", ")); + } } } - diff --git a/src/Databases/TablesLoader.h b/src/Databases/TablesLoader.h index fface310bb6..35dae8a5ad6 100644 --- a/src/Databases/TablesLoader.h +++ b/src/Databases/TablesLoader.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -19,32 +20,59 @@ class AtomicStopwatch; namespace DB { +void logAboutProgress(Poco::Logger * log, size_t processed, size_t total, AtomicStopwatch & watch); + + class IDatabase; using DatabasePtr = std::shared_ptr; -void logAboutProgress(Poco::Logger * log, size_t processed, size_t total, AtomicStopwatch & watch); +using ParsedMetadata = std::map>; +using TableNames = std::vector; + +struct DependenciesInfo +{ + /// How many dependencies this table have + size_t dependencies_count = 0; + /// List of tables which depend on this table + TableNames dependent_tables; +}; + +using DependenciesInfos = std::unordered_map; +using DependenciesInfosIter = std::unordered_map::iterator; struct ParsedTablesMetadata { String default_database; - using ParsedMetadata = std::map>; std::mutex mutex; ParsedMetadata metadata; + + /// For logging size_t total_dictionaries = 0; - std::unordered_set independent_tables; - std::unordered_map> table_dependencies; + + /// List of tables that do not have any dependencies and can be loaded + TableNames independent_tables; + + /// Actually it contains two different maps (with, probably, intersecting keys): + /// 1. table name -> number of dependencies + /// 2. table name -> dependent tables list (adjacency list of dependencies graph). + /// If table A depends on table B, then there is an edge B --> A, i.e. dependencies_info[B].dependent_tables contains A. + /// And dependencies_info[C].dependencies_count is a number of incoming edges for vertex C (how many tables we have to load before C). + DependenciesInfos dependencies_info; }; +/// Loads tables (and dictionaries) from specified databases +/// taking into account dependencies between them. class TablesLoader { public: - - using Databases = std::vector; + using Databases = std::map; TablesLoader(ContextMutablePtr global_context_, Databases databases_, bool force_restore_ = false, bool force_attach_ = false); + TablesLoader() = delete; void loadTables(); + void startupTables(); private: ContextMutablePtr global_context; @@ -52,19 +80,27 @@ private: bool force_restore; bool force_attach; - std::map databases_to_load; + Strings databases_to_load; ParsedTablesMetadata all_tables; Poco::Logger * log; std::atomic tables_processed{0}; + AtomicStopwatch stopwatch; + ThreadPool pool; - using RemoveDependencyPredicate = std::function; - void removeDependencies(RemoveDependencyPredicate need_remove_dependency, std::unordered_set & independent_tables); + void removeUnresolvableDependencies(); - void startLoadingIndependentTables(ThreadPool & pool, AtomicStopwatch & watch, size_t level); + void loadTablesInTopologicalOrder(ThreadPool & pool); + + DependenciesInfosIter removeResolvedDependency(const DependenciesInfosIter & info_it, TableNames & independent_tables); + + void startLoadingIndependentTables(ThreadPool & pool, size_t level); void checkCyclicDependencies() const; + size_t getNumberOfTablesWithDependencies() const; + + void logDependencyGraph() const; }; } diff --git a/src/Interpreters/ExternalDictionariesLoader.cpp b/src/Interpreters/ExternalDictionariesLoader.cpp index cbb0e52b91b..2cbcf9e362c 100644 --- a/src/Interpreters/ExternalDictionariesLoader.cpp +++ b/src/Interpreters/ExternalDictionariesLoader.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #if !defined(ARCADIA_BUILD) # include "config_core.h" @@ -30,6 +31,7 @@ ExternalDictionariesLoader::ExternalDictionariesLoader(ContextPtr global_context setConfigSettings({"dictionary", "name", "database", "uuid"}); enableAsyncLoading(true); enablePeriodicUpdates(true); + log = &Poco::Logger::get("EDL"); } ExternalLoader::LoadablePtr ExternalDictionariesLoader::create( @@ -89,12 +91,14 @@ DictionaryStructure ExternalDictionariesLoader::getDictionaryStructure(const std std::string ExternalDictionariesLoader::resolveDictionaryName(const std::string & dictionary_name, const std::string & current_database_name) const { + LOG_INFO(log, "Looking for {} ({})", dictionary_name, current_database_name); bool has_dictionary = has(dictionary_name); if (has_dictionary) return dictionary_name; std::string resolved_name = resolveDictionaryNameFromDatabaseCatalog(dictionary_name); has_dictionary = has(resolved_name); + LOG_INFO(log, "Got resolved name {}, {}", resolved_name, has_dictionary); if (!has_dictionary) { diff --git a/src/Interpreters/ExternalDictionariesLoader.h b/src/Interpreters/ExternalDictionariesLoader.h index 06f64ef30c5..3e698cb5d66 100644 --- a/src/Interpreters/ExternalDictionariesLoader.h +++ b/src/Interpreters/ExternalDictionariesLoader.h @@ -4,6 +4,7 @@ #include #include #include +#include #include @@ -46,6 +47,7 @@ protected: friend class StorageSystemDictionaries; friend class DatabaseDictionary; + Poco::Logger * log; }; } diff --git a/src/Interpreters/ExternalLoader.cpp b/src/Interpreters/ExternalLoader.cpp index eb7824a1124..10051026f6a 100644 --- a/src/Interpreters/ExternalLoader.cpp +++ b/src/Interpreters/ExternalLoader.cpp @@ -14,6 +14,7 @@ #include #include #include +#include namespace CurrentStatusInfo @@ -467,6 +468,7 @@ public: if (infos.find(name) == infos.end()) { Info & info = infos.emplace(name, Info{name, config}).first->second; + LOG_TRACE(log, "Inserted {} into infos", name); if (always_load_everything) { LOG_TRACE(log, "Will load '{}' because always_load_everything flag is set.", name); diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index d885b9a2ac5..db4b8a72a7d 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -275,8 +275,9 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create) if (!load_database_without_tables) { /// We use global context here, because storages lifetime is bigger than query context lifetime - TablesLoader loader{getContext()->getGlobalContext(), {database}, has_force_restore_data_flag, create.attach && force_attach}; //-V560 + TablesLoader loader{getContext()->getGlobalContext(), {{database_name, database}}, has_force_restore_data_flag, create.attach && force_attach}; //-V560 loader.loadTables(); + loader.startupTables(); } } catch (...) diff --git a/src/Interpreters/InterpreterCreateQuery.h b/src/Interpreters/InterpreterCreateQuery.h index 47d0e2f492d..89d27a30555 100644 --- a/src/Interpreters/InterpreterCreateQuery.h +++ b/src/Interpreters/InterpreterCreateQuery.h @@ -52,11 +52,6 @@ public: force_attach = force_attach_; } - void setSkipStartupTables(bool skip_startup_tables_) - { - skip_startup_tables = skip_startup_tables_; - } - void setLoadDatabaseWithoutTables(bool load_database_without_tables_) { load_database_without_tables = load_database_without_tables_; @@ -104,7 +99,6 @@ private: /// Is this an internal query - not from the user. bool internal = false; bool force_attach = false; - bool skip_startup_tables = false; bool load_database_without_tables = false; mutable String as_database_saved; diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index e4c73e7d4e5..a6563553470 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -44,8 +44,7 @@ static void executeCreateQuery( interpreter.setInternal(true); interpreter.setForceAttach(true); interpreter.setForceRestoreData(has_force_restore_data_flag); - interpreter.setSkipStartupTables(true); - interpreter.setLoadDatabaseWithoutTables(database != DatabaseCatalog::SYSTEM_DATABASE); + interpreter.setLoadDatabaseWithoutTables(true); interpreter.execute(); } @@ -161,11 +160,12 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam for (const auto & [name, db_path] : databases) { loadDatabase(context, name, db_path, has_force_restore_data_flag); - loaded_databases.emplace_back(DatabaseCatalog::instance().getDatabase(name)); + loaded_databases.insert({name, DatabaseCatalog::instance().getDatabase(name)}); } TablesLoader loader{context, std::move(loaded_databases), has_force_restore_data_flag, /* force_attach */ true}; loader.loadTables(); + loader.startupTables(); if (has_force_restore_data_flag) { @@ -199,6 +199,17 @@ void loadMetadataSystem(ContextMutablePtr context) executeCreateQuery(database_create_query, context, DatabaseCatalog::SYSTEM_DATABASE, "", true); } + TablesLoader loader{context, {{DatabaseCatalog::SYSTEM_DATABASE, DatabaseCatalog::instance().getSystemDatabase()}}, + /* force_restore */true, /* force_attach */ true}; + loader.loadTables(); + /// Will startup tables in system database after all databases are loaded. +} + + +void startupSystemTables() +{ + ThreadPool pool; + DatabaseCatalog::instance().getSystemDatabase()->startupTables(pool, /* force_restore */true, /* force_attach */ true); } } diff --git a/src/Interpreters/loadMetadata.h b/src/Interpreters/loadMetadata.h index cf038a42855..9ff4432b464 100644 --- a/src/Interpreters/loadMetadata.h +++ b/src/Interpreters/loadMetadata.h @@ -1,6 +1,7 @@ #pragma once #include +#include namespace DB @@ -13,4 +14,6 @@ void loadMetadataSystem(ContextMutablePtr context); /// Load tables from databases and add them to context. Database 'system' is ignored. Use separate function to load system tables. void loadMetadata(ContextMutablePtr context, const String & default_database_name = {}); +void startupSystemTables(); + } From 666a3aee99407e836a9c11fe8f09f6ba40b4e225 Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Thu, 2 Sep 2021 16:34:46 +0300 Subject: [PATCH 04/10] add another test --- src/Databases/DDLDependencyVisitor.cpp | 30 +++++++++++++ src/Databases/DDLDependencyVisitor.h | 4 ++ src/Databases/DatabaseOrdinary.cpp | 2 + src/Databases/IDatabase.h | 3 +- src/Databases/TablesLoader.cpp | 8 ++-- .../ExternalDictionariesLoader.cpp | 4 -- src/Interpreters/ExternalDictionariesLoader.h | 2 - src/Interpreters/ExternalLoader.cpp | 2 - .../test_dictionaries_dependency_xml/test.py | 44 ++++++++++++++++++- 9 files changed, 85 insertions(+), 14 deletions(-) diff --git a/src/Databases/DDLDependencyVisitor.cpp b/src/Databases/DDLDependencyVisitor.cpp index e11d4739604..a5f2ce995a7 100644 --- a/src/Databases/DDLDependencyVisitor.cpp +++ b/src/Databases/DDLDependencyVisitor.cpp @@ -3,6 +3,9 @@ #include #include #include +#include +#include +#include namespace DB { @@ -11,6 +14,8 @@ void DDLDependencyVisitor::visit(const ASTPtr & ast, Data & data) { if (const auto * function = ast->as()) visit(*function, data); + else if (const auto * dict_source = ast->as()) + visit(*dict_source, data); } bool DDLDependencyVisitor::needChildVisit(const ASTPtr & node, const ASTPtr & /*child*/) @@ -29,6 +34,31 @@ void DDLDependencyVisitor::visit(const ASTFunction & function, Data & data) } } +void DDLDependencyVisitor::visit(const ASTFunctionWithKeyValueArguments & dict_source, Data & data) +{ + if (dict_source.name != "clickhouse") + return; + if (!dict_source.elements) + return; + + auto config = getDictionaryConfigurationFromAST(data.create_query->as(), data.global_context); + String host = config->getString("dictionary.source.clickhouse.host", ""); + UInt16 port = config->getUInt("dictionary.source.clickhouse.port", 0); + String database = config->getString("dictionary.source.clickhouse.db", ""); + String table = config->getString("dictionary.source.clickhouse.table", ""); + bool secure = config->getBool("dictionary.source.clickhouse.secure", false); + if (host.empty() || port == 0 || table.empty()) + return; + UInt16 default_port = secure ? data.global_context->getTCPPortSecure().value_or(0) : data.global_context->getTCPPort(); + if (!isLocalAddress({host, port}, default_port)) + return; + + if (database.empty()) + database = data.default_database; + data.dependencies.emplace(QualifiedTableName{std::move(database), std::move(table)}); +} + + void DDLDependencyVisitor::extractTableNameFromArgument(const ASTFunction & function, Data & data, size_t arg_idx) { /// Just ignore incorrect arguments, proper exception will be thrown later diff --git a/src/Databases/DDLDependencyVisitor.h b/src/Databases/DDLDependencyVisitor.h index 43dbbef9e25..1d26adb6e6d 100644 --- a/src/Databases/DDLDependencyVisitor.h +++ b/src/Databases/DDLDependencyVisitor.h @@ -7,6 +7,7 @@ namespace DB { class ASTFunction; +class ASTFunctionWithKeyValueArguments; class DDLDependencyVisitor @@ -17,6 +18,8 @@ public: using TableNamesSet = std::set; String default_database; TableNamesSet dependencies; + ContextPtr global_context; + ASTPtr create_query; }; using Visitor = ConstInDepthNodeVisitor; @@ -26,6 +29,7 @@ public: private: static void visit(const ASTFunction & function, Data & data); + static void visit(const ASTFunctionWithKeyValueArguments & dict_source, Data & data); static void extractTableNameFromArgument(const ASTFunction & function, Data & data, size_t arg_idx); }; diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 77bde83aa65..4c73d3c30ff 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -183,6 +183,8 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables TableLoadingDependenciesVisitor::Data data; data.default_database = metadata.default_database; + data.create_query = ast; + data.global_context = getContext(); TableLoadingDependenciesVisitor visitor{data}; visitor.visit(ast); QualifiedTableName qualified_name{database_name, create_query->table}; diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index dc8c24e0bcc..fe17312cc0b 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -8,8 +8,6 @@ #include #include -#include //FIXME - #include #include #include @@ -29,6 +27,7 @@ struct StorageID; class ASTCreateQuery; using DictionariesWithID = std::vector>; struct ParsedTablesMetadata; +struct QualifiedTableName; namespace ErrorCodes { diff --git a/src/Databases/TablesLoader.cpp b/src/Databases/TablesLoader.cpp index 4aa8f422043..30a9bdd324e 100644 --- a/src/Databases/TablesLoader.cpp +++ b/src/Databases/TablesLoader.cpp @@ -14,6 +14,7 @@ namespace DB namespace ErrorCodes { extern const int INFINITE_LOOP; + extern const int LOGICAL_ERROR; } static constexpr size_t PRINT_MESSAGE_EACH_N_OBJECTS = 256; @@ -60,7 +61,8 @@ void TablesLoader::loadTables() databases[database_name]->loadTablesMetadata(global_context, all_tables); } - LOG_INFO(log, "Parsed metadata of {} tables in {} sec", all_tables.metadata.size(), stopwatch.elapsedSeconds()); + LOG_INFO(log, "Parsed metadata of {} tables in {} databases in {} sec", + all_tables.metadata.size(), databases_to_load.size(), stopwatch.elapsedSeconds()); stopwatch.restart(); logDependencyGraph(); @@ -88,11 +90,11 @@ void TablesLoader::removeUnresolvableDependencies() return false; /// Table exists and it's already loaded if (DatabaseCatalog::instance().isTableExist(StorageID(dependency_name.database, dependency_name.table), global_context)) - return false; + return true; /// It's XML dictionary. It was loaded before tables and DDL dictionaries. if (dependency_name.database == all_tables.default_database && global_context->getExternalDictionariesLoader().has(dependency_name.table)) - return false; + return true; /// Some tables depends on table "dependency_name", but there is no such table in DatabaseCatalog and we don't have its metadata. /// We will ignore it and try to load dependent tables without "dependency_name" diff --git a/src/Interpreters/ExternalDictionariesLoader.cpp b/src/Interpreters/ExternalDictionariesLoader.cpp index 2cbcf9e362c..cbb0e52b91b 100644 --- a/src/Interpreters/ExternalDictionariesLoader.cpp +++ b/src/Interpreters/ExternalDictionariesLoader.cpp @@ -5,7 +5,6 @@ #include #include #include -#include #if !defined(ARCADIA_BUILD) # include "config_core.h" @@ -31,7 +30,6 @@ ExternalDictionariesLoader::ExternalDictionariesLoader(ContextPtr global_context setConfigSettings({"dictionary", "name", "database", "uuid"}); enableAsyncLoading(true); enablePeriodicUpdates(true); - log = &Poco::Logger::get("EDL"); } ExternalLoader::LoadablePtr ExternalDictionariesLoader::create( @@ -91,14 +89,12 @@ DictionaryStructure ExternalDictionariesLoader::getDictionaryStructure(const std std::string ExternalDictionariesLoader::resolveDictionaryName(const std::string & dictionary_name, const std::string & current_database_name) const { - LOG_INFO(log, "Looking for {} ({})", dictionary_name, current_database_name); bool has_dictionary = has(dictionary_name); if (has_dictionary) return dictionary_name; std::string resolved_name = resolveDictionaryNameFromDatabaseCatalog(dictionary_name); has_dictionary = has(resolved_name); - LOG_INFO(log, "Got resolved name {}, {}", resolved_name, has_dictionary); if (!has_dictionary) { diff --git a/src/Interpreters/ExternalDictionariesLoader.h b/src/Interpreters/ExternalDictionariesLoader.h index 3e698cb5d66..06f64ef30c5 100644 --- a/src/Interpreters/ExternalDictionariesLoader.h +++ b/src/Interpreters/ExternalDictionariesLoader.h @@ -4,7 +4,6 @@ #include #include #include -#include #include @@ -47,7 +46,6 @@ protected: friend class StorageSystemDictionaries; friend class DatabaseDictionary; - Poco::Logger * log; }; } diff --git a/src/Interpreters/ExternalLoader.cpp b/src/Interpreters/ExternalLoader.cpp index 10051026f6a..eb7824a1124 100644 --- a/src/Interpreters/ExternalLoader.cpp +++ b/src/Interpreters/ExternalLoader.cpp @@ -14,7 +14,6 @@ #include #include #include -#include namespace CurrentStatusInfo @@ -468,7 +467,6 @@ public: if (infos.find(name) == infos.end()) { Info & info = infos.emplace(name, Info{name, config}).first->second; - LOG_TRACE(log, "Inserted {} into infos", name); if (always_load_everything) { LOG_TRACE(log, "Will load '{}' because always_load_everything flag is set.", name); diff --git a/tests/integration/test_dictionaries_dependency_xml/test.py b/tests/integration/test_dictionaries_dependency_xml/test.py index cfd7d58d574..849fdf57980 100644 --- a/tests/integration/test_dictionaries_dependency_xml/test.py +++ b/tests/integration/test_dictionaries_dependency_xml/test.py @@ -6,7 +6,7 @@ DICTIONARY_FILES = ['configs/dictionaries/dep_x.xml', 'configs/dictionaries/dep_ 'configs/dictionaries/dep_z.xml'] cluster = ClickHouseCluster(__file__) -instance = cluster.add_instance('instance', dictionaries=DICTIONARY_FILES) +instance = cluster.add_instance('instance', dictionaries=DICTIONARY_FILES, stay_alive=True) @pytest.fixture(scope="module") @@ -73,3 +73,45 @@ def test_get_data(started_cluster): assert query("SELECT dictGetString('dep_x', 'a', toUInt64(4))") == "ether\n" assert query("SELECT dictGetString('dep_y', 'a', toUInt64(4))") == "ether\n" assert query("SELECT dictGetString('dep_z', 'a', toUInt64(4))") == "ether\n" + +def dependent_tables_assert(): + res = instance.query("select database || '.' || name from system.tables") + assert "system.join" in res + assert "default.src" in res + assert "dict.dep_y" in res + assert "lazy.log" in res + assert "test.d" in res + assert "default.join" in res + assert "a.t" in res + +def test_dependent_tables(started_cluster): + query = instance.query + query("create database lazy engine=Lazy(10)") + query("create database a") + query("create table lazy.src (n int, m int) engine=Log") + query("create dictionary a.d (n int default 0, m int default 42) primary key n " + "source(clickhouse(host 'localhost' port tcpPort() user 'default' table 'src' password '' db 'lazy'))" + "lifetime(min 1 max 10) layout(flat())") + query("create table system.join (n int, m int) engine=Join(any, left, n)") + query("insert into system.join values (1, 1)") + query("create table src (n int, m default joinGet('system.join', 'm', 1::int)," + "t default dictGetOrNull('a.d', 'm', toUInt64(3))," + "k default dictGet('a.d', 'm', toUInt64(4))) engine=MergeTree order by n") + query("create dictionary test.d (n int default 0, m int default 42) primary key n " + "source(clickhouse(host 'localhost' port tcpPort() user 'default' table 'src' password '' db 'default'))" + "lifetime(min 1 max 10) layout(flat())") + query("create table join (n int, m default dictGet('a.d', 'm', toUInt64(3))," + "k default dictGet('test.d', 'm', toUInt64(0))) engine=Join(any, left, n)") + query("create table lazy.log (n default dictGet(test.d, 'm', toUInt64(0))) engine=Log") + query("create table a.t (n default joinGet('system.join', 'm', 1::int)," + "m default dictGet('test.d', 'm', toUInt64(3))," + "k default joinGet(join, 'm', 1::int)) engine=MergeTree order by n") + + dependent_tables_assert() + instance.restart_clickhouse() + dependent_tables_assert() + query("drop database a") + query("drop database lazy") + query("drop table src") + query("drop table join") + query("drop table system.join") From 1e2844f999e564abcb32dfd04a3a8d21b6ea0432 Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Thu, 2 Sep 2021 16:48:41 +0300 Subject: [PATCH 05/10] support function IN --- src/Databases/DDLDependencyVisitor.cpp | 6 ++++++ .../queries/0_stateless/01160_table_dependencies.reference | 1 + tests/queries/0_stateless/01160_table_dependencies.sh | 4 +++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Databases/DDLDependencyVisitor.cpp b/src/Databases/DDLDependencyVisitor.cpp index a5f2ce995a7..2b70421641b 100644 --- a/src/Databases/DDLDependencyVisitor.cpp +++ b/src/Databases/DDLDependencyVisitor.cpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace DB { @@ -32,6 +33,11 @@ void DDLDependencyVisitor::visit(const ASTFunction & function, Data & data) { extractTableNameFromArgument(function, data, 0); } + else if (Poco::toLower(function.name) == "in") + { + extractTableNameFromArgument(function, data, 1); + } + } void DDLDependencyVisitor::visit(const ASTFunctionWithKeyValueArguments & dict_source, Data & data) diff --git a/tests/queries/0_stateless/01160_table_dependencies.reference b/tests/queries/0_stateless/01160_table_dependencies.reference index 6691df07cb9..39a58b06076 100644 --- a/tests/queries/0_stateless/01160_table_dependencies.reference +++ b/tests/queries/0_stateless/01160_table_dependencies.reference @@ -2,4 +2,5 @@ dict1 dict2 dict_src join +s t diff --git a/tests/queries/0_stateless/01160_table_dependencies.sh b/tests/queries/0_stateless/01160_table_dependencies.sh index 0ea213ba5ff..149439f2981 100755 --- a/tests/queries/0_stateless/01160_table_dependencies.sh +++ b/tests/queries/0_stateless/01160_table_dependencies.sh @@ -25,8 +25,10 @@ PRIMARY KEY n SOURCE(CLICKHOUSE(HOST 'localhost' PORT tcpPort() USER 'default' TABLE 'join' PASSWORD '' DB '$CLICKHOUSE_DATABASE')) LIFETIME(MIN 1 MAX 10) LAYOUT(FLAT());" +$CLICKHOUSE_CLIENT -q "create table s (x default joinGet($CLICKHOUSE_DATABASE.join, 'm', 42::int)) engine=Set" + $CLICKHOUSE_CLIENT -q "create table t (n int, m int default joinGet($CLICKHOUSE_DATABASE.join, 'm', 42::int), -s String default dictGet($CLICKHOUSE_DATABASE.dict1, 's', 42::UInt64)) engine=MergeTree order by n;" +s String default dictGet($CLICKHOUSE_DATABASE.dict1, 's', 42::UInt64), x default in(1, $CLICKHOUSE_DATABASE.s)) engine=MergeTree order by n;" CLICKHOUSE_CLIENT_DEFAULT_DB=$(echo ${CLICKHOUSE_CLIENT} | sed 's/'"--database=${CLICKHOUSE_DATABASE}"'/--database=default/g') From df56e99b87f63b5650057d7ecbad597354f2867f Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Mon, 13 Sep 2021 22:11:16 +0300 Subject: [PATCH 06/10] fixes after review --- src/Backups/renameInCreateQuery.cpp | 25 ++--- src/Core/QualifiedTableName.h | 47 +++++++++ src/Databases/DDLDependencyVisitor.cpp | 55 ++++------- src/Databases/DDLDependencyVisitor.h | 5 +- src/Databases/DatabaseAtomic.cpp | 10 +- src/Databases/DatabaseAtomic.h | 4 +- src/Databases/DatabaseLazy.cpp | 2 +- src/Databases/DatabaseLazy.h | 2 +- src/Databases/DatabaseOnDisk.cpp | 4 +- src/Databases/DatabaseOnDisk.h | 2 +- src/Databases/DatabaseOrdinary.cpp | 40 ++++---- src/Databases/DatabaseOrdinary.h | 2 +- src/Databases/DatabaseReplicated.cpp | 8 +- src/Databases/DatabaseReplicated.h | 4 +- src/Databases/IDatabase.h | 4 +- src/Databases/TablesLoader.cpp | 98 ++++++++++--------- src/Databases/TablesLoader.h | 28 +++--- .../PostgreSQLDictionarySource.cpp | 17 ++-- src/Dictionaries/XDBCDictionarySource.cpp | 17 +--- .../getDictionaryConfigurationFromAST.cpp | 27 ++++- .../getDictionaryConfigurationFromAST.h | 9 ++ src/Functions/FunctionJoinGet.cpp | 21 +--- .../ExternalDictionariesLoader.cpp | 45 ++++----- src/Interpreters/ExternalDictionariesLoader.h | 2 +- src/Interpreters/loadMetadata.h | 2 + src/TableFunctions/TableFunctionRemote.cpp | 11 +-- 26 files changed, 272 insertions(+), 219 deletions(-) diff --git a/src/Backups/renameInCreateQuery.cpp b/src/Backups/renameInCreateQuery.cpp index a36995654ee..5d99ea585b5 100644 --- a/src/Backups/renameInCreateQuery.cpp +++ b/src/Backups/renameInCreateQuery.cpp @@ -160,26 +160,29 @@ namespace if (args.size() <= db_name_index) return; - String db_name = evaluateConstantExpressionForDatabaseName(args[db_name_index], data.context)->as().value.safeGet(); + String name = evaluateConstantExpressionForDatabaseName(args[db_name_index], data.context)->as().value.safeGet(); - String table_name; size_t table_name_index = static_cast(-1); - size_t dot = String::npos; - if (function.name != "Distributed") - dot = db_name.find('.'); - if (dot != String::npos) - { - table_name = db_name.substr(dot + 1); - db_name.resize(dot); - } + + QualifiedTableName qualified_name; + + if (function.name == "Distributed") + qualified_name.table = name; else + qualified_name = QualifiedTableName::parseFromString(name); + + if(qualified_name.database.empty()) { + std::swap(qualified_name.database, qualified_name.table); table_name_index = 2; if (args.size() <= table_name_index) return; - table_name = evaluateConstantExpressionForDatabaseName(args[table_name_index], data.context)->as().value.safeGet(); + qualified_name.table = evaluateConstantExpressionForDatabaseName(args[table_name_index], data.context)->as().value.safeGet(); } + const String & db_name = qualified_name.database; + const String & table_name = qualified_name.table; + if (db_name.empty() || table_name.empty()) return; diff --git a/src/Core/QualifiedTableName.h b/src/Core/QualifiedTableName.h index 2b48d38ca2f..dd043b86ee1 100644 --- a/src/Core/QualifiedTableName.h +++ b/src/Core/QualifiedTableName.h @@ -2,6 +2,8 @@ #include #include +#include +#include #include #include #include @@ -9,6 +11,11 @@ namespace DB { +namespace ErrorCodes +{ +extern const int SYNTAX_ERROR; +} + //TODO replace with StorageID struct QualifiedTableName { @@ -32,6 +39,46 @@ struct QualifiedTableName hash_state.update(table.data(), table.size()); return hash_state.get64(); } + + /// NOTE: It's different from compound identifier parsing and does not support escaping and dots in name. + /// Usually it's better to use ParserIdentifier instead, + /// but we parse DDL dictionary name (and similar things) this way for historical reasons. + static std::optional tryParseFromString(const String & maybe_qualified_name) + { + if (maybe_qualified_name.empty()) + return {}; + + /// Do not allow dot at the beginning and at the end + auto pos = maybe_qualified_name.find('.'); + if (pos == 0 || pos == (maybe_qualified_name.size() - 1)) + return {}; + + QualifiedTableName name; + if (pos == std::string::npos) + { + name.table = std::move(maybe_qualified_name); + } + else if (maybe_qualified_name.find('.', pos + 1)) + { + /// Do not allow multiple dots + return {}; + } + else + { + name.database = maybe_qualified_name.substr(0, pos); + name.table = maybe_qualified_name.substr(pos + 1); + } + + return name; + } + + static QualifiedTableName parseFromString(const String & maybe_qualified_name) + { + auto name = tryParseFromString(maybe_qualified_name); + if (!name) + throw Exception(ErrorCodes::SYNTAX_ERROR, "Invalid qualified name: {}", maybe_qualified_name); + return *name; + } }; } diff --git a/src/Databases/DDLDependencyVisitor.cpp b/src/Databases/DDLDependencyVisitor.cpp index 2b70421641b..0399ec59b16 100644 --- a/src/Databases/DDLDependencyVisitor.cpp +++ b/src/Databases/DDLDependencyVisitor.cpp @@ -4,8 +4,6 @@ #include #include #include -#include -#include #include namespace DB @@ -13,6 +11,7 @@ namespace DB void DDLDependencyVisitor::visit(const ASTPtr & ast, Data & data) { + /// Looking for functions in column default expressions and dictionary source definition if (const auto * function = ast->as()) visit(*function, data); else if (const auto * dict_source = ast->as()) @@ -48,20 +47,14 @@ void DDLDependencyVisitor::visit(const ASTFunctionWithKeyValueArguments & dict_s return; auto config = getDictionaryConfigurationFromAST(data.create_query->as(), data.global_context); - String host = config->getString("dictionary.source.clickhouse.host", ""); - UInt16 port = config->getUInt("dictionary.source.clickhouse.port", 0); - String database = config->getString("dictionary.source.clickhouse.db", ""); - String table = config->getString("dictionary.source.clickhouse.table", ""); - bool secure = config->getBool("dictionary.source.clickhouse.secure", false); - if (host.empty() || port == 0 || table.empty()) - return; - UInt16 default_port = secure ? data.global_context->getTCPPortSecure().value_or(0) : data.global_context->getTCPPort(); - if (!isLocalAddress({host, port}, default_port)) + auto info = getInfoIfClickHouseDictionarySource(config, data.global_context); + + if (!info || !info->is_local) return; - if (database.empty()) - database = data.default_database; - data.dependencies.emplace(QualifiedTableName{std::move(database), std::move(table)}); + if (info->table_name.database.empty()) + info->table_name.database = data.default_database; + data.dependencies.emplace(std::move(info->table_name)); } @@ -71,8 +64,7 @@ void DDLDependencyVisitor::extractTableNameFromArgument(const ASTFunction & func if (!function.arguments || function.arguments->children.size() <= arg_idx) return; - String database_name; - String table_name; + QualifiedTableName qualified_name; const auto * arg = function.arguments->as()->children[arg_idx].get(); if (const auto * literal = arg->as()) @@ -80,31 +72,22 @@ void DDLDependencyVisitor::extractTableNameFromArgument(const ASTFunction & func if (literal->value.getType() != Field::Types::String) return; - String maybe_qualified_name = literal->value.get(); - auto pos = maybe_qualified_name.find('.'); - if (pos == 0 || pos == (maybe_qualified_name.size() - 1)) - { - /// Most likely name is invalid + auto maybe_qualified_name = QualifiedTableName::tryParseFromString(literal->value.get()); + /// Just return if name if invalid + if (!maybe_qualified_name) return; - } - else if (pos == std::string::npos) - { - table_name = std::move(maybe_qualified_name); - } - else - { - database_name = maybe_qualified_name.substr(0, pos); - table_name = maybe_qualified_name.substr(pos + 1); - } + + qualified_name = std::move(*maybe_qualified_name); } else if (const auto * identifier = arg->as()) { auto table_identifier = identifier->createTable(); + /// Just return if table identified is invalid if (!table_identifier) return; - database_name = table_identifier->getDatabaseName(); - table_name = table_identifier->shortName(); + qualified_name.database = table_identifier->getDatabaseName(); + qualified_name.table = table_identifier->shortName(); } else { @@ -112,9 +95,9 @@ void DDLDependencyVisitor::extractTableNameFromArgument(const ASTFunction & func return; } - if (database_name.empty()) - database_name = data.default_database; - data.dependencies.emplace(QualifiedTableName{std::move(database_name), std::move(table_name)}); + if (qualified_name.database.empty()) + qualified_name.database = data.default_database; + data.dependencies.emplace(std::move(qualified_name)); } } diff --git a/src/Databases/DDLDependencyVisitor.h b/src/Databases/DDLDependencyVisitor.h index 1d26adb6e6d..c0b39d70b08 100644 --- a/src/Databases/DDLDependencyVisitor.h +++ b/src/Databases/DDLDependencyVisitor.h @@ -9,7 +9,10 @@ namespace DB class ASTFunction; class ASTFunctionWithKeyValueArguments; - +/// Visits ASTCreateQuery and extracts names of table (or dictionary) dependencies +/// from column default expressions (joinGet, dictGet, etc) +/// or dictionary source (for dictionaries from local ClickHouse table). +/// Does not validate AST, works a best-effort way. class DDLDependencyVisitor { public: diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index 83763ccd856..5c75f6f1036 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -416,9 +416,9 @@ UUID DatabaseAtomic::tryGetTableUUID(const String & table_name) const return UUIDHelpers::Nil; } -void DatabaseAtomic::beforeLoadingMetadata(ContextMutablePtr /*context*/, bool has_force_restore_data_flag, bool /*force_attach*/) +void DatabaseAtomic::beforeLoadingMetadata(ContextMutablePtr /*context*/, bool force_restore, bool /*force_attach*/) { - if (!has_force_restore_data_flag) + if (!force_restore) return; /// Recreate symlinks to table data dirs in case of force restore, because some of them may be broken @@ -435,10 +435,10 @@ void DatabaseAtomic::beforeLoadingMetadata(ContextMutablePtr /*context*/, bool h } void DatabaseAtomic::loadStoredObjects( - ContextMutablePtr local_context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) + ContextMutablePtr local_context, bool force_restore, bool force_attach, bool skip_startup_tables) { - beforeLoadingMetadata(local_context, has_force_restore_data_flag, force_attach); - DatabaseOrdinary::loadStoredObjects(local_context, has_force_restore_data_flag, force_attach, skip_startup_tables); + beforeLoadingMetadata(local_context, force_restore, force_attach); + DatabaseOrdinary::loadStoredObjects(local_context, force_restore, force_attach, skip_startup_tables); } void DatabaseAtomic::startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) diff --git a/src/Databases/DatabaseAtomic.h b/src/Databases/DatabaseAtomic.h index db9cef4dbc6..1fe13f8b27f 100644 --- a/src/Databases/DatabaseAtomic.h +++ b/src/Databases/DatabaseAtomic.h @@ -47,9 +47,9 @@ public: DatabaseTablesIteratorPtr getTablesIterator(ContextPtr context, const FilterByNameFunction & filter_by_table_name) const override; - void loadStoredObjects(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) override; + void loadStoredObjects(ContextMutablePtr context, bool force_restore, bool force_attach, bool skip_startup_tables) override; - void beforeLoadingMetadata(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach) override; + void beforeLoadingMetadata(ContextMutablePtr context, bool force_restore, bool force_attach) override; void startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) override; diff --git a/src/Databases/DatabaseLazy.cpp b/src/Databases/DatabaseLazy.cpp index 7e0e1b7aa43..384c5ff47dd 100644 --- a/src/Databases/DatabaseLazy.cpp +++ b/src/Databases/DatabaseLazy.cpp @@ -36,7 +36,7 @@ DatabaseLazy::DatabaseLazy(const String & name_, const String & metadata_path_, void DatabaseLazy::loadStoredObjects( - ContextMutablePtr local_context, bool /* has_force_restore_data_flag */, bool /*force_attach*/, bool /* skip_startup_tables */) + ContextMutablePtr local_context, bool /* force_restore */, bool /*force_attach*/, bool /* skip_startup_tables */) { iterateMetadataFiles(local_context, [this](const String & file_name) { diff --git a/src/Databases/DatabaseLazy.h b/src/Databases/DatabaseLazy.h index bc79a49b2fe..45c816c2e76 100644 --- a/src/Databases/DatabaseLazy.h +++ b/src/Databases/DatabaseLazy.h @@ -26,7 +26,7 @@ public: bool canContainDistributedTables() const override { return false; } - void loadStoredObjects(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) override; + void loadStoredObjects(ContextMutablePtr context, bool force_restore, bool force_attach, bool skip_startup_tables) override; void createTable( ContextPtr context, diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index 620e560b64c..40edeb5cd27 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -46,7 +46,7 @@ std::pair createTableFromAST( const String & database_name, const String & table_data_path_relative, ContextMutablePtr context, - bool has_force_restore_data_flag) + bool force_restore) { ast_create_query.attach = true; ast_create_query.database = database_name; @@ -88,7 +88,7 @@ std::pair createTableFromAST( context->getGlobalContext(), columns, constraints, - has_force_restore_data_flag) + force_restore) }; } diff --git a/src/Databases/DatabaseOnDisk.h b/src/Databases/DatabaseOnDisk.h index e7dda7cb36b..74056d887ae 100644 --- a/src/Databases/DatabaseOnDisk.h +++ b/src/Databases/DatabaseOnDisk.h @@ -16,7 +16,7 @@ std::pair createTableFromAST( const String & database_name, const String & table_data_path_relative, ContextMutablePtr context, - bool has_force_restore_data_flag); + bool force_restore); /** Get the string with the table definition based on the CREATE query. * It is an ATTACH query that you can execute to create a table from the correspondent database. diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 4c73d3c30ff..1bdb273c9fb 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -39,7 +39,7 @@ namespace DatabaseOrdinary & database, const String & database_name, const String & metadata_path, - bool has_force_restore_data_flag) + bool force_restore) { try { @@ -48,7 +48,7 @@ namespace database_name, database.getTableDataPath(query), context, - has_force_restore_data_flag); + force_restore); database.attachTable(table_name, table, database.getTableDataPath(query)); } @@ -75,7 +75,7 @@ DatabaseOrdinary::DatabaseOrdinary( } void DatabaseOrdinary::loadStoredObjects( - ContextMutablePtr local_context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) + ContextMutablePtr local_context, bool force_restore, bool force_attach, bool skip_startup_tables) { /** Tables load faster if they are loaded in sorted (by name) order. * Otherwise (for the ext4 filesystem), `DirectoryIterator` iterates through them in some order, @@ -85,7 +85,7 @@ void DatabaseOrdinary::loadStoredObjects( ParsedTablesMetadata metadata; loadTablesMetadata(local_context, metadata); - size_t total_tables = metadata.metadata.size() - metadata.total_dictionaries; + size_t total_tables = metadata.parsed_tables.size() - metadata.total_dictionaries; AtomicStopwatch watch; std::atomic dictionaries_processed{0}; @@ -101,18 +101,18 @@ void DatabaseOrdinary::loadStoredObjects( /// loading of its config only, it doesn't involve loading the dictionary itself. /// Attach dictionaries. - for (const auto & name_with_path_and_query : metadata.metadata) + for (const auto & name_with_path_and_query : metadata.parsed_tables) { const auto & name = name_with_path_and_query.first; - const auto & path = name_with_path_and_query.second.first; - const auto & ast = name_with_path_and_query.second.second; + const auto & path = name_with_path_and_query.second.path; + const auto & ast = name_with_path_and_query.second.ast; const auto & create_query = ast->as(); if (create_query.is_dictionary) { pool.scheduleOrThrowOnError([&]() { - loadTableFromMetadata(local_context, path, name, ast, has_force_restore_data_flag); + loadTableFromMetadata(local_context, path, name, ast, force_restore); /// Messages, so that it's not boring to wait for the server to load for a long time. logAboutProgress(log, ++dictionaries_processed, metadata.total_dictionaries, watch); @@ -123,18 +123,18 @@ void DatabaseOrdinary::loadStoredObjects( pool.wait(); /// Attach tables. - for (const auto & name_with_path_and_query : metadata.metadata) + for (const auto & name_with_path_and_query : metadata.parsed_tables) { const auto & name = name_with_path_and_query.first; - const auto & path = name_with_path_and_query.second.first; - const auto & ast = name_with_path_and_query.second.second; + const auto & path = name_with_path_and_query.second.path; + const auto & ast = name_with_path_and_query.second.ast; const auto & create_query = ast->as(); if (!create_query.is_dictionary) { pool.scheduleOrThrowOnError([&]() { - loadTableFromMetadata(local_context, path, name, ast, has_force_restore_data_flag); + loadTableFromMetadata(local_context, path, name, ast, force_restore); /// Messages, so that it's not boring to wait for the server to load for a long time. logAboutProgress(log, ++tables_processed, total_tables, watch); @@ -147,13 +147,13 @@ void DatabaseOrdinary::loadStoredObjects( if (!skip_startup_tables) { /// After all tables was basically initialized, startup them. - startupTables(pool, has_force_restore_data_flag, force_attach); + startupTables(pool, force_restore, force_attach); } } void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTablesMetadata & metadata) { - size_t prev_tables_count = metadata.metadata.size(); + size_t prev_tables_count = metadata.parsed_tables.size(); size_t prev_total_dictionaries = metadata.total_dictionaries; auto process_metadata = [&metadata, this](const String & file_name) @@ -190,16 +190,16 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables QualifiedTableName qualified_name{database_name, create_query->table}; std::lock_guard lock{metadata.mutex}; - metadata.metadata[qualified_name] = std::make_pair(full_path.string(), std::move(ast)); + metadata.parsed_tables[qualified_name] = ParsedTableMetadata{full_path.string(), ast}; if (data.dependencies.empty()) { - metadata.independent_tables.emplace_back(std::move(qualified_name)); + metadata.independent_database_objects.emplace_back(std::move(qualified_name)); } else { for (const auto & dependency : data.dependencies) { - metadata.dependencies_info[dependency].dependent_tables.push_back(qualified_name); + metadata.dependencies_info[dependency].dependent_database_objects.push_back(qualified_name); ++metadata.dependencies_info[qualified_name].dependencies_count; } } @@ -215,11 +215,12 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables iterateMetadataFiles(local_context, process_metadata); - size_t objects_in_database = metadata.metadata.size() - prev_tables_count; + size_t objects_in_database = metadata.parsed_tables.size() - prev_tables_count; size_t dictionaries_in_database = metadata.total_dictionaries - prev_total_dictionaries; size_t tables_in_database = objects_in_database - dictionaries_in_database; - LOG_INFO(log, "Total {} tables and {} dictionaries.", tables_in_database, dictionaries_in_database); + LOG_INFO(log, "Metadata processed, database {} has {} tables and {} dictionaries in total.", + database_name, tables_in_database, dictionaries_in_database); } void DatabaseOrdinary::loadTableFromMetadata(ContextMutablePtr local_context, const String & file_path, const QualifiedTableName & name, const ASTPtr & ast, bool force_restore) @@ -261,6 +262,7 @@ void DatabaseOrdinary::startupTables(ThreadPool & thread_pool, bool /*force_rest } catch (...) { + /// We have to wait for jobs to finish here, because job function has reference to variables on the stack of current thread. thread_pool.wait(); throw; } diff --git a/src/Databases/DatabaseOrdinary.h b/src/Databases/DatabaseOrdinary.h index 3f300bfb3eb..5f6d9a30385 100644 --- a/src/Databases/DatabaseOrdinary.h +++ b/src/Databases/DatabaseOrdinary.h @@ -20,7 +20,7 @@ public: String getEngineName() const override { return "Ordinary"; } - void loadStoredObjects(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) override; + void loadStoredObjects(ContextMutablePtr context, bool force_restore, bool force_attach, bool skip_startup_tables) override; bool supportsLoadingInTopologicalOrder() const override { return true; } diff --git a/src/Databases/DatabaseReplicated.cpp b/src/Databases/DatabaseReplicated.cpp index 9aebc701aa9..c2ff002ea36 100644 --- a/src/Databases/DatabaseReplicated.cpp +++ b/src/Databases/DatabaseReplicated.cpp @@ -305,16 +305,16 @@ void DatabaseReplicated::createReplicaNodesInZooKeeper(const zkutil::ZooKeeperPt createEmptyLogEntry(current_zookeeper); } -void DatabaseReplicated::beforeLoadingMetadata(ContextMutablePtr /*context*/, bool /*has_force_restore_data_flag*/, bool force_attach) +void DatabaseReplicated::beforeLoadingMetadata(ContextMutablePtr /*context*/, bool /*force_restore*/, bool force_attach) { tryConnectToZooKeeperAndInitDatabase(force_attach); } void DatabaseReplicated::loadStoredObjects( - ContextMutablePtr local_context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) + ContextMutablePtr local_context, bool force_restore, bool force_attach, bool skip_startup_tables) { - beforeLoadingMetadata(local_context, has_force_restore_data_flag, force_attach); - DatabaseAtomic::loadStoredObjects(local_context, has_force_restore_data_flag, force_attach, skip_startup_tables); + beforeLoadingMetadata(local_context, force_restore, force_attach); + DatabaseAtomic::loadStoredObjects(local_context, force_restore, force_attach, skip_startup_tables); } void DatabaseReplicated::startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) diff --git a/src/Databases/DatabaseReplicated.h b/src/Databases/DatabaseReplicated.h index daba7dad17b..60526a1e5b0 100644 --- a/src/Databases/DatabaseReplicated.h +++ b/src/Databases/DatabaseReplicated.h @@ -57,9 +57,9 @@ public: void drop(ContextPtr /*context*/) override; - void loadStoredObjects(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach, bool skip_startup_tables) override; + void loadStoredObjects(ContextMutablePtr context, bool force_restore, bool force_attach, bool skip_startup_tables) override; - void beforeLoadingMetadata(ContextMutablePtr context, bool has_force_restore_data_flag, bool force_attach) override; + void beforeLoadingMetadata(ContextMutablePtr context, bool force_restore, bool force_attach) override; void startupTables(ThreadPool & thread_pool, bool force_restore, bool force_attach) override; diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index fe17312cc0b..19279e545eb 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -129,7 +129,7 @@ public: /// You can call only once, right after the object is created. virtual void loadStoredObjects( ContextMutablePtr /*context*/, - bool /*has_force_restore_data_flag*/, + bool /*force_restore*/, bool /*force_attach*/ = false, bool /* skip_startup_tables */ = false) { @@ -139,7 +139,7 @@ public: virtual void beforeLoadingMetadata( ContextMutablePtr /*context*/, - bool /*has_force_restore_data_flag*/, + bool /*force_restore*/, bool /*force_attach*/) { } diff --git a/src/Databases/TablesLoader.cpp b/src/Databases/TablesLoader.cpp index 30a9bdd324e..48d751b5795 100644 --- a/src/Databases/TablesLoader.cpp +++ b/src/Databases/TablesLoader.cpp @@ -36,7 +36,7 @@ TablesLoader::TablesLoader(ContextMutablePtr global_context_, Databases database , force_restore(force_restore_) , force_attach(force_attach_) { - all_tables.default_database = global_context->getCurrentDatabase(); + metadata.default_database = global_context->getCurrentDatabase(); log = &Poco::Logger::get("TablesLoader"); } @@ -54,15 +54,19 @@ void TablesLoader::loadTables() database.second->loadStoredObjects(global_context, force_restore, force_attach, true); } + if (databases_to_load.empty()) + return; + /// Read and parse metadata from Ordinary, Atomic, Materialized*, Replicated, etc databases. Build dependency graph. for (auto & database_name : databases_to_load) { databases[database_name]->beforeLoadingMetadata(global_context, force_restore, force_attach); - databases[database_name]->loadTablesMetadata(global_context, all_tables); + databases[database_name]->loadTablesMetadata(global_context, metadata); } LOG_INFO(log, "Parsed metadata of {} tables in {} databases in {} sec", - all_tables.metadata.size(), databases_to_load.size(), stopwatch.elapsedSeconds()); + metadata.parsed_tables.size(), databases_to_load.size(), stopwatch.elapsedSeconds()); + stopwatch.restart(); logDependencyGraph(); @@ -86,13 +90,13 @@ void TablesLoader::removeUnresolvableDependencies() auto need_exclude_dependency = [this](const QualifiedTableName & dependency_name, const DependenciesInfo & info) { /// Table exists and will be loaded - if (all_tables.metadata.contains(dependency_name)) + if (metadata.parsed_tables.contains(dependency_name)) return false; /// Table exists and it's already loaded if (DatabaseCatalog::instance().isTableExist(StorageID(dependency_name.database, dependency_name.table), global_context)) return true; /// It's XML dictionary. It was loaded before tables and DDL dictionaries. - if (dependency_name.database == all_tables.default_database && + if (dependency_name.database == metadata.default_database && global_context->getExternalDictionariesLoader().has(dependency_name.table)) return true; @@ -100,24 +104,24 @@ void TablesLoader::removeUnresolvableDependencies() /// We will ignore it and try to load dependent tables without "dependency_name" /// (but most likely dependent tables will fail to load). LOG_WARNING(log, "Tables {} depend on {}, but seems like the it does not exist. Will ignore it and try to load existing tables", - fmt::join(info.dependent_tables, ", "), dependency_name); + fmt::join(info.dependent_database_objects, ", "), dependency_name); if (info.dependencies_count) throw Exception(ErrorCodes::LOGICAL_ERROR, "Table {} does not exist, but we have seen its AST and found {} dependencies." "It's a bug", dependency_name, info.dependencies_count); - if (info.dependent_tables.empty()) + if (info.dependent_database_objects.empty()) throw Exception(ErrorCodes::LOGICAL_ERROR, "Table {} does not have dependencies and dependent tables as it expected to." "It's a bug", dependency_name); return true; }; - auto table_it = all_tables.dependencies_info.begin(); - while (table_it != all_tables.dependencies_info.end()) + auto table_it = metadata.dependencies_info.begin(); + while (table_it != metadata.dependencies_info.end()) { auto & info = table_it->second; if (need_exclude_dependency(table_it->first, info)) - table_it = removeResolvedDependency(table_it, all_tables.independent_tables); + table_it = removeResolvedDependency(table_it, metadata.independent_database_objects); else ++table_it; } @@ -125,79 +129,81 @@ void TablesLoader::removeUnresolvableDependencies() void TablesLoader::loadTablesInTopologicalOrder(ThreadPool & pool) { - /// While we have some independent tables to load, load them in parallel. - /// Then remove independent tables from graph and find new ones. + /// Load independent tables in parallel. + /// Then remove loaded tables from dependency graph, find tables/dictionaries that do not have unresolved dependencies anymore, + /// move them to the list of independent tables and load. + /// Repeat until we have some tables to load. + /// If we do not, then either all objects are loaded or there is cyclic dependency. + /// Complexity: O(V + E) size_t level = 0; do { - assert(all_tables.metadata.size() == tables_processed + all_tables.independent_tables.size() + getNumberOfTablesWithDependencies()); + assert(metadata.parsed_tables.size() == tables_processed + metadata.independent_database_objects.size() + getNumberOfTablesWithDependencies()); logDependencyGraph(); startLoadingIndependentTables(pool, level); - TableNames new_independent_tables; - for (const auto & table_name : all_tables.independent_tables) + TableNames new_independent_database_objects; + for (const auto & table_name : metadata.independent_database_objects) { - auto info_it = all_tables.dependencies_info.find(table_name); - if (info_it == all_tables.dependencies_info.end()) + auto info_it = metadata.dependencies_info.find(table_name); + if (info_it == metadata.dependencies_info.end()) { /// No tables depend on table_name and it was not even added to dependencies_info continue; } - removeResolvedDependency(info_it, new_independent_tables); + removeResolvedDependency(info_it, new_independent_database_objects); } pool.wait(); - all_tables.independent_tables = std::move(new_independent_tables); + metadata.independent_database_objects = std::move(new_independent_database_objects); ++level; - } while (!all_tables.independent_tables.empty()); + } while (!metadata.independent_database_objects.empty()); checkCyclicDependencies(); } -DependenciesInfosIter TablesLoader::removeResolvedDependency(const DependenciesInfosIter & info_it, TableNames & independent_tables) +DependenciesInfosIter TablesLoader::removeResolvedDependency(const DependenciesInfosIter & info_it, TableNames & independent_database_objects) { auto & info = info_it->second; if (info.dependencies_count) throw Exception(ErrorCodes::LOGICAL_ERROR, "Table {} is in list of independent tables, but dependencies count is {}." "It's a bug", info_it->first, info.dependencies_count); - if (info.dependent_tables.empty()) + if (info.dependent_database_objects.empty()) throw Exception(ErrorCodes::LOGICAL_ERROR, "Table {} does not have dependent tables. It's a bug", info_it->first); /// Decrement number of dependencies for each dependent table - for (auto & dependent_table : info.dependent_tables) + for (auto & dependent_table : info.dependent_database_objects) { - auto & dependent_info = all_tables.dependencies_info[dependent_table]; + auto & dependent_info = metadata.dependencies_info[dependent_table]; auto & dependencies_count = dependent_info.dependencies_count; if (dependencies_count == 0) throw Exception(ErrorCodes::LOGICAL_ERROR, "Trying to decrement 0 dependencies counter for {}. It's a bug", dependent_table); --dependencies_count; if (dependencies_count == 0) { - independent_tables.push_back(dependent_table); - if (dependent_info.dependent_tables.empty()) - all_tables.dependencies_info.erase(dependent_table); + independent_database_objects.push_back(dependent_table); + if (dependent_info.dependent_database_objects.empty()) + metadata.dependencies_info.erase(dependent_table); } } - return all_tables.dependencies_info.erase(info_it); + return metadata.dependencies_info.erase(info_it); } void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, size_t level) { - size_t total_tables = all_tables.metadata.size(); + size_t total_tables = metadata.parsed_tables.size(); - LOG_INFO(log, "Loading {} tables with {} dependency level", all_tables.independent_tables.size(), level); + LOG_INFO(log, "Loading {} tables with {} dependency level", metadata.independent_database_objects.size(), level); - for (const auto & table_name : all_tables.independent_tables) + for (const auto & table_name : metadata.independent_database_objects) { pool.scheduleOrThrowOnError([this, total_tables, &table_name]() { - const auto & path_and_query = all_tables.metadata[table_name]; - const auto & path = path_and_query.first; - const auto & ast = path_and_query.second; - databases[table_name.database]->loadTableFromMetadata(global_context, path, table_name, ast, force_restore); + const auto & path_and_query = metadata.parsed_tables[table_name]; + databases[table_name.database]->loadTableFromMetadata(global_context, path_and_query.path, table_name, path_and_query.ast, force_restore); logAboutProgress(log, ++tables_processed, total_tables, stopwatch); }); } @@ -206,7 +212,7 @@ void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, size_t level size_t TablesLoader::getNumberOfTablesWithDependencies() const { size_t number_of_tables_with_dependencies = 0; - for (const auto & info : all_tables.dependencies_info) + for (const auto & info : metadata.dependencies_info) if (info.second.dependencies_count) ++number_of_tables_with_dependencies; return number_of_tables_with_dependencies; @@ -215,32 +221,34 @@ size_t TablesLoader::getNumberOfTablesWithDependencies() const void TablesLoader::checkCyclicDependencies() const { /// Loading is finished if all dependencies are resolved - if (all_tables.dependencies_info.empty()) + if (metadata.dependencies_info.empty()) return; - for (const auto & info : all_tables.dependencies_info) + for (const auto & info : metadata.dependencies_info) { LOG_WARNING(log, "Cannot resolve dependencies: Table {} have {} dependencies and {} dependent tables. List of dependent tables: {}", info.first, info.second.dependencies_count, - info.second.dependent_tables.size(), fmt::join(info.second.dependent_tables, ", ")); + info.second.dependent_database_objects.size(), fmt::join(info.second.dependent_database_objects, ", ")); assert(info.second.dependencies_count == 0); } throw Exception(ErrorCodes::INFINITE_LOOP, "Cannot attach {} tables due to cyclic dependencies. " - "See server log for details.", all_tables.dependencies_info.size()); + "See server log for details.", metadata.dependencies_info.size()); } void TablesLoader::logDependencyGraph() const { - LOG_TRACE(log, "Have {} independent tables: {}", all_tables.independent_tables.size(), fmt::join(all_tables.independent_tables, ", ")); - for (const auto & dependencies : all_tables.dependencies_info) + LOG_TEST(log, "Have {} independent tables: {}", + metadata.independent_database_objects.size(), + fmt::join(metadata.independent_database_objects, ", ")); + for (const auto & dependencies : metadata.dependencies_info) { - LOG_TRACE(log, + LOG_TEST(log, "Table {} have {} dependencies and {} dependent tables. List of dependent tables: {}", dependencies.first, dependencies.second.dependencies_count, - dependencies.second.dependent_tables.size(), - fmt::join(dependencies.second.dependent_tables, ", ")); + dependencies.second.dependent_database_objects.size(), + fmt::join(dependencies.second.dependent_database_objects, ", ")); } } diff --git a/src/Databases/TablesLoader.h b/src/Databases/TablesLoader.h index 35dae8a5ad6..12f6c2e86a5 100644 --- a/src/Databases/TablesLoader.h +++ b/src/Databases/TablesLoader.h @@ -26,15 +26,21 @@ void logAboutProgress(Poco::Logger * log, size_t processed, size_t total, Atomic class IDatabase; using DatabasePtr = std::shared_ptr; -using ParsedMetadata = std::map>; +struct ParsedTableMetadata +{ + String path; + ASTPtr ast; +}; + +using ParsedMetadata = std::map; using TableNames = std::vector; struct DependenciesInfo { /// How many dependencies this table have size_t dependencies_count = 0; - /// List of tables which depend on this table - TableNames dependent_tables; + /// List of tables/dictionaries which depend on this table/dictionary + TableNames dependent_database_objects; }; using DependenciesInfos = std::unordered_map; @@ -45,18 +51,18 @@ struct ParsedTablesMetadata String default_database; std::mutex mutex; - ParsedMetadata metadata; + ParsedMetadata parsed_tables; /// For logging size_t total_dictionaries = 0; - /// List of tables that do not have any dependencies and can be loaded - TableNames independent_tables; + /// List of tables/dictionaries that do not have any dependencies and can be loaded + TableNames independent_database_objects; /// Actually it contains two different maps (with, probably, intersecting keys): - /// 1. table name -> number of dependencies - /// 2. table name -> dependent tables list (adjacency list of dependencies graph). - /// If table A depends on table B, then there is an edge B --> A, i.e. dependencies_info[B].dependent_tables contains A. + /// 1. table/dictionary name -> number of dependencies + /// 2. table/dictionary name -> dependent tables/dictionaries list (adjacency list of dependencies graph). + /// If table A depends on table B, then there is an edge B --> A, i.e. dependencies_info[B].dependent_database_objects contains A. /// And dependencies_info[C].dependencies_count is a number of incoming edges for vertex C (how many tables we have to load before C). DependenciesInfos dependencies_info; }; @@ -81,7 +87,7 @@ private: bool force_attach; Strings databases_to_load; - ParsedTablesMetadata all_tables; + ParsedTablesMetadata metadata; Poco::Logger * log; std::atomic tables_processed{0}; AtomicStopwatch stopwatch; @@ -92,7 +98,7 @@ private: void loadTablesInTopologicalOrder(ThreadPool & pool); - DependenciesInfosIter removeResolvedDependency(const DependenciesInfosIter & info_it, TableNames & independent_tables); + DependenciesInfosIter removeResolvedDependency(const DependenciesInfosIter & info_it, TableNames & independent_database_objects); void startLoadingIndependentTables(ThreadPool & pool, size_t level); diff --git a/src/Dictionaries/PostgreSQLDictionarySource.cpp b/src/Dictionaries/PostgreSQLDictionarySource.cpp index 3fe9e899cd9..50be5592918 100644 --- a/src/Dictionaries/PostgreSQLDictionarySource.cpp +++ b/src/Dictionaries/PostgreSQLDictionarySource.cpp @@ -1,6 +1,7 @@ #include "PostgreSQLDictionarySource.h" #include +#include #include "DictionarySourceFactory.h" #include "registerDictionaries.h" @@ -29,19 +30,13 @@ namespace { ExternalQueryBuilder makeExternalQueryBuilder(const DictionaryStructure & dict_struct, const String & schema, const String & table, const String & query, const String & where) { - auto schema_value = schema; - auto table_value = table; + QualifiedTableName qualified_name{schema, table}; + + if (qualified_name.database.empty()) + qualified_name = QualifiedTableName::parseFromString(qualified_name.table); - if (schema_value.empty()) - { - if (auto pos = table_value.find('.'); pos != std::string::npos) - { - schema_value = table_value.substr(0, pos); - table_value = table_value.substr(pos + 1); - } - } /// Do not need db because it is already in a connection string. - return {dict_struct, "", schema_value, table_value, query, where, IdentifierQuotingStyle::DoubleQuotes}; + return {dict_struct, "", qualified_name.database, qualified_name.table, query, where, IdentifierQuotingStyle::DoubleQuotes}; } } diff --git a/src/Dictionaries/XDBCDictionarySource.cpp b/src/Dictionaries/XDBCDictionarySource.cpp index 9fc7e92634b..bf7526580c0 100644 --- a/src/Dictionaries/XDBCDictionarySource.cpp +++ b/src/Dictionaries/XDBCDictionarySource.cpp @@ -38,29 +38,22 @@ namespace const std::string & where_, IXDBCBridgeHelper & bridge_) { - std::string schema = schema_; - std::string table = table_; + QualifiedTableName qualified_name{schema_, table_}; if (bridge_.isSchemaAllowed()) { - if (schema.empty()) - { - if (auto pos = table.find('.'); pos != std::string::npos) - { - schema = table.substr(0, pos); - table = table.substr(pos + 1); - } - } + if (qualified_name.database.empty()) + qualified_name = QualifiedTableName::parseFromString(qualified_name.table); } else { - if (!schema.empty()) + if (!qualified_name.database.empty()) throw Exception(ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, "Dictionary source of type {} specifies a schema but schema is not supported by {}-driver", bridge_.getName()); } - return {dict_struct_, db_, schema, table, query_, where_, bridge_.getIdentifierQuotingStyle()}; + return {dict_struct_, db_, qualified_name.database, qualified_name.table, query_, where_, bridge_.getIdentifierQuotingStyle()}; } } diff --git a/src/Dictionaries/getDictionaryConfigurationFromAST.cpp b/src/Dictionaries/getDictionaryConfigurationFromAST.cpp index c77ac36ade6..0ed5b3af83d 100644 --- a/src/Dictionaries/getDictionaryConfigurationFromAST.cpp +++ b/src/Dictionaries/getDictionaryConfigurationFromAST.cpp @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include @@ -16,6 +15,8 @@ #include #include #include +#include +#include namespace DB @@ -576,4 +577,28 @@ getDictionaryConfigurationFromAST(const ASTCreateQuery & query, ContextPtr conte return conf; } +std::optional +getInfoIfClickHouseDictionarySource(DictionaryConfigurationPtr & config, ContextPtr global_context) +{ + ClickHouseDictionarySourceInfo info; + + String host = config->getString("dictionary.source.clickhouse.host", ""); + UInt16 port = config->getUInt("dictionary.source.clickhouse.port", 0); + String database = config->getString("dictionary.source.clickhouse.db", ""); + String table = config->getString("dictionary.source.clickhouse.table", ""); + bool secure = config->getBool("dictionary.source.clickhouse.secure", false); + + if (host.empty() || port == 0 || table.empty()) + return {}; + + info.table_name = {database, table}; + + UInt16 default_port = secure ? global_context->getTCPPortSecure().value_or(0) : global_context->getTCPPort(); + if (!isLocalAddress({host, port}, default_port)) + return info; + + info.is_local = true; + return info; +} + } diff --git a/src/Dictionaries/getDictionaryConfigurationFromAST.h b/src/Dictionaries/getDictionaryConfigurationFromAST.h index b464fdf1d8c..ec44b9815ff 100644 --- a/src/Dictionaries/getDictionaryConfigurationFromAST.h +++ b/src/Dictionaries/getDictionaryConfigurationFromAST.h @@ -15,4 +15,13 @@ using DictionaryConfigurationPtr = Poco::AutoPtr +getInfoIfClickHouseDictionarySource(DictionaryConfigurationPtr & config, ContextPtr global_context); + } diff --git a/src/Functions/FunctionJoinGet.cpp b/src/Functions/FunctionJoinGet.cpp index ee173607437..f0dff0ac7e4 100644 --- a/src/Functions/FunctionJoinGet.cpp +++ b/src/Functions/FunctionJoinGet.cpp @@ -48,22 +48,11 @@ getJoin(const ColumnsWithTypeAndName & arguments, ContextPtr context) "Illegal type " + arguments[0].type->getName() + " of first argument of function joinGet, expected a const string.", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); - size_t dot = join_name.find('.'); - String database_name; - if (dot == String::npos) - { - database_name = context->getCurrentDatabase(); - dot = 0; - } - else - { - database_name = join_name.substr(0, dot); - ++dot; - } - String table_name = join_name.substr(dot); - if (table_name.empty()) - throw Exception("joinGet does not allow empty table name", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); - auto table = DatabaseCatalog::instance().getTable({database_name, table_name}, std::const_pointer_cast(context)); + auto qualified_name = QualifiedTableName::parseFromString(join_name); + if (qualified_name.database.empty()) + qualified_name.database = context->getCurrentDatabase(); + + auto table = DatabaseCatalog::instance().getTable({qualified_name.database, qualified_name.table}, std::const_pointer_cast(context)); auto storage_join = std::dynamic_pointer_cast(table); if (!storage_join) throw Exception("Table " + join_name + " should have engine StorageJoin", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); diff --git a/src/Interpreters/ExternalDictionariesLoader.cpp b/src/Interpreters/ExternalDictionariesLoader.cpp index cbb0e52b91b..fdd371c5038 100644 --- a/src/Interpreters/ExternalDictionariesLoader.cpp +++ b/src/Interpreters/ExternalDictionariesLoader.cpp @@ -89,47 +89,40 @@ DictionaryStructure ExternalDictionariesLoader::getDictionaryStructure(const std std::string ExternalDictionariesLoader::resolveDictionaryName(const std::string & dictionary_name, const std::string & current_database_name) const { - bool has_dictionary = has(dictionary_name); - if (has_dictionary) + if (has(dictionary_name)) return dictionary_name; - std::string resolved_name = resolveDictionaryNameFromDatabaseCatalog(dictionary_name); - has_dictionary = has(resolved_name); + std::string resolved_name = resolveDictionaryNameFromDatabaseCatalog(dictionary_name, current_database_name); - if (!has_dictionary) - { - /// If dictionary not found. And database was not implicitly specified - /// we can qualify dictionary name with current database name. - /// It will help if dictionary is created with DDL and is in current database. - if (dictionary_name.find('.') == std::string::npos) - { - String dictionary_name_with_database = current_database_name + '.' + dictionary_name; - resolved_name = resolveDictionaryNameFromDatabaseCatalog(dictionary_name_with_database); - has_dictionary = has(resolved_name); - } - } + if (has(resolved_name)) + return resolved_name; - if (!has_dictionary) - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Dictionary ({}) not found", backQuote(dictionary_name)); - - return resolved_name; + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Dictionary ({}) not found", backQuote(dictionary_name)); } -std::string ExternalDictionariesLoader::resolveDictionaryNameFromDatabaseCatalog(const std::string & name) const +std::string ExternalDictionariesLoader::resolveDictionaryNameFromDatabaseCatalog(const std::string & name, const std::string & current_database_name) const { /// If it's dictionary from Atomic database, then we need to convert qualified name to UUID. /// Try to split name and get id from associated StorageDictionary. /// If something went wrong, return name as is. - auto pos = name.find('.'); - if (pos == std::string::npos || name.find('.', pos + 1) != std::string::npos) + auto qualified_name = QualifiedTableName::tryParseFromString(name); + if (!qualified_name) return name; - std::string maybe_database_name = name.substr(0, pos); - std::string maybe_table_name = name.substr(pos + 1); + if (qualified_name->database.empty()) + { + /// Ether database name is not specified and we should use current one + /// or it's an XML dictionary. + bool is_xml_dictionary = has(name); + if (is_xml_dictionary) + return name; + else + qualified_name->database = current_database_name; + } auto [db, table] = DatabaseCatalog::instance().tryGetDatabaseAndTable( - {maybe_database_name, maybe_table_name}, + {qualified_name->database, qualified_name->table}, const_pointer_cast(getContext())); if (!db) diff --git a/src/Interpreters/ExternalDictionariesLoader.h b/src/Interpreters/ExternalDictionariesLoader.h index 06f64ef30c5..f748d75d908 100644 --- a/src/Interpreters/ExternalDictionariesLoader.h +++ b/src/Interpreters/ExternalDictionariesLoader.h @@ -42,7 +42,7 @@ protected: std::string resolveDictionaryName(const std::string & dictionary_name, const std::string & current_database_name) const; /// Try convert qualified dictionary name to persistent UUID - std::string resolveDictionaryNameFromDatabaseCatalog(const std::string & name) const; + std::string resolveDictionaryNameFromDatabaseCatalog(const std::string & name, const std::string & current_database_name) const; friend class StorageSystemDictionaries; friend class DatabaseDictionary; diff --git a/src/Interpreters/loadMetadata.h b/src/Interpreters/loadMetadata.h index e44c84ff2ba..e918b5f530c 100644 --- a/src/Interpreters/loadMetadata.h +++ b/src/Interpreters/loadMetadata.h @@ -15,6 +15,8 @@ void loadMetadataSystem(ContextMutablePtr context); /// Use separate function to load system tables. void loadMetadata(ContextMutablePtr context, const String & default_database_name = {}); +/// Background operations in system tables may slowdown loading of the rest tables, +/// so we startup system tables after all databases are loaded. void startupSystemTables(); } diff --git a/src/TableFunctions/TableFunctionRemote.cpp b/src/TableFunctions/TableFunctionRemote.cpp index 08f61a49fa5..62a0978d42f 100644 --- a/src/TableFunctions/TableFunctionRemote.cpp +++ b/src/TableFunctions/TableFunctionRemote.cpp @@ -93,14 +93,8 @@ void TableFunctionRemote::parseArguments(const ASTPtr & ast_function, ContextPtr ++arg_num; - size_t dot = remote_database.find('.'); - if (dot != String::npos) - { - /// NOTE Bad - do not support identifiers in backquotes. - remote_table = remote_database.substr(dot + 1); - remote_database = remote_database.substr(0, dot); - } - else + auto qualified_name = QualifiedTableName::parseFromString(remote_database); + if (qualified_name.database.empty()) { if (arg_num >= args.size()) { @@ -108,6 +102,7 @@ void TableFunctionRemote::parseArguments(const ASTPtr & ast_function, ContextPtr } else { + std::swap(qualified_name.database, qualified_name.table); args[arg_num] = evaluateConstantExpressionOrIdentifierAsLiteral(args[arg_num], context); remote_table = args[arg_num]->as().value.safeGet(); ++arg_num; From 2ec9b6fe3be4bacce182cfa73df015e1dda14d9a Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Mon, 13 Sep 2021 22:36:55 +0300 Subject: [PATCH 07/10] fix --- src/Core/QualifiedTableName.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/QualifiedTableName.h b/src/Core/QualifiedTableName.h index dd043b86ee1..c1cb9b27d15 100644 --- a/src/Core/QualifiedTableName.h +++ b/src/Core/QualifiedTableName.h @@ -58,7 +58,7 @@ struct QualifiedTableName { name.table = std::move(maybe_qualified_name); } - else if (maybe_qualified_name.find('.', pos + 1)) + else if (maybe_qualified_name.find('.', pos + 1) != std::string::npos) { /// Do not allow multiple dots return {}; From f5c38fe027cb12a0147a30c0b2e46247d6401272 Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Tue, 14 Sep 2021 00:39:50 +0300 Subject: [PATCH 08/10] fix --- src/Backups/renameInCreateQuery.cpp | 2 +- src/TableFunctions/TableFunctionRemote.cpp | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Backups/renameInCreateQuery.cpp b/src/Backups/renameInCreateQuery.cpp index 5d99ea585b5..4c78844d266 100644 --- a/src/Backups/renameInCreateQuery.cpp +++ b/src/Backups/renameInCreateQuery.cpp @@ -171,7 +171,7 @@ namespace else qualified_name = QualifiedTableName::parseFromString(name); - if(qualified_name.database.empty()) + if (qualified_name.database.empty()) { std::swap(qualified_name.database, qualified_name.table); table_name_index = 2; diff --git a/src/TableFunctions/TableFunctionRemote.cpp b/src/TableFunctions/TableFunctionRemote.cpp index 62a0978d42f..3c39e3f2ec0 100644 --- a/src/TableFunctions/TableFunctionRemote.cpp +++ b/src/TableFunctions/TableFunctionRemote.cpp @@ -104,10 +104,13 @@ void TableFunctionRemote::parseArguments(const ASTPtr & ast_function, ContextPtr { std::swap(qualified_name.database, qualified_name.table); args[arg_num] = evaluateConstantExpressionOrIdentifierAsLiteral(args[arg_num], context); - remote_table = args[arg_num]->as().value.safeGet(); + qualified_name.table = args[arg_num]->as().value.safeGet(); ++arg_num; } } + + remote_database = std::move(qualified_name.database); + remote_table = std::move(qualified_name.table); } /// Cluster function may have sharding key for insert From f03484e0dcda7edb65b073f7d549021cb01fef46 Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Tue, 14 Sep 2021 11:46:40 +0300 Subject: [PATCH 09/10] fix test --- .../0_stateless/01372_remote_table_function_empty_table.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/01372_remote_table_function_empty_table.sql b/tests/queries/0_stateless/01372_remote_table_function_empty_table.sql index 4153dc632f3..55c9d3f63d3 100644 --- a/tests/queries/0_stateless/01372_remote_table_function_empty_table.sql +++ b/tests/queries/0_stateless/01372_remote_table_function_empty_table.sql @@ -1,4 +1,4 @@ -SELECT * FROM remote('127..2', 'a.'); -- { serverError 36 } +SELECT * FROM remote('127..2', 'a.'); -- { serverError 62 } -- Clear cache to avoid future errors in the logs SYSTEM DROP DNS CACHE From 749b91347dfbd21248b046df7179aa2403943552 Mon Sep 17 00:00:00 2001 From: tavplubix Date: Fri, 17 Sep 2021 22:39:54 +0300 Subject: [PATCH 10/10] Update PostgreSQLDictionarySource.cpp --- src/Dictionaries/PostgreSQLDictionarySource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Dictionaries/PostgreSQLDictionarySource.cpp b/src/Dictionaries/PostgreSQLDictionarySource.cpp index 50be5592918..ae153eaed53 100644 --- a/src/Dictionaries/PostgreSQLDictionarySource.cpp +++ b/src/Dictionaries/PostgreSQLDictionarySource.cpp @@ -32,7 +32,7 @@ namespace { QualifiedTableName qualified_name{schema, table}; - if (qualified_name.database.empty()) + if (qualified_name.database.empty() && !qualified_name.table.empty()) qualified_name = QualifiedTableName::parseFromString(qualified_name.table); /// Do not need db because it is already in a connection string.